| Message ID | 20161222091538.28702-19-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 22, 2016 at 11:15 AM, Jan Kara <jack@suse.cz> wrote: > Inline these helpers as they are very thin. We still keep them as we > don't want to expose details about how list type is determined. > > Signed-off-by: Jan Kara <jack@suse.cz> This patch looks good, but see comment below about suggested extra cleanup. Reviewed-by: Amir Goldstein <amir73il@gmail.com> > +/* run all the marks in a group, and clear all of the vfsmount marks */ > +static inline void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group) > +{ > + fsnotify_clear_marks_by_group_flags(group, FSNOTIFY_LIST_TYPE_VFSMOUNT); Suggestion for extra cleanup while at it: IMO, the choice of name fsnotify_clear_marks_by_group_flags() was a bad choice, because 1. it sounds like "by group->flags" and its not 2. it is presented as a generic helper, but it is never likely to be used by anything other then those 2 call sites for FAN_MARK_FLUSH api So given the above, I think it would make more sense to name the function fsnotify_clear_marks_by_group_and_type() for what it really is. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 26-12-16 18:57:38, Amir Goldstein wrote: > On Thu, Dec 22, 2016 at 11:15 AM, Jan Kara <jack@suse.cz> wrote: > > Inline these helpers as they are very thin. We still keep them as we > > don't want to expose details about how list type is determined. > > > > Signed-off-by: Jan Kara <jack@suse.cz> > > This patch looks good, but see comment below about suggested extra cleanup. > > Reviewed-by: Amir Goldstein <amir73il@gmail.com> Thanks. > > +/* run all the marks in a group, and clear all of the vfsmount marks */ > > +static inline void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group) > > +{ > > + fsnotify_clear_marks_by_group_flags(group, FSNOTIFY_LIST_TYPE_VFSMOUNT); > > Suggestion for extra cleanup while at it: > IMO, the choice of name fsnotify_clear_marks_by_group_flags() was a > bad choice, because > 1. it sounds like "by group->flags" and its not > 2. it is presented as a generic helper, but it is never likely to be > used by anything other then > those 2 call sites for FAN_MARK_FLUSH api > > So given the above, I think it would make more sense to name the function > fsnotify_clear_marks_by_group_and_type() for what it really is. I agree on the bad choice of the name. I will probably add another cleanup patch that will just name the function fsnotify_clear_marks_by_group() and cleanup comments about that function in several places. Honza
diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c index c6888dab682b..bdc15f736082 100644 --- a/fs/notify/inode_mark.c +++ b/fs/notify/inode_mark.c @@ -31,14 +31,6 @@ #include "../internal.h" /* - * Given a group clear all of the inode marks associated with that group. - */ -void fsnotify_clear_inode_marks_by_group(struct fsnotify_group *group) -{ - fsnotify_clear_marks_by_group_flags(group, FSNOTIFY_LIST_TYPE_INODE); -} - -/* * given a group and inode, find the mark associated with that combination. * if found take a reference to that mark and return it, else return NULL */ diff --git a/fs/notify/vfsmount_mark.c b/fs/notify/vfsmount_mark.c index 548c33e8f2fa..1e692c56deec 100644 --- a/fs/notify/vfsmount_mark.c +++ b/fs/notify/vfsmount_mark.c @@ -29,11 +29,6 @@ #include <linux/fsnotify_backend.h> #include "fsnotify.h" -void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group) -{ - fsnotify_clear_marks_by_group_flags(group, FSNOTIFY_LIST_TYPE_VFSMOUNT); -} - /* * given a group and vfsmount, find the mark associated with that combination. * if found take a reference to that mark and return it, else return NULL diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h index 5c33ba9bf1ec..c485b17edba9 100644 --- a/include/linux/fsnotify_backend.h +++ b/include/linux/fsnotify_backend.h @@ -353,12 +353,18 @@ extern void fsnotify_destroy_mark(struct fsnotify_mark *mark, extern void fsnotify_detach_mark(struct fsnotify_mark *mark); /* free mark */ extern void fsnotify_free_mark(struct fsnotify_mark *mark); -/* run all the marks in a group, and clear all of the vfsmount marks */ -extern void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group); -/* run all the marks in a group, and clear all of the inode marks */ -extern void fsnotify_clear_inode_marks_by_group(struct fsnotify_group *group); /* run all the marks in a group, and clear all of the marks attached to given object type */ extern void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group, unsigned int flags); +/* run all the marks in a group, and clear all of the vfsmount marks */ +static inline void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group) +{ + fsnotify_clear_marks_by_group_flags(group, FSNOTIFY_LIST_TYPE_VFSMOUNT); +} +/* run all the marks in a group, and clear all of the inode marks */ +static inline void fsnotify_clear_inode_marks_by_group(struct fsnotify_group *group) +{ + fsnotify_clear_marks_by_group_flags(group, FSNOTIFY_LIST_TYPE_INODE); +} extern void fsnotify_get_mark(struct fsnotify_mark *mark); extern void fsnotify_put_mark(struct fsnotify_mark *mark); extern void fsnotify_unmount_inodes(struct super_block *sb);
Inline these helpers as they are very thin. We still keep them as we don't want to expose details about how list type is determined. Signed-off-by: Jan Kara <jack@suse.cz> --- fs/notify/inode_mark.c | 8 -------- fs/notify/vfsmount_mark.c | 5 ----- include/linux/fsnotify_backend.h | 14 ++++++++++---- 3 files changed, 10 insertions(+), 17 deletions(-)