From: Amir Goldstein <amir73il@gmail.com>
To: Jan Kara <jack@suse.cz>, Jens Axboe <axboe@kernel.dk>
Cc: Christian Brauner <brauner@kernel.org>, linux-fsdevel@vger.kernel.org
Subject: [PATCH v3] fsnotify: optimize the case of no parent watcher
Date: Tue, 16 Jan 2024 13:32:47 +0200 [thread overview]
Message-ID: <20240116113247.758848-1-amir73il@gmail.com> (raw)
If parent inode is not watching, check for the event in masks of
sb/mount/inode masks early to optimize out most of the code in
__fsnotify_parent() and avoid calling fsnotify().
Jens has reported that this optimization improves BW and IOPS in an
io_uring benchmark by more than 10% and reduces perf reported CPU usage.
before:
+ 4.51% io_uring [kernel.vmlinux] [k] fsnotify
+ 3.67% io_uring [kernel.vmlinux] [k] __fsnotify_parent
after:
+ 2.37% io_uring [kernel.vmlinux] [k] __fsnotify_parent
Reported-and-tested-by: Jens Axboe <axboe@kernel.dk>
Link: https://lore.kernel.org/linux-fsdevel/b45bd8ff-5654-4e67-90a6-aad5e6759e0b@kernel.dk/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
Jan,
Considering that this change looks like a clear win and it actually
the change that you suggested, I cleaned it up a bit and posting for
your consideration.
I've kept the wrappers fsnotify_path() and fsnotify_sb_has_watchers()
although they are not directly related to the optimization, because they
makes the code a bit nicer IMO.
Jens,
I took the liberty to add Reported-and-tested-by to attribute for your
help even though the patch that you tested was slightly different than
this cleaned up version, because logically it should be identical.
Thanks,
Amir.
Changes since v2:
- Add helper fsnotify_object_watched()
- Move the optimization from inlined fsnotify_path() to
extern __fsnotify_parent()
- Optimize for any event - not only content events
- Drop the SB_I_CONTENT_WATCHED flag
fs/notify/fsnotify.c | 28 +++++++++++++++++-----------
include/linux/fsnotify.h | 22 +++++++++++++++-------
2 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 8bfd690e9f10..6582204cca70 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -141,7 +141,7 @@ void __fsnotify_update_child_dentry_flags(struct inode *inode)
}
/* Are inode/sb/mount interested in parent and name info with this event? */
-static bool fsnotify_event_needs_parent(struct inode *inode, struct mount *mnt,
+static bool fsnotify_event_needs_parent(struct inode *inode, __u32 mnt_mask,
__u32 mask)
{
__u32 marks_mask = 0;
@@ -160,13 +160,22 @@ static bool fsnotify_event_needs_parent(struct inode *inode, struct mount *mnt,
/* Did either inode/sb/mount subscribe for events with parent/name? */
marks_mask |= fsnotify_parent_needed_mask(inode->i_fsnotify_mask);
marks_mask |= fsnotify_parent_needed_mask(inode->i_sb->s_fsnotify_mask);
- if (mnt)
- marks_mask |= fsnotify_parent_needed_mask(mnt->mnt_fsnotify_mask);
+ marks_mask |= fsnotify_parent_needed_mask(mnt_mask);
/* Did they subscribe for this event with parent/name info? */
return mask & marks_mask;
}
+/* 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)
+{
+ __u32 marks_mask = inode->i_fsnotify_mask | mnt_mask |
+ inode->i_sb->s_fsnotify_mask;
+
+ return mask & marks_mask & ALL_FSNOTIFY_EVENTS;
+}
+
/*
* Notify this dentry's parent about a child's events with child name info
* if parent is watching or if inode/sb/mount are interested in events with
@@ -179,7 +188,7 @@ int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data,
int data_type)
{
const struct path *path = fsnotify_data_path(data, data_type);
- struct mount *mnt = path ? real_mount(path->mnt) : NULL;
+ __u32 mnt_mask = path ? real_mount(path->mnt)->mnt_fsnotify_mask : 0;
struct inode *inode = d_inode(dentry);
struct dentry *parent;
bool parent_watched = dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED;
@@ -190,16 +199,13 @@ int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data,
struct qstr *file_name = NULL;
int ret = 0;
- /*
- * Do inode/sb/mount care about parent and name info on non-dir?
- * Do they care about any event at all?
- */
- if (!inode->i_fsnotify_marks && !inode->i_sb->s_fsnotify_marks &&
- (!mnt || !mnt->mnt_fsnotify_marks) && !parent_watched)
+ /* Optimize the likely case of nobody watching this path */
+ if (likely(!parent_watched) &&
+ likely(!fsnotify_object_watched(inode, mnt_mask, mask)))
return 0;
parent = NULL;
- parent_needed = fsnotify_event_needs_parent(inode, mnt, mask);
+ parent_needed = fsnotify_event_needs_parent(inode, mnt_mask, mask);
if (!parent_watched && !parent_needed)
goto notify;
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 8300a5286988..2ddfd7d1eca8 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -17,6 +17,12 @@
#include <linux/slab.h>
#include <linux/bug.h>
+/* Are there any inode/mount/sb objects that are being watched at all? */
+static inline bool fsnotify_sb_has_watchers(struct super_block *sb)
+{
+ return atomic_long_read(&sb->s_fsnotify_connectors);
+}
+
/*
* Notify this @dir inode about a change in a child directory entry.
* The directory entry may have turned positive or negative or its inode may
@@ -30,7 +36,7 @@ static inline int fsnotify_name(__u32 mask, const void *data, int data_type,
struct inode *dir, const struct qstr *name,
u32 cookie)
{
- if (atomic_long_read(&dir->i_sb->s_fsnotify_connectors) == 0)
+ if (!fsnotify_sb_has_watchers(dir->i_sb))
return 0;
return fsnotify(mask, data, data_type, dir, name, NULL, cookie);
@@ -44,7 +50,7 @@ static inline void fsnotify_dirent(struct inode *dir, struct dentry *dentry,
static inline void fsnotify_inode(struct inode *inode, __u32 mask)
{
- if (atomic_long_read(&inode->i_sb->s_fsnotify_connectors) == 0)
+ if (!fsnotify_sb_has_watchers(inode->i_sb))
return;
if (S_ISDIR(inode->i_mode))
@@ -59,7 +65,7 @@ static inline int fsnotify_parent(struct dentry *dentry, __u32 mask,
{
struct inode *inode = d_inode(dentry);
- if (atomic_long_read(&inode->i_sb->s_fsnotify_connectors) == 0)
+ if (!fsnotify_sb_has_watchers(inode->i_sb))
return 0;
if (S_ISDIR(inode->i_mode)) {
@@ -89,15 +95,17 @@ static inline void fsnotify_dentry(struct dentry *dentry, __u32 mask)
fsnotify_parent(dentry, mask, dentry, FSNOTIFY_EVENT_DENTRY);
}
-static inline int fsnotify_file(struct file *file, __u32 mask)
+static inline int fsnotify_path(const struct path *path, __u32 mask)
{
- const struct path *path;
+ return fsnotify_parent(path->dentry, mask, path, FSNOTIFY_EVENT_PATH);
+}
+static inline int fsnotify_file(struct file *file, __u32 mask)
+{
if (file->f_mode & FMODE_NONOTIFY)
return 0;
- path = &file->f_path;
- return fsnotify_parent(path->dentry, mask, path, FSNOTIFY_EVENT_PATH);
+ return fsnotify_path(&file->f_path, mask);
}
#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
--
2.34.1
next reply other threads:[~2024-01-16 11:32 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-16 11:32 Amir Goldstein [this message]
2024-01-16 12:04 ` [PATCH v3] fsnotify: optimize the case of no parent watcher Jan Kara
2024-01-16 12:53 ` Amir Goldstein
2024-01-24 16:07 ` Jan Kara
2024-01-24 16:20 ` Amir Goldstein
2024-02-13 19:45 ` Amir Goldstein
2024-02-14 11:23 ` Jan Kara
2024-02-14 13:40 ` Amir Goldstein
2024-02-15 8:36 ` Jan Kara
2024-03-06 14:51 ` Amir Goldstein
2024-03-08 16:00 ` Jan Kara
2024-03-11 13:51 ` Christian Brauner
2024-02-15 15:07 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240116113247.758848-1-amir73il@gmail.com \
--to=amir73il@gmail.com \
--cc=axboe@kernel.dk \
--cc=brauner@kernel.org \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).