On Thu, 12 Dec 2024 at 12:27, Jan Kara <jack@xxxxxxx> wrote:
> Why not:
> if (p->prev_ns == p->mnt_ns) {
> fsnotify_mnt_move(p->mnt_ns, &p->mnt);
> return;
> }
I don't really care, but I think this fails both as an optimization
(zero chance of actually making a difference) and as a readability
improvement.
> > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> > index 24c7c5df4998..a9dc004291bf 100644
> > --- a/fs/notify/fanotify/fanotify.c
> > +++ b/fs/notify/fanotify/fanotify.c
> > @@ -166,6 +166,8 @@ static bool fanotify_should_merge(struct fanotify_event *old,
> > case FANOTIFY_EVENT_TYPE_FS_ERROR:
> > return fanotify_error_event_equal(FANOTIFY_EE(old),
> > FANOTIFY_EE(new));
> > + case FANOTIFY_EVENT_TYPE_MNT:
> > + return false;
>
> Perhaps instead of handling this in fanotify_should_merge(), we could
> modify fanotify_merge() directly to don't even try if the event is of type
> FANOTIFY_EVENT_TYPE_MNT? Similarly as we do it there for permission events.
Okay.
>
> > @@ -303,7 +305,11 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
> > pr_debug("%s: report_mask=%x mask=%x data=%p data_type=%d\n",
> > __func__, iter_info->report_mask, event_mask, data, data_type);
> >
> > - if (!fid_mode) {
> > + if (FAN_GROUP_FLAG(group, FAN_REPORT_MNT))
> > + {
>
> Unusual style here..
Yeah, fixed.
> Now if we expect these mount notification groups will not have more than
> these two events, then probably it isn't worth the hassle. If we expect
> more event types may eventually materialize, it may be worth it. What do
> people think?
I have a bad feeling about just overloading mask values. How about
reserving a single mask bit for all mount events? I.e.
#define FAN_MNT_ATTACH 0x00100001
#define FAN_MNT_DETACH 0x00100002
...
Thanks,
Miklos
