On Wed 11-12-24 16:37:08, Miklos Szeredi wrote:
> Add notifications for attaching and detaching mounts. The following new
> event masks are added:
>
> FAN_MNT_ATTACH - Mount was attached
> FAN_MNT_DETACH - Mount was detached
>
> If a mount is moved, then the event is reported with (FAN_MNT_ATTACH |
> FAN_MNT_DETACH).
>
> These events add an info record of type FAN_EVENT_INFO_TYPE_MNT containing
> these fields identifying the affected mounts:
>
> __u64 mnt_id - the ID of the mount (see statmount(2))
>
> FAN_REPORT_MNT must be supplied to fanotify_init() to receive these events
> and no other type of event can be received with this report type.
>
> Marks are added with FAN_MARK_MNTNS, which records the mount namespace
> belonging to the supplied path.
>
> Prior to this patch mount namespace changes could be monitored by polling
> /proc/self/mountinfo, which did not convey any information about what
> changed.
>
> Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
...
> @@ -1683,17 +1687,53 @@ int may_umount(struct vfsmount *mnt)
>
> EXPORT_SYMBOL(may_umount);
>
> +static void notify_mount(struct mount *p)
> +{
> + if (!p->prev_ns && p->mnt_ns) {
> + fsnotify_mnt_attach(p->mnt_ns, &p->mnt);
> + } else if (p->prev_ns && !p->mnt_ns) {
> + fsnotify_mnt_detach(p->prev_ns, &p->mnt);
> + } else if (p->prev_ns == p->mnt_ns) {
> + fsnotify_mnt_move(p->mnt_ns, &p->mnt);
> + } else {
> + fsnotify_mnt_detach(p->prev_ns, &p->mnt);
> + fsnotify_mnt_attach(p->mnt_ns, &p->mnt);
> + }
Why not:
if (p->prev_ns == p->mnt_ns) {
fsnotify_mnt_move(p->mnt_ns, &p->mnt);
return;
}
if (p->prev_ns)
fsnotify_mnt_detach(p->prev_ns, &p->mnt);
if (p->mnt_ns)
fsnotify_mnt_attach(p->mnt_ns, &p->mnt);
p->prev_ns = p->mnt_ns;
> + p->prev_ns = p->mnt_ns;
> +}
> +
...
> 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.
> @@ -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..
> + if (data_type != FSNOTIFY_EVENT_MNT)
> + return 0;
> + } else if (!fid_mode) {
> /* Do we have path to open a file descriptor? */
> if (!path)
> return 0;
...
> @@ -910,7 +933,7 @@ static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
> BUILD_BUG_ON(FAN_FS_ERROR != FS_ERROR);
> BUILD_BUG_ON(FAN_RENAME != FS_RENAME);
>
> - BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 21);
> + BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 23);
>
> mask = fanotify_group_event_mask(group, iter_info, &match_mask,
> mask, data, data_type, dir);
So we have 9 bits left for fanotify events. I want throw in one idea for
discussion: Since these notification groups that can receive mount
notification events are special (they cannot receive anything else), we
could consider event value spaces separate as well (i.e., have say
FAN_MNT_ATTACH 0x1, FAN_MNT_DETACH 0x2). The internal FS_MNT_ATTACH,
FS_MNT_DETACH event bits would still stay separate for simplicity but those
are much easier to change if we ever start running out of internal fsnotify
event bits since we don't expose them to userspace. What this would mean is
to convert bits from FAN_MNT_* to FS_MNT_* in fanotify_mark() and then back
when copying event to userspace.
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?
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
