Skip to content

Commit 71d7341

Browse files
gormanmjankara
authored andcommitted
fsnotify: Rearrange fast path to minimise overhead when there is no watcher
The fsnotify paths are trivial to hit even when there are no watchers and they are surprisingly expensive. For example, every successful vfs_write() hits fsnotify_modify which calls both fsnotify_parent and fsnotify unless FMODE_NONOTIFY is set which is an internal flag invisible to userspace. As it stands, fsnotify_parent is a guaranteed functional call even if there are no watchers and fsnotify() does a substantial amount of unnecessary work before it checks if there are any watchers. A perf profile showed that applying mnt->mnt_fsnotify_mask in fnotify() was almost half of the total samples taken in that function during a test. This patch rearranges the fast paths to reduce the amount of work done when there are no watchers. The test motivating this was "perf bench sched messaging --pipe". Despite the fact the pipes are anonymous, fsnotify is still called a lot and the overhead is noticeable even though it's completely pointless. It's likely the overhead is negligible for real IO so this is an extreme example. This is a comparison of hackbench using processes and pipes on a 1-socket machine with 8 CPU threads without fanotify watchers. 5.7.0 5.7.0 vanilla fastfsnotify-v1r1 Amean 1 0.4837 ( 0.00%) 0.4630 * 4.27%* Amean 3 1.5447 ( 0.00%) 1.4557 ( 5.76%) Amean 5 2.6037 ( 0.00%) 2.4363 ( 6.43%) Amean 7 3.5987 ( 0.00%) 3.4757 ( 3.42%) Amean 12 5.8267 ( 0.00%) 5.6983 ( 2.20%) Amean 18 8.4400 ( 0.00%) 8.1327 ( 3.64%) Amean 24 11.0187 ( 0.00%) 10.0290 * 8.98%* Amean 30 13.1013 ( 0.00%) 12.8510 ( 1.91%) Amean 32 13.9190 ( 0.00%) 13.2410 ( 4.87%) 5.7.0 5.7.0 vanilla fastfsnotify-v1r1 Duration User 157.05 152.79 Duration System 1279.98 1219.32 Duration Elapsed 182.81 174.52 This is showing that the latencies are improved by roughly 2-9%. The variability is not shown but some of these results are within the noise as this workload heavily overloads the machine. That said, the system CPU usage is reduced by quite a bit so it makes sense to avoid the overhead even if it is a bit tricky to detect at times. A perf profile of just 1 group of tasks showed that 5.14% of samples taken were in either fsnotify() or fsnotify_parent(). With the patch, 2.8% of samples were in fsnotify, mostly function entry and the initial check for watchers. The check for watchers is complicated enough that inlining it may be controversial. [Amir] Slightly simplify with mnt_or_sb_mask => marks_mask Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Mel Gorman <[email protected]> Signed-off-by: Amir Goldstein <[email protected]> Signed-off-by: Jan Kara <[email protected]>
1 parent 47aaabd commit 71d7341

File tree

3 files changed

+27
-14
lines changed

3 files changed

+27
-14
lines changed

fs/notify/fsnotify.c

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ void __fsnotify_update_child_dentry_flags(struct inode *inode)
143143
}
144144

145145
/* Notify this dentry's parent about a child's events. */
146-
int fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data,
146+
int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data,
147147
int data_type)
148148
{
149149
struct dentry *parent;
@@ -174,7 +174,7 @@ int fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data,
174174

175175
return ret;
176176
}
177-
EXPORT_SYMBOL_GPL(fsnotify_parent);
177+
EXPORT_SYMBOL_GPL(__fsnotify_parent);
178178

179179
static int send_to_group(struct inode *to_tell,
180180
__u32 mask, const void *data,
@@ -315,17 +315,11 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
315315
struct fsnotify_iter_info iter_info = {};
316316
struct super_block *sb = to_tell->i_sb;
317317
struct mount *mnt = NULL;
318-
__u32 mnt_or_sb_mask = sb->s_fsnotify_mask;
319318
int ret = 0;
320-
__u32 test_mask = (mask & ALL_FSNOTIFY_EVENTS);
319+
__u32 test_mask, marks_mask;
321320

322-
if (path) {
321+
if (path)
323322
mnt = real_mount(path->mnt);
324-
mnt_or_sb_mask |= mnt->mnt_fsnotify_mask;
325-
}
326-
/* An event "on child" is not intended for a mount/sb mark */
327-
if (mask & FS_EVENT_ON_CHILD)
328-
mnt_or_sb_mask = 0;
329323

330324
/*
331325
* Optimization: srcu_read_lock() has a memory barrier which can
@@ -337,13 +331,22 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
337331
if (!to_tell->i_fsnotify_marks && !sb->s_fsnotify_marks &&
338332
(!mnt || !mnt->mnt_fsnotify_marks))
339333
return 0;
334+
335+
/* An event "on child" is not intended for a mount/sb mark */
336+
marks_mask = to_tell->i_fsnotify_mask;
337+
if (!(mask & FS_EVENT_ON_CHILD)) {
338+
marks_mask |= sb->s_fsnotify_mask;
339+
if (mnt)
340+
marks_mask |= mnt->mnt_fsnotify_mask;
341+
}
342+
340343
/*
341344
* if this is a modify event we may need to clear the ignored masks
342345
* otherwise return if neither the inode nor the vfsmount/sb care about
343346
* this type of event.
344347
*/
345-
if (!(mask & FS_MODIFY) &&
346-
!(test_mask & (to_tell->i_fsnotify_mask | mnt_or_sb_mask)))
348+
test_mask = (mask & ALL_FSNOTIFY_EVENTS);
349+
if (!(mask & FS_MODIFY) && !(test_mask & marks_mask))
347350
return 0;
348351

349352
iter_info.srcu_idx = srcu_read_lock(&fsnotify_mark_srcu);

include/linux/fsnotify.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,16 @@ static inline void fsnotify_dirent(struct inode *dir, struct dentry *dentry,
4444
fsnotify_name(dir, mask, d_inode(dentry), &dentry->d_name, 0);
4545
}
4646

47+
/* Notify this dentry's parent about a child's events. */
48+
static inline int fsnotify_parent(struct dentry *dentry, __u32 mask,
49+
const void *data, int data_type)
50+
{
51+
if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED))
52+
return 0;
53+
54+
return __fsnotify_parent(dentry, mask, data, data_type);
55+
}
56+
4757
/*
4858
* Simple wrappers to consolidate calls fsnotify_parent()/fsnotify() when
4959
* an event is on a file/dentry.

include/linux/fsnotify_backend.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ struct fsnotify_mark {
379379
/* main fsnotify call to send events */
380380
extern int fsnotify(struct inode *to_tell, __u32 mask, const void *data,
381381
int data_type, const struct qstr *name, u32 cookie);
382-
extern int fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data,
382+
extern int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data,
383383
int data_type);
384384
extern void __fsnotify_inode_delete(struct inode *inode);
385385
extern void __fsnotify_vfsmount_delete(struct vfsmount *mnt);
@@ -541,7 +541,7 @@ static inline int fsnotify(struct inode *to_tell, __u32 mask, const void *data,
541541
return 0;
542542
}
543543

544-
static inline int fsnotify_parent(struct dentry *dentry, __u32 mask,
544+
static inline int __fsnotify_parent(struct dentry *dentry, __u32 mask,
545545
const void *data, int data_type)
546546
{
547547
return 0;

0 commit comments

Comments
 (0)