Re: [PATCH v8 03/19] fsnotify: add helper to check if file is actually being watched

Jan Kara
2024-11-20T16:02:49Z

On Fri 15-11-24 10:30:16, Josef Bacik wrote:
> From: Amir Goldstein <amir73il@gmail.com>
>
> So far, we set FMODE_NONOTIFY_ flags at open time if we know that there
> are no permission event watchers at all on the filesystem, but lack of
> FMODE_NONOTIFY_ flags does not mean that the file is actually watched.
>
> To make the flags more accurate we add a helper that checks if the
> file's inode, mount, sb or parent are being watched for a set of events.
>
> This is going to be used for setting FMODE_NONOTIFY_HSM only when the
> specific file is actually watched for pre-content events.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

I did some changes here as well. See below:

> -/* Are there any inode/mount/sb objects that are interested in this event? */
> -static inline bool fsnotify_object_watched(struct inode *inode, __u32 mnt_mask,
> - __u32 mask)
> +/* Are there any inode/mount/sb objects that watch for these events? */
> +static inline __u32 fsnotify_object_watched(struct inode *inode, __u32 mnt_mask,
> + __u32 events_mask)
> {
> __u32 marks_mask = READ_ONCE(inode->i_fsnotify_mask) | mnt_mask |
> READ_ONCE(inode->i_sb->s_fsnotify_mask);
>
> - return mask & marks_mask & ALL_FSNOTIFY_EVENTS;
> + return events_mask & marks_mask;
> }
>
> +/* Are there any inode/mount/sb/parent objects that watch for these events? */
> +__u32 fsnotify_file_object_watched(struct file *file, __u32 events_mask)
> +{
> + struct dentry *dentry = file->f_path.dentry;
> + struct dentry *parent;
> + __u32 marks_mask, mnt_mask =
> + READ_ONCE(real_mount(file->f_path.mnt)->mnt_fsnotify_mask);
> +
> + marks_mask = fsnotify_object_watched(d_inode(dentry), mnt_mask,
> + events_mask);
> +
> + if (likely(!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED)))
> + return marks_mask;
> +
> + parent = dget_parent(dentry);
> + marks_mask |= fsnotify_inode_watches_children(d_inode(parent));
> + dput(parent);
> +
> + return marks_mask & events_mask;
> +}
> +EXPORT_SYMBOL_GPL(fsnotify_file_object_watched);

I find it confusing that fsnotify_object_watched() does not take parent
into account while fsnotify_file_object_watched() does. Furthermore the
naming doesn't very well reflect the fact we are actually returning a mask
of events. I've ended up dropping this helper (it's used in a single place
anyway) and instead doing the same directly in file_set_fsnotify_mode().

@@ -658,6 +660,27 @@ void file_set_fsnotify_mode(struct file *file)
file->f_mode |= FMODE_NONOTIFY | FMODE_NONOTIFY_PERM;
return;
}
+
+ /*
+ * OK, there are some pre-content watchers. Check if anybody can be
+ * watching for pre-content events on *this* file.
+ */
+ mnt_mask = READ_ONCE(real_mount(file->f_path.mnt)->mnt_fsnotify_mask);
+ if (likely(!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED) &&
+ !fsnotify_object_watched(d_inode(dentry), mnt_mask,
+ FSNOTIFY_PRE_CONTENT_EVENTS))) {
+ file->f_mode |= FMODE_NONOTIFY | FMODE_NONOTIFY_PERM;
+ return;
+ }
+
+ /* Even parent is not watching for pre-content events on this file? */
+ parent = dget_parent(dentry);
+ p_mask = fsnotify_inode_watches_children(d_inode(parent));
+ dput(parent);
+ if (!(p_mask & FSNOTIFY_PRE_CONTENT_EVENTS)) {
+ file->f_mode |= FMODE_NONOTIFY | FMODE_NONOTIFY_PERM;
+ return;
+ }
}

Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR