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

[Jan Kara <jack@suse.cz>]

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