Re: [RFC][PATCH v2] fsnotify: optimize the case of no content event watchers

[Amir Goldstein <amir73il@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]

> > So at this time, we have patch v2 which looks like a clear win.
> > It uses a slightly hackish SB_I_CONTENT_WATCHED to work around
> > the fact that real_mount() is not accessible to the inlined call sites.
>
> But won't it be better to move mnt_fsnotify_mask and perhaps
> mnt_fsnotify_marks (to keep things in one place) into struct vfsmount in
> that case?

I am afraid to even bring this up to Al and Christian ;-)

> IMHO working around this visibility problem with extra flag (and
> the code maintaining it) is not a great tradeoff...

I agree.

>
> As I wrote in my email to Jens I think your v3 patch just makes the real
> cost of fsnotify checks visible in fsnotify_path() instead of being hidden
> in the cost of read / write.

There is a flaw in that argument -
Assuming that in Jens' test no parent/mnt/sb are watching,
all the reads/write on regular file would end up calling __fsnotify_parent()
in upstream code - there is nothing to optimize away this call.

But now that I am looking closer at __fsnotify_parent(), I see what you
were trying to say - if I move the checks from fsnotify_path() to the
beginning of __fsnotify_parent(), it should get most of the performance
from v3 patch, so this is all that should be needed is this simple patch
attached.

I cannot really understand why we did not do this sooner...

Jens, if you have time for another round please test.
My expectation would be to see something like

+1.46%  [kernel.vmlinux]  [k] __fsnotify_parent

Thanks,
Amir.

+
+               if (!(mask & marks_mask))
+                       return 0;
+       }

[-- Attachment #2: 0001-fsnotify-optimize-the-case-of-no-parent-watcher.patch --]
[-- Type: text/x-patch, Size: 1455 bytes --]

From 43bbeff601ed510936ac51b572d24388107f033b Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir73il@gmail.com>
Date: Fri, 9 Dec 2022 11:50:26 +0200
Subject: [PATCH] fsnotify: optimize the case of no parent watcher

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 fsnotify().

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fsnotify.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 8bfd690e9f10..ae74f6e60c64 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -190,13 +190,15 @@ 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)
-		return 0;
+	/* Optimize the likely case of parent not watching */
+	if (likely(!parent_watched)) {
+		__u32 marks_mask = inode->i_fsnotify_mask |
+				   inode->i_sb->s_fsnotify_mask |
+				   (mnt ? mnt->mnt_fsnotify_mask : 0);
+
+		if (!(mask & marks_mask))
+			return 0;
+	}
 
 	parent = NULL;
 	parent_needed = fsnotify_event_needs_parent(inode, mnt, mask);
-- 
2.34.1