fsnotify: optimize FMODE_NONOTIFY_PERM for the common cases

The most unlikely watched permission event is FAN_ACCESS_PERM, because
at the time that it was introduced there were no evictable ignore mark,
so subscribing to FAN_ACCESS_PERM would have incured a very high
overhead.

Yet, when we set the fmode to FMODE_NOTIFY_HSM(), we never skip trying
to send FAN_ACCESS_PERM, which is almost always a waste of cycles.

We got to this logic because of bundling FAN_OPEN*_PERM and
FAN_ACCESS_PERM in the same category and because FAN_OPEN_PERM is a
commonly used event.

By open coding fsnotify_open_perm() in fsnotify_open_perm_and_set_mode(),
we no longer need to regard FAN_OPEN*_PERM when calculating fmode.

This leaves the case of having pre-content events and not having any
other permission event in the object masks a more likely case than the
other way around.

Rework the fmode macros and code so that their meaning now refers only
to hooks on an already open file:

- FMODE_NOTIFY_NONE()		skip all events
- FMODE_NOTIFY_ACCESS_PERM()	send all permission events including
  				FAN_ACCESS_PERM
- FMODE_NOTIFY_HSM()		send pre-content permission events

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20250708143641.418603-3-amir73il@gmail.com

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index de7e7425428b21d4293ab97c3d9175bb6efea21a..079b868552c21d6d6f51c2d935f2854c9f8cd6ce 100644 (file)
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -199,8 +199,8 @@ static bool fsnotify_event_needs_parent(struct inode *inode, __u32 mnt_mask,
 }
 
 /* Are there any inode/mount/sb objects that watch for these events? */
-static inline bool fsnotify_object_watched(struct inode *inode, __u32 mnt_mask,
-                                          __u32 mask)
+static inline __u32 fsnotify_object_watched(struct inode *inode, __u32 mnt_mask,
+                                           __u32 mask)
 {
        __u32 marks_mask = READ_ONCE(inode->i_fsnotify_mask) | mnt_mask |
                           READ_ONCE(inode->i_sb->s_fsnotify_mask);
@@ -665,7 +665,7 @@ int fsnotify_open_perm_and_set_mode(struct file *file)
 {
        struct dentry *dentry = file->f_path.dentry, *parent;
        struct super_block *sb = dentry->d_sb;
-       __u32 mnt_mask, p_mask;
+       __u32 mnt_mask, p_mask = 0;
 
        /* Is it a file opened by fanotify? */
        if (FMODE_FSNOTIFY_NONE(file->f_mode))
@@ -683,45 +683,60 @@ int fsnotify_open_perm_and_set_mode(struct file *file)
        }
 
        /*
-        * If there are permission event watchers but no pre-content event
-        * watchers, set FMODE_NONOTIFY | FMODE_NONOTIFY_PERM to indicate that.
+        * OK, there are some permission event watchers. Check if anybody is
+        * watching for permission events on *this* file.
         */
-       if ((!d_is_dir(dentry) && !d_is_reg(dentry)) ||
-           likely(!fsnotify_sb_has_priority_watchers(sb,
-                                               FSNOTIFY_PRIO_PRE_CONTENT))) {
-               file_set_fsnotify_mode(file, FMODE_NONOTIFY |
-                                      FMODE_NONOTIFY_PERM);
-               goto open_perm;
+       mnt_mask = READ_ONCE(real_mount(file->f_path.mnt)->mnt_fsnotify_mask);
+       p_mask = fsnotify_object_watched(d_inode(dentry), mnt_mask,
+                                        ALL_FSNOTIFY_PERM_EVENTS);
+       if (dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED) {
+               parent = dget_parent(dentry);
+               p_mask |= fsnotify_inode_watches_children(d_inode(parent));
+               dput(parent);
        }
 
        /*
-        * OK, there are some pre-content watchers. Check if anybody is
-        * watching for pre-content events on *this* file.
+        * Legacy FAN_ACCESS_PERM events have very high performance overhead,
+        * so unlikely to be used in the wild. If they are used there will be
+        * no optimizations at all.
         */
-       mnt_mask = READ_ONCE(real_mount(file->f_path.mnt)->mnt_fsnotify_mask);
-       if (unlikely(fsnotify_object_watched(d_inode(dentry), mnt_mask,
-                                    FSNOTIFY_PRE_CONTENT_EVENTS))) {
-               /* Enable pre-content events */
+       if (unlikely(p_mask & FS_ACCESS_PERM)) {
+               /* Enable all permission and pre-content events */
                file_set_fsnotify_mode(file, 0);
                goto open_perm;
        }
 
-       /* Is parent watching for pre-content events on this file? */
-       if (dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED) {
-               parent = dget_parent(dentry);
-               p_mask = fsnotify_inode_watches_children(d_inode(parent));
-               dput(parent);
-               if (p_mask & FSNOTIFY_PRE_CONTENT_EVENTS) {
-                       /* Enable pre-content events */
-                       file_set_fsnotify_mode(file, 0);
-                       goto open_perm;
-               }
+       /*
+        * Pre-content events are only supported on regular files.
+        * If there are pre-content event watchers and no permission access
+        * watchers, set FMODE_NONOTIFY | FMODE_NONOTIFY_PERM to indicate that.
+        * That is the common case with HSM service.
+        */
+       if (d_is_reg(dentry) && (p_mask & FSNOTIFY_PRE_CONTENT_EVENTS)) {
+               file_set_fsnotify_mode(file, FMODE_NONOTIFY |
+                                            FMODE_NONOTIFY_PERM);
+               goto open_perm;
        }
-       /* Nobody watching for pre-content events from this file */
-       file_set_fsnotify_mode(file, FMODE_NONOTIFY | FMODE_NONOTIFY_PERM);
+
+       /* Nobody watching permission and pre-content events on this file */
+       file_set_fsnotify_mode(file, FMODE_NONOTIFY_PERM);
 
 open_perm:
-       return fsnotify_open_perm(file);
+       /*
+        * Send open perm events depending on object masks and regardless of
+        * FMODE_NONOTIFY_PERM.
+        */
+       if (file->f_flags & __FMODE_EXEC && p_mask & FS_OPEN_EXEC_PERM) {
+               int ret = fsnotify_path(&file->f_path, FS_OPEN_EXEC_PERM);
+
+               if (ret)
+                       return ret;
+       }
+
+       if (p_mask & FS_OPEN_PERM)
+               return fsnotify_path(&file->f_path, FS_OPEN_PERM);
+
+       return 0;
 }
 #endif
 

diff --git a/include/linux/fs.h b/include/linux/fs.h
index b085f161ed2211af18b9e130335fe4cf87b109c7..bc92fdb8bfcc35ce7dd1d029aa8e7deefc6aee94 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -200,12 +200,12 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 
 /*
  * The two FMODE_NONOTIFY* define which fsnotify events should not be generated
- * for a file. These are the possible values of (f->f_mode &
- * FMODE_FSNOTIFY_MASK) and their meaning:
+ * for an open file. These are the possible values of
+ * (f->f_mode & FMODE_FSNOTIFY_MASK) and their meaning:
  *
  * FMODE_NONOTIFY - suppress all (incl. non-permission) events.
  * FMODE_NONOTIFY_PERM - suppress permission (incl. pre-content) events.
- * FMODE_NONOTIFY | FMODE_NONOTIFY_PERM - suppress only pre-content events.
+ * FMODE_NONOTIFY | FMODE_NONOTIFY_PERM - suppress only FAN_ACCESS_PERM.
  */
 #define FMODE_FSNOTIFY_MASK \
        (FMODE_NONOTIFY | FMODE_NONOTIFY_PERM)
@@ -213,13 +213,13 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 #define FMODE_FSNOTIFY_NONE(mode) \
        ((mode & FMODE_FSNOTIFY_MASK) == FMODE_NONOTIFY)
 #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
-#define FMODE_FSNOTIFY_PERM(mode) \
+#define FMODE_FSNOTIFY_HSM(mode) \
        ((mode & FMODE_FSNOTIFY_MASK) == 0 || \
         (mode & FMODE_FSNOTIFY_MASK) == (FMODE_NONOTIFY | FMODE_NONOTIFY_PERM))
-#define FMODE_FSNOTIFY_HSM(mode) \
+#define FMODE_FSNOTIFY_ACCESS_PERM(mode) \
        ((mode & FMODE_FSNOTIFY_MASK) == 0)
 #else
-#define FMODE_FSNOTIFY_PERM(mode)      0
+#define FMODE_FSNOTIFY_ACCESS_PERM(mode) 0
 #define FMODE_FSNOTIFY_HSM(mode)       0
 #endif
 

diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 8c1fa617d375d4d4984057dc81ecbb7cf360a895..28a9cb13fbfa383d75f511a2de9ceeb87bab4720 100644 (file)
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -147,9 +147,6 @@ static inline int fsnotify_file_area_perm(struct file *file, int perm_mask,
        if (!(perm_mask & (MAY_READ | MAY_WRITE | MAY_ACCESS)))
                return 0;
 
-       if (likely(!FMODE_FSNOTIFY_PERM(file->f_mode)))
-               return 0;
-
        /*
         * read()/write() and other types of access generate pre-content events.
         */
@@ -160,7 +157,8 @@ static inline int fsnotify_file_area_perm(struct file *file, int perm_mask,
                        return ret;
        }
 
-       if (!(perm_mask & MAY_READ))
+       if (!(perm_mask & MAY_READ) ||
+           likely(!FMODE_FSNOTIFY_ACCESS_PERM(file->f_mode)))
                return 0;
 
        /*
@@ -208,22 +206,6 @@ static inline int fsnotify_file_perm(struct file *file, int perm_mask)
        return fsnotify_file_area_perm(file, perm_mask, NULL, 0);
 }
 
-/*
- * fsnotify_open_perm - permission hook before file open
- */
-static inline int fsnotify_open_perm(struct file *file)
-{
-       int ret;
-
-       if (file->f_flags & __FMODE_EXEC) {
-               ret = fsnotify_path(&file->f_path, FS_OPEN_EXEC_PERM);
-               if (ret)
-                       return ret;
-       }
-
-       return fsnotify_path(&file->f_path, FS_OPEN_PERM);
-}
-
 #else
 static inline int fsnotify_open_perm_and_set_mode(struct file *file)
 {
@@ -251,11 +233,6 @@ static inline int fsnotify_file_perm(struct file *file, int perm_mask)
 {
        return 0;
 }
-
-static inline int fsnotify_open_perm(struct file *file)
-{
-       return 0;
-}
 #endif
 
 /*