On Tue 02-10-18 19:23:53, Amir Goldstein wrote: > On Tue, Oct 2, 2018 at 6:56 PM Jan Kara <jack@xxxxxxx> wrote: > > Regarding to this patch I have just two nits: > > > > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c > > > index 94b52157bf8d..e5a3c69848e4 100644 > > > --- a/fs/notify/fanotify/fanotify.c > > > +++ b/fs/notify/fanotify/fanotify.c > > > @@ -131,8 +131,8 @@ static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info, > > > !(marks_mask & FS_ISDIR & ~marks_ignored_mask)) > > > return false; > > > > > > - if (event_mask & FAN_ALL_OUTGOING_EVENTS & marks_mask & > > > - ~marks_ignored_mask) > > > + if (event_mask & FAN_USER_OUTGOING_EVENTS & > > > + marks_mask & ~marks_ignored_mask) > > > return true; > > > > I don't like the _USER_ part of the constant name. How about _KNOWN_? > > I.e., FAN_KNOWN_OUTGOING_EVENTS sounds about like what it should? > > > > Do you mean just for *this* constant? I would rather have some uniformity > across constants if possible. No, I meant for all. Sorry for the confusion. > Errr... Maybe FAN_VALID_EVENTS/FAN_VALID_OUTGOING_EVENTS > Although _VALID_ is more appropriate for user inputs, so maybe just > FAN_KNOWN_OUTGOING_EVENTS as an exception to the convention > is fine. > > Or simply FAN_EVENTS_MASK/FAN_OUTGOING_EVENTS_MASK, like > the recent new constant FAN_MARK_TYPE_MASK? Yeah, MASK is fine with me as well. So are the names with VALID. Just pick one you like the most. > > > diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h > > > index 096c96f4f16a..a67430811006 100644 > > > --- a/include/linux/fanotify.h > > > +++ b/include/linux/fanotify.h > > > @@ -4,6 +4,56 @@ > > > > > > #include <uapi/linux/fanotify.h> > > > > > > -/* not valid from userspace, only kernel internal */ > > > -#define FAN_MARK_ONDIR 0x00000100 > > > +/* > > > + * Flags not valid from userspace, only kernel internal. > > > + * Use high bits so we won't collide with userspace flags. > > > + */ > > > +#define FAN_MARK_ONDIR 0x80000000 > > > > This ought to be a separate change as I wrote above. > > > > Sure and maybe I'll name it FAN_KERN_MARK_ONDIR. Fine by me. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
