* [PATCH 0/2] fsnotify hooks consolidation
@ 2026-03-02 18:37 Amir Goldstein
2026-03-02 18:37 ` [PATCH 1/2] fsnotify: make fsnotify_create() agnostic to file/dir Amir Goldstein
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Amir Goldstein @ 2026-03-02 18:37 UTC (permalink / raw)
To: Jan Kara
Cc: Christian Brauner, Al Viro, Chuck Lever, Jeff Layton,
Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel
Jan,
I've found this opportunity to reduce the amount of sprinkled
fsnotify hooks.
There are still a few fsnotify hooks sprinkled in some pseudo fs,
but all those removed by this series are obvious boiler plate code
and for most of these fs, this removes all of the custom hooks.
I could send a series to convert each fs with its own patch,
but to me that seems a bit unnecessary.
WDYT?
Amir.
Amir Goldstein (2):
fsnotify: make fsnotify_create() agnostic to file/dir
fs: use simple_end_creating helper to consolidate fsnotify hooks
drivers/android/binder/rust_binderfs.c | 11 +++--------
drivers/android/binderfs.c | 10 +++-------
fs/debugfs/inode.c | 5 +----
fs/libfs.c | 14 ++++++++++++++
fs/nfsd/nfsctl.c | 11 +++--------
fs/tracefs/event_inode.c | 2 --
fs/tracefs/inode.c | 5 +----
include/linux/fs.h | 1 +
include/linux/fsnotify.h | 8 +++++++-
net/sunrpc/rpc_pipe.c | 10 +++-------
10 files changed, 36 insertions(+), 41 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/2] fsnotify: make fsnotify_create() agnostic to file/dir 2026-03-02 18:37 [PATCH 0/2] fsnotify hooks consolidation Amir Goldstein @ 2026-03-02 18:37 ` Amir Goldstein 2026-03-02 18:37 ` [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks Amir Goldstein 2026-03-03 12:45 ` [PATCH 0/2] fsnotify hooks consolidation Jeff Layton 2 siblings, 0 replies; 16+ messages in thread From: Amir Goldstein @ 2026-03-02 18:37 UTC (permalink / raw) To: Jan Kara Cc: Christian Brauner, Al Viro, Chuck Lever, Jeff Layton, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel Like fsnotify_delete(), let fsnotify_create() handle creation of both dir and non-dir objects, if d_inode is available at creation time. Unlike fsnotify_rmdir(), we do not call fsnotify_create() from fsnotify_mkdir(), because of the case where d_inode is instantiated lazily (e.g. kernfs). Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- include/linux/fsnotify.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h index 079c18bcdbde6..13156d165d845 100644 --- a/include/linux/fsnotify.h +++ b/include/linux/fsnotify.h @@ -318,9 +318,15 @@ static inline void fsnotify_inoderemove(struct inode *inode) */ static inline void fsnotify_create(struct inode *dir, struct dentry *dentry) { + struct inode *inode = d_inode(dentry); + __u32 mask = FS_CREATE; + + if (inode && S_ISDIR(inode->i_mode)) + mask |= FS_ISDIR; + audit_inode_child(dir, dentry, AUDIT_TYPE_CHILD_CREATE); - fsnotify_dirent(dir, dentry, FS_CREATE); + fsnotify_dirent(dir, dentry, mask); } /* -- 2.53.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks 2026-03-02 18:37 [PATCH 0/2] fsnotify hooks consolidation Amir Goldstein 2026-03-02 18:37 ` [PATCH 1/2] fsnotify: make fsnotify_create() agnostic to file/dir Amir Goldstein @ 2026-03-02 18:37 ` Amir Goldstein 2026-03-02 22:28 ` Chuck Lever 2026-03-03 15:01 ` Alice Ryhl 2026-03-03 12:45 ` [PATCH 0/2] fsnotify hooks consolidation Jeff Layton 2 siblings, 2 replies; 16+ messages in thread From: Amir Goldstein @ 2026-03-02 18:37 UTC (permalink / raw) To: Jan Kara Cc: Christian Brauner, Al Viro, Chuck Lever, Jeff Layton, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel Add simple_end_creating() helper which combines fsnotify_create/mkdir() hook and simple_done_creating(). Use the new helper to consolidate this pattern in several pseudo fs which had open coded fsnotify_create/mkdir() hooks: binderfs, debugfs, nfsctl, tracefs, rpc_pipefs. For those filesystems, the paired fsnotify_delete() hook is already inside the library helper simple_recursive_removal(). Note that in debugfs_create_symlink(), the fsnotify hook was missing, so the missing hook is fixed by this change. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- drivers/android/binder/rust_binderfs.c | 11 +++-------- drivers/android/binderfs.c | 10 +++------- fs/debugfs/inode.c | 5 +---- fs/libfs.c | 14 ++++++++++++++ fs/nfsd/nfsctl.c | 11 +++-------- fs/tracefs/event_inode.c | 2 -- fs/tracefs/inode.c | 5 +---- include/linux/fs.h | 1 + net/sunrpc/rpc_pipe.c | 10 +++------- 9 files changed, 29 insertions(+), 40 deletions(-) diff --git a/drivers/android/binder/rust_binderfs.c b/drivers/android/binder/rust_binderfs.c index ade1c4d924992..d69490c6b9e09 100644 --- a/drivers/android/binder/rust_binderfs.c +++ b/drivers/android/binder/rust_binderfs.c @@ -3,7 +3,6 @@ #include <linux/compiler_types.h> #include <linux/errno.h> #include <linux/fs.h> -#include <linux/fsnotify.h> #include <linux/gfp.h> #include <linux/idr.h> #include <linux/init.h> @@ -186,9 +185,7 @@ static int binderfs_binder_device_create(struct inode *ref_inode, inode->i_private = device; d_make_persistent(dentry, inode); - - fsnotify_create(root->d_inode, dentry); - simple_done_creating(dentry); + simple_end_creating(dentry); return 0; @@ -481,8 +478,7 @@ static struct dentry *rust_binderfs_create_file(struct dentry *parent, const cha } d_make_persistent(dentry, new_inode); - fsnotify_create(parent->d_inode, dentry); - simple_done_creating(dentry); + simple_end_creating(dentry); return dentry; } @@ -522,8 +518,7 @@ static struct dentry *binderfs_create_dir(struct dentry *parent, inc_nlink(parent->d_inode); set_nlink(new_inode, 2); d_make_persistent(dentry, new_inode); - fsnotify_mkdir(parent->d_inode, dentry); - simple_done_creating(dentry); + simple_end_creating(dentry); return dentry; } diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c index 361d69f756f50..60277f0bb679f 100644 --- a/drivers/android/binderfs.c +++ b/drivers/android/binderfs.c @@ -3,7 +3,6 @@ #include <linux/compiler_types.h> #include <linux/errno.h> #include <linux/fs.h> -#include <linux/fsnotify.h> #include <linux/gfp.h> #include <linux/idr.h> #include <linux/init.h> @@ -190,8 +189,7 @@ static int binderfs_binder_device_create(struct inode *ref_inode, } inode->i_private = device; d_make_persistent(dentry, inode); - fsnotify_create(root->d_inode, dentry); - simple_done_creating(dentry); + simple_end_creating(dentry); binder_add_device(device); @@ -490,8 +488,7 @@ struct dentry *binderfs_create_file(struct dentry *parent, const char *name, new_inode->i_fop = fops; new_inode->i_private = data; d_make_persistent(dentry, new_inode); - fsnotify_create(parent_inode, dentry); - simple_done_creating(dentry); + simple_end_creating(dentry); return dentry; // borrowed } @@ -521,8 +518,7 @@ static struct dentry *binderfs_create_dir(struct dentry *parent, set_nlink(new_inode, 2); d_make_persistent(dentry, new_inode); inc_nlink(parent_inode); - fsnotify_mkdir(parent_inode, dentry); - simple_done_creating(dentry); + simple_end_creating(dentry); return dentry; } diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 4598142355b97..f51b008d42cbf 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -409,7 +409,7 @@ static struct dentry *debugfs_failed_creating(struct dentry *dentry) static struct dentry *debugfs_end_creating(struct dentry *dentry) { - simple_done_creating(dentry); + simple_end_creating(dentry); return dentry; // borrowed } @@ -448,7 +448,6 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode, DEBUGFS_I(inode)->aux = (void *)aux; d_make_persistent(dentry, inode); - fsnotify_create(d_inode(dentry->d_parent), dentry); return debugfs_end_creating(dentry); } @@ -590,7 +589,6 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent) inc_nlink(inode); d_make_persistent(dentry, inode); inc_nlink(d_inode(dentry->d_parent)); - fsnotify_mkdir(d_inode(dentry->d_parent), dentry); return debugfs_end_creating(dentry); } EXPORT_SYMBOL_GPL(debugfs_create_dir); @@ -632,7 +630,6 @@ struct dentry *debugfs_create_automount(const char *name, inc_nlink(inode); d_make_persistent(dentry, inode); inc_nlink(d_inode(dentry->d_parent)); - fsnotify_mkdir(d_inode(dentry->d_parent), dentry); return debugfs_end_creating(dentry); } EXPORT_SYMBOL(debugfs_create_automount); diff --git a/fs/libfs.c b/fs/libfs.c index 74134ba2e8d1e..2f6dcae40ce7f 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -2322,3 +2322,17 @@ void simple_done_creating(struct dentry *child) dput(child); } EXPORT_SYMBOL(simple_done_creating); + +/* + * Like simple_done_creating(), but also fires fsnotify_create() for the + * new dentry. Call on the success path after d_make_persistent(). + * Use simple_done_creating() on the failure path where the child dentry is + * still negative. + */ +void simple_end_creating(struct dentry *child) +{ + fsnotify_create(child->d_parent->d_inode, child); + inode_unlock(child->d_parent->d_inode); + dput(child); +} +EXPORT_SYMBOL(simple_end_creating); diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index e9acd2cd602cb..6e600d52b66d0 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -17,7 +17,6 @@ #include <linux/sunrpc/rpc_pipe_fs.h> #include <linux/sunrpc/svc.h> #include <linux/module.h> -#include <linux/fsnotify.h> #include <linux/nfslocalio.h> #include "idmap.h" @@ -1146,8 +1145,7 @@ static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *nc } d_make_persistent(dentry, inode); inc_nlink(dir); - fsnotify_mkdir(dir, dentry); - simple_done_creating(dentry); + simple_end_creating(dentry); return dentry; // borrowed } @@ -1178,8 +1176,7 @@ static void _nfsd_symlink(struct dentry *parent, const char *name, inode->i_size = strlen(content); d_make_persistent(dentry, inode); - fsnotify_create(dir, dentry); - simple_done_creating(dentry); + simple_end_creating(dentry); } #else static inline void _nfsd_symlink(struct dentry *parent, const char *name, @@ -1219,7 +1216,6 @@ static int nfsdfs_create_files(struct dentry *root, struct nfsdfs_client *ncl, struct dentry **fdentries) { - struct inode *dir = d_inode(root); struct dentry *dentry; for (int i = 0; files->name && files->name[0]; i++, files++) { @@ -1236,10 +1232,9 @@ static int nfsdfs_create_files(struct dentry *root, inode->i_fop = files->ops; inode->i_private = ncl; d_make_persistent(dentry, inode); - fsnotify_create(dir, dentry); if (fdentries) fdentries[i] = dentry; // borrowed - simple_done_creating(dentry); + simple_end_creating(dentry); } return 0; } diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 8e5ac464b3284..059ea7560b9b4 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -14,7 +14,6 @@ * inodes/dentries in a just-in-time (JIT) manner. The eventfs will clean up * and delete the inodes/dentries when they are no longer referenced. */ -#include <linux/fsnotify.h> #include <linux/fs.h> #include <linux/namei.h> #include <linux/workqueue.h> @@ -826,7 +825,6 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry d_make_persistent(dentry, inode); /* The dentry of the "events" parent does keep track though */ inc_nlink(dentry->d_parent->d_inode); - fsnotify_mkdir(dentry->d_parent->d_inode, dentry); tracefs_end_creating(dentry); return ei; diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c index 51c00c8fa1755..90a15c064e309 100644 --- a/fs/tracefs/inode.c +++ b/fs/tracefs/inode.c @@ -16,7 +16,6 @@ #include <linux/kobject.h> #include <linux/namei.h> #include <linux/tracefs.h> -#include <linux/fsnotify.h> #include <linux/security.h> #include <linux/seq_file.h> #include <linux/magic.h> @@ -578,7 +577,7 @@ struct dentry *tracefs_failed_creating(struct dentry *dentry) struct dentry *tracefs_end_creating(struct dentry *dentry) { - simple_done_creating(dentry); + simple_end_creating(dentry); return dentry; // borrowed } @@ -661,7 +660,6 @@ struct dentry *tracefs_create_file(const char *name, umode_t mode, inode->i_uid = d_inode(dentry->d_parent)->i_uid; inode->i_gid = d_inode(dentry->d_parent)->i_gid; d_make_persistent(dentry, inode); - fsnotify_create(d_inode(dentry->d_parent), dentry); return tracefs_end_creating(dentry); } @@ -693,7 +691,6 @@ static struct dentry *__create_dir(const char *name, struct dentry *parent, inc_nlink(inode); d_make_persistent(dentry, inode); inc_nlink(d_inode(dentry->d_parent)); - fsnotify_mkdir(d_inode(dentry->d_parent), dentry); return tracefs_end_creating(dentry); } diff --git a/include/linux/fs.h b/include/linux/fs.h index 8b3dd145b25ec..8e5080be34308 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3271,6 +3271,7 @@ extern int simple_pin_fs(struct file_system_type *, struct vfsmount **mount, int extern void simple_release_fs(struct vfsmount **mount, int *count); struct dentry *simple_start_creating(struct dentry *, const char *); void simple_done_creating(struct dentry *); +void simple_end_creating(struct dentry *); extern ssize_t simple_read_from_buffer(void __user *to, size_t count, loff_t *ppos, const void *from, size_t available); diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index 9d349cfbc4831..84d0ba37d9d6f 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -16,7 +16,6 @@ #include <linux/mount.h> #include <linux/fs_context.h> #include <linux/namei.h> -#include <linux/fsnotify.h> #include <linux/kernel.h> #include <linux/rcupdate.h> #include <linux/utsname.h> @@ -544,8 +543,7 @@ static int rpc_new_file(struct dentry *parent, inode->i_fop = i_fop; rpc_inode_setowner(inode, private); d_make_persistent(dentry, inode); - fsnotify_create(dir, dentry); - simple_done_creating(dentry); + simple_end_creating(dentry); return 0; } @@ -569,8 +567,7 @@ static struct dentry *rpc_new_dir(struct dentry *parent, inode->i_ino = iunique(dir->i_sb, 100); inc_nlink(dir); d_make_persistent(dentry, inode); - fsnotify_mkdir(dir, dentry); - simple_done_creating(dentry); + simple_end_creating(dentry); return dentry; // borrowed } @@ -667,8 +664,7 @@ int rpc_mkpipe_dentry(struct dentry *parent, const char *name, rpc_inode_setowner(inode, private); pipe->dentry = dentry; // borrowed d_make_persistent(dentry, inode); - fsnotify_create(dir, dentry); - simple_done_creating(dentry); + simple_end_creating(dentry); return 0; failed: -- 2.53.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks 2026-03-02 18:37 ` [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks Amir Goldstein @ 2026-03-02 22:28 ` Chuck Lever 2026-03-03 10:05 ` Amir Goldstein 2026-03-03 15:01 ` Alice Ryhl 1 sibling, 1 reply; 16+ messages in thread From: Chuck Lever @ 2026-03-02 22:28 UTC (permalink / raw) To: Amir Goldstein, Jan Kara Cc: Christian Brauner, Al Viro, Jeff Layton, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel On 3/2/26 1:37 PM, Amir Goldstein wrote: > Add simple_end_creating() helper which combines fsnotify_create/mkdir() > hook and simple_done_creating(). > > Use the new helper to consolidate this pattern in several pseudo fs > which had open coded fsnotify_create/mkdir() hooks: > binderfs, debugfs, nfsctl, tracefs, rpc_pipefs. > > For those filesystems, the paired fsnotify_delete() hook is already > inside the library helper simple_recursive_removal(). > > Note that in debugfs_create_symlink(), the fsnotify hook was missing, > so the missing hook is fixed by this change. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > index e9acd2cd602cb..6e600d52b66d0 100644 > --- a/fs/nfsd/nfsctl.c > +++ b/fs/nfsd/nfsctl.c > @@ -17,7 +17,6 @@ > #include <linux/sunrpc/rpc_pipe_fs.h> > #include <linux/sunrpc/svc.h> > #include <linux/module.h> > -#include <linux/fsnotify.h> > #include <linux/nfslocalio.h> > > #include "idmap.h" > @@ -1146,8 +1145,7 @@ static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *nc > } > d_make_persistent(dentry, inode); > inc_nlink(dir); > - fsnotify_mkdir(dir, dentry); > - simple_done_creating(dentry); > + simple_end_creating(dentry); > return dentry; // borrowed > } > > @@ -1178,8 +1176,7 @@ static void _nfsd_symlink(struct dentry *parent, const char *name, > inode->i_size = strlen(content); > > d_make_persistent(dentry, inode); > - fsnotify_create(dir, dentry); > - simple_done_creating(dentry); > + simple_end_creating(dentry); > } > #else > static inline void _nfsd_symlink(struct dentry *parent, const char *name, > @@ -1219,7 +1216,6 @@ static int nfsdfs_create_files(struct dentry *root, > struct nfsdfs_client *ncl, > struct dentry **fdentries) > { > - struct inode *dir = d_inode(root); > struct dentry *dentry; > > for (int i = 0; files->name && files->name[0]; i++, files++) { > @@ -1236,10 +1232,9 @@ static int nfsdfs_create_files(struct dentry *root, > inode->i_fop = files->ops; > inode->i_private = ncl; > d_make_persistent(dentry, inode); > - fsnotify_create(dir, dentry); > if (fdentries) > fdentries[i] = dentry; // borrowed > - simple_done_creating(dentry); > + simple_end_creating(dentry); > } > return 0; > } For the NFSD hunks: Acked-by: Chuck Lever <chuck.lever@oracle.com> -- Chuck Lever ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks 2026-03-02 22:28 ` Chuck Lever @ 2026-03-03 10:05 ` Amir Goldstein 2026-03-03 10:28 ` NeilBrown 2026-03-03 14:24 ` Chuck Lever 0 siblings, 2 replies; 16+ messages in thread From: Amir Goldstein @ 2026-03-03 10:05 UTC (permalink / raw) To: Chuck Lever Cc: Jan Kara, Christian Brauner, Al Viro, Jeff Layton, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel, NeilBrown On Mon, Mar 2, 2026 at 11:28 PM Chuck Lever <chuck.lever@oracle.com> wrote: > > On 3/2/26 1:37 PM, Amir Goldstein wrote: > > Add simple_end_creating() helper which combines fsnotify_create/mkdir() > > hook and simple_done_creating(). > > > > Use the new helper to consolidate this pattern in several pseudo fs > > which had open coded fsnotify_create/mkdir() hooks: > > binderfs, debugfs, nfsctl, tracefs, rpc_pipefs. > > > > For those filesystems, the paired fsnotify_delete() hook is already > > inside the library helper simple_recursive_removal(). > > > > Note that in debugfs_create_symlink(), the fsnotify hook was missing, > > so the missing hook is fixed by this change. > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > > index e9acd2cd602cb..6e600d52b66d0 100644 > > --- a/fs/nfsd/nfsctl.c > > +++ b/fs/nfsd/nfsctl.c > > @@ -17,7 +17,6 @@ > > #include <linux/sunrpc/rpc_pipe_fs.h> > > #include <linux/sunrpc/svc.h> > > #include <linux/module.h> > > -#include <linux/fsnotify.h> > > #include <linux/nfslocalio.h> > > > > #include "idmap.h" > > @@ -1146,8 +1145,7 @@ static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *nc > > } > > d_make_persistent(dentry, inode); > > inc_nlink(dir); > > - fsnotify_mkdir(dir, dentry); > > - simple_done_creating(dentry); > > + simple_end_creating(dentry); > > return dentry; // borrowed > > } > > > > @@ -1178,8 +1176,7 @@ static void _nfsd_symlink(struct dentry *parent, const char *name, > > inode->i_size = strlen(content); > > > > d_make_persistent(dentry, inode); > > - fsnotify_create(dir, dentry); > > - simple_done_creating(dentry); > > + simple_end_creating(dentry); > > } > > #else > > static inline void _nfsd_symlink(struct dentry *parent, const char *name, > > @@ -1219,7 +1216,6 @@ static int nfsdfs_create_files(struct dentry *root, > > struct nfsdfs_client *ncl, > > struct dentry **fdentries) > > { > > - struct inode *dir = d_inode(root); > > struct dentry *dentry; > > > > for (int i = 0; files->name && files->name[0]; i++, files++) { > > @@ -1236,10 +1232,9 @@ static int nfsdfs_create_files(struct dentry *root, > > inode->i_fop = files->ops; > > inode->i_private = ncl; > > d_make_persistent(dentry, inode); > > - fsnotify_create(dir, dentry); > > if (fdentries) > > fdentries[i] = dentry; // borrowed > > - simple_done_creating(dentry); > > + simple_end_creating(dentry); > > } > > return 0; > > } > > For the NFSD hunks: > > Acked-by: Chuck Lever <chuck.lever@oracle.com> FWIW, you are technically also CCed for the sunrpc hunk ;) BTW, forgot to CC Neil and mention this patch: https://lore.kernel.org/linux-fsdevel/20260224222542.3458677-5-neilb@ownmail.net/ Since simple_done_creating() starts using end_creating() so should simple_end_creating(). I will change that in v2 after waiting for more feedback on v1. I don't want to get into naming discussion - I will just say that I wanted to avoid renaming simple_done_creating() to avoid unneeded churn. Thanks, Amir. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks 2026-03-03 10:05 ` Amir Goldstein @ 2026-03-03 10:28 ` NeilBrown 2026-03-03 11:27 ` Amir Goldstein 2026-03-03 14:24 ` Chuck Lever 1 sibling, 1 reply; 16+ messages in thread From: NeilBrown @ 2026-03-03 10:28 UTC (permalink / raw) To: Amir Goldstein Cc: Chuck Lever, Jan Kara, Christian Brauner, Al Viro, Jeff Layton, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel On Tue, 03 Mar 2026, Amir Goldstein wrote: > On Mon, Mar 2, 2026 at 11:28 PM Chuck Lever <chuck.lever@oracle.com> wrote: > > > > On 3/2/26 1:37 PM, Amir Goldstein wrote: > > > Add simple_end_creating() helper which combines fsnotify_create/mkdir() > > > hook and simple_done_creating(). > > > > > > Use the new helper to consolidate this pattern in several pseudo fs > > > which had open coded fsnotify_create/mkdir() hooks: > > > binderfs, debugfs, nfsctl, tracefs, rpc_pipefs. > > > > > > For those filesystems, the paired fsnotify_delete() hook is already > > > inside the library helper simple_recursive_removal(). > > > > > > Note that in debugfs_create_symlink(), the fsnotify hook was missing, > > > so the missing hook is fixed by this change. > > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > > > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > > > index e9acd2cd602cb..6e600d52b66d0 100644 > > > --- a/fs/nfsd/nfsctl.c > > > +++ b/fs/nfsd/nfsctl.c > > > @@ -17,7 +17,6 @@ > > > #include <linux/sunrpc/rpc_pipe_fs.h> > > > #include <linux/sunrpc/svc.h> > > > #include <linux/module.h> > > > -#include <linux/fsnotify.h> > > > #include <linux/nfslocalio.h> > > > > > > #include "idmap.h" > > > @@ -1146,8 +1145,7 @@ static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *nc > > > } > > > d_make_persistent(dentry, inode); > > > inc_nlink(dir); > > > - fsnotify_mkdir(dir, dentry); > > > - simple_done_creating(dentry); > > > + simple_end_creating(dentry); > > > return dentry; // borrowed > > > } > > > > > > @@ -1178,8 +1176,7 @@ static void _nfsd_symlink(struct dentry *parent, const char *name, > > > inode->i_size = strlen(content); > > > > > > d_make_persistent(dentry, inode); > > > - fsnotify_create(dir, dentry); > > > - simple_done_creating(dentry); > > > + simple_end_creating(dentry); > > > } > > > #else > > > static inline void _nfsd_symlink(struct dentry *parent, const char *name, > > > @@ -1219,7 +1216,6 @@ static int nfsdfs_create_files(struct dentry *root, > > > struct nfsdfs_client *ncl, > > > struct dentry **fdentries) > > > { > > > - struct inode *dir = d_inode(root); > > > struct dentry *dentry; > > > > > > for (int i = 0; files->name && files->name[0]; i++, files++) { > > > @@ -1236,10 +1232,9 @@ static int nfsdfs_create_files(struct dentry *root, > > > inode->i_fop = files->ops; > > > inode->i_private = ncl; > > > d_make_persistent(dentry, inode); > > > - fsnotify_create(dir, dentry); > > > if (fdentries) > > > fdentries[i] = dentry; // borrowed > > > - simple_done_creating(dentry); > > > + simple_end_creating(dentry); > > > } > > > return 0; > > > } > > > > For the NFSD hunks: > > > > Acked-by: Chuck Lever <chuck.lever@oracle.com> > > FWIW, you are technically also CCed for the sunrpc hunk ;) > > BTW, forgot to CC Neil and mention this patch: > https://lore.kernel.org/linux-fsdevel/20260224222542.3458677-5-neilb@ownmail.net/ > > Since simple_done_creating() starts using end_creating() > so should simple_end_creating(). > I will change that in v2 after waiting for more feedback on v1. > > I don't want to get into naming discussion - I will just say that I wanted > to avoid renaming simple_done_creating() to avoid unneeded churn. > > Thanks, > Amir. > Thanks for the Cc. Would there be a problem with doing the fs-notify for *every* caller of simple_done_creating? It does make sense to include the notify in the common code, but having two different interfaces that only differ in the notification (and don't contain some form of "notify" in their name) does seem like a recipe for confusion. Thanks, NeilBrown ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks 2026-03-03 10:28 ` NeilBrown @ 2026-03-03 11:27 ` Amir Goldstein 2026-03-05 1:22 ` NeilBrown 0 siblings, 1 reply; 16+ messages in thread From: Amir Goldstein @ 2026-03-03 11:27 UTC (permalink / raw) To: NeilBrown Cc: Chuck Lever, Jan Kara, Christian Brauner, Al Viro, Jeff Layton, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel On Tue, Mar 3, 2026 at 11:28 AM NeilBrown <neilb@ownmail.net> wrote: > > On Tue, 03 Mar 2026, Amir Goldstein wrote: > > On Mon, Mar 2, 2026 at 11:28 PM Chuck Lever <chuck.lever@oracle.com> wrote: > > > > > > On 3/2/26 1:37 PM, Amir Goldstein wrote: > > > > Add simple_end_creating() helper which combines fsnotify_create/mkdir() > > > > hook and simple_done_creating(). > > > > > > > > Use the new helper to consolidate this pattern in several pseudo fs > > > > which had open coded fsnotify_create/mkdir() hooks: > > > > binderfs, debugfs, nfsctl, tracefs, rpc_pipefs. > > > > > > > > For those filesystems, the paired fsnotify_delete() hook is already > > > > inside the library helper simple_recursive_removal(). > > > > > > > > Note that in debugfs_create_symlink(), the fsnotify hook was missing, > > > > so the missing hook is fixed by this change. > > > > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > > > > > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > > > > index e9acd2cd602cb..6e600d52b66d0 100644 > > > > --- a/fs/nfsd/nfsctl.c > > > > +++ b/fs/nfsd/nfsctl.c > > > > @@ -17,7 +17,6 @@ > > > > #include <linux/sunrpc/rpc_pipe_fs.h> > > > > #include <linux/sunrpc/svc.h> > > > > #include <linux/module.h> > > > > -#include <linux/fsnotify.h> > > > > #include <linux/nfslocalio.h> > > > > > > > > #include "idmap.h" > > > > @@ -1146,8 +1145,7 @@ static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *nc > > > > } > > > > d_make_persistent(dentry, inode); > > > > inc_nlink(dir); > > > > - fsnotify_mkdir(dir, dentry); > > > > - simple_done_creating(dentry); > > > > + simple_end_creating(dentry); > > > > return dentry; // borrowed > > > > } > > > > > > > > @@ -1178,8 +1176,7 @@ static void _nfsd_symlink(struct dentry *parent, const char *name, > > > > inode->i_size = strlen(content); > > > > > > > > d_make_persistent(dentry, inode); > > > > - fsnotify_create(dir, dentry); > > > > - simple_done_creating(dentry); > > > > + simple_end_creating(dentry); > > > > } > > > > #else > > > > static inline void _nfsd_symlink(struct dentry *parent, const char *name, > > > > @@ -1219,7 +1216,6 @@ static int nfsdfs_create_files(struct dentry *root, > > > > struct nfsdfs_client *ncl, > > > > struct dentry **fdentries) > > > > { > > > > - struct inode *dir = d_inode(root); > > > > struct dentry *dentry; > > > > > > > > for (int i = 0; files->name && files->name[0]; i++, files++) { > > > > @@ -1236,10 +1232,9 @@ static int nfsdfs_create_files(struct dentry *root, > > > > inode->i_fop = files->ops; > > > > inode->i_private = ncl; > > > > d_make_persistent(dentry, inode); > > > > - fsnotify_create(dir, dentry); > > > > if (fdentries) > > > > fdentries[i] = dentry; // borrowed > > > > - simple_done_creating(dentry); > > > > + simple_end_creating(dentry); > > > > } > > > > return 0; > > > > } > > > > > > For the NFSD hunks: > > > > > > Acked-by: Chuck Lever <chuck.lever@oracle.com> > > > > FWIW, you are technically also CCed for the sunrpc hunk ;) > > > > BTW, forgot to CC Neil and mention this patch: > > https://lore.kernel.org/linux-fsdevel/20260224222542.3458677-5-neilb@ownmail.net/ > > > > Since simple_done_creating() starts using end_creating() > > so should simple_end_creating(). > > I will change that in v2 after waiting for more feedback on v1. > > > > I don't want to get into naming discussion - I will just say that I wanted > > to avoid renaming simple_done_creating() to avoid unneeded churn. > > > > Thanks, > > Amir. > > > > Thanks for the Cc. > > Would there be a problem with doing the fs-notify for *every* caller of > simple_done_creating? 1. simple_done_creating() is also called for the failed case, where the fsnotify hook is not desired. I am not sure if failure is always equivalent to negative child dentry. 2. I assume you meant the simple_done_creating() calls in other fs that do not have fsnotify hooks in current code. This is a valid point. I am hesitant to add the FS_CREATE events for *all* the pseudo fs dentry creations. Specifically, I think we need not send the events for init/populate of fs. It's worth nothing that some of those fs do already send FS_DELETE events via simple_recursive_removal(). > It does make sense to include the notify in the common code, but having > two different interfaces that only differ in the notification (and don't > contain some form of "notify" in their name) does seem like a recipe for > confusion. Right. I could make it simple_end_creating_notify(). Now where does this sound familiar from? Oh yeah, my old fsnotify path hooks series [1] :) Cheers, Amir. [1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxi-UhF=6eaxhybvdBX-L5qYx_uEuu-eCiiUzJPvz2U8aw@mail.gmail.com/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks 2026-03-03 11:27 ` Amir Goldstein @ 2026-03-05 1:22 ` NeilBrown 0 siblings, 0 replies; 16+ messages in thread From: NeilBrown @ 2026-03-05 1:22 UTC (permalink / raw) To: Amir Goldstein Cc: Chuck Lever, Jan Kara, Christian Brauner, Al Viro, Jeff Layton, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel On Tue, 03 Mar 2026, Amir Goldstein wrote: > On Tue, Mar 3, 2026 at 11:28 AM NeilBrown <neilb@ownmail.net> wrote: > > > > On Tue, 03 Mar 2026, Amir Goldstein wrote: > > > On Mon, Mar 2, 2026 at 11:28 PM Chuck Lever <chuck.lever@oracle.com> wrote: > > > > > > > > On 3/2/26 1:37 PM, Amir Goldstein wrote: > > > > > Add simple_end_creating() helper which combines fsnotify_create/mkdir() > > > > > hook and simple_done_creating(). > > > > > > > > > > Use the new helper to consolidate this pattern in several pseudo fs > > > > > which had open coded fsnotify_create/mkdir() hooks: > > > > > binderfs, debugfs, nfsctl, tracefs, rpc_pipefs. > > > > > > > > > > For those filesystems, the paired fsnotify_delete() hook is already > > > > > inside the library helper simple_recursive_removal(). > > > > > > > > > > Note that in debugfs_create_symlink(), the fsnotify hook was missing, > > > > > so the missing hook is fixed by this change. > > > > > > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > > > > > > > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > > > > > index e9acd2cd602cb..6e600d52b66d0 100644 > > > > > --- a/fs/nfsd/nfsctl.c > > > > > +++ b/fs/nfsd/nfsctl.c > > > > > @@ -17,7 +17,6 @@ > > > > > #include <linux/sunrpc/rpc_pipe_fs.h> > > > > > #include <linux/sunrpc/svc.h> > > > > > #include <linux/module.h> > > > > > -#include <linux/fsnotify.h> > > > > > #include <linux/nfslocalio.h> > > > > > > > > > > #include "idmap.h" > > > > > @@ -1146,8 +1145,7 @@ static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *nc > > > > > } > > > > > d_make_persistent(dentry, inode); > > > > > inc_nlink(dir); > > > > > - fsnotify_mkdir(dir, dentry); > > > > > - simple_done_creating(dentry); > > > > > + simple_end_creating(dentry); > > > > > return dentry; // borrowed > > > > > } > > > > > > > > > > @@ -1178,8 +1176,7 @@ static void _nfsd_symlink(struct dentry *parent, const char *name, > > > > > inode->i_size = strlen(content); > > > > > > > > > > d_make_persistent(dentry, inode); > > > > > - fsnotify_create(dir, dentry); > > > > > - simple_done_creating(dentry); > > > > > + simple_end_creating(dentry); > > > > > } > > > > > #else > > > > > static inline void _nfsd_symlink(struct dentry *parent, const char *name, > > > > > @@ -1219,7 +1216,6 @@ static int nfsdfs_create_files(struct dentry *root, > > > > > struct nfsdfs_client *ncl, > > > > > struct dentry **fdentries) > > > > > { > > > > > - struct inode *dir = d_inode(root); > > > > > struct dentry *dentry; > > > > > > > > > > for (int i = 0; files->name && files->name[0]; i++, files++) { > > > > > @@ -1236,10 +1232,9 @@ static int nfsdfs_create_files(struct dentry *root, > > > > > inode->i_fop = files->ops; > > > > > inode->i_private = ncl; > > > > > d_make_persistent(dentry, inode); > > > > > - fsnotify_create(dir, dentry); > > > > > if (fdentries) > > > > > fdentries[i] = dentry; // borrowed > > > > > - simple_done_creating(dentry); > > > > > + simple_end_creating(dentry); > > > > > } > > > > > return 0; > > > > > } > > > > > > > > For the NFSD hunks: > > > > > > > > Acked-by: Chuck Lever <chuck.lever@oracle.com> > > > > > > FWIW, you are technically also CCed for the sunrpc hunk ;) > > > > > > BTW, forgot to CC Neil and mention this patch: > > > https://lore.kernel.org/linux-fsdevel/20260224222542.3458677-5-neilb@ownmail.net/ > > > > > > Since simple_done_creating() starts using end_creating() > > > so should simple_end_creating(). > > > I will change that in v2 after waiting for more feedback on v1. > > > > > > I don't want to get into naming discussion - I will just say that I wanted > > > to avoid renaming simple_done_creating() to avoid unneeded churn. > > > > > > Thanks, > > > Amir. > > > > > > > Thanks for the Cc. > > > > Would there be a problem with doing the fs-notify for *every* caller of > > simple_done_creating? > > 1. simple_done_creating() is also called for the failed case, where the > fsnotify hook is not desired. I am not sure if failure is always equivalent > to negative child dentry. If the dentry is still hashed and positive, then it is hard to see that could represent a failure. If it is unhashed or negative, then ... it could possibly be a success as some filesystems create the object but leave it to a subsequent ->lookup to add it to the dcache. I doubt such filesystems would be using simple_* though. > > 2. I assume you meant the simple_done_creating() calls in other fs that > do not have fsnotify hooks in current code. This is a valid point. > I am hesitant to add the FS_CREATE events for *all* the pseudo fs > dentry creations. > Specifically, I think we need not send the events for init/populate of fs. fsnotify_create hooks are effectively no-op if no "marks" have be registered on sb or parent inode. During init/populate there would be no marks. I think filesystems should have a good reason for opting out of fsnotify, and it should be harder to opt-out than to opt-in. > It's worth nothing that some of those fs do already send FS_DELETE > events via simple_recursive_removal(). > \x16 > > It does make sense to include the notify in the common code, but having > > two different interfaces that only differ in the notification (and don't > > contain some form of "notify" in their name) does seem like a recipe for > > confusion. > > Right. I could make it simple_end_creating_notify(). > > Now where does this sound familiar from? > > Oh yeah, my old fsnotify path hooks series [1] :) Everything old is new again :-) Thanks, NeilBrown > > Cheers, > Amir. > > [1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxi-UhF=6eaxhybvdBX-L5qYx_uEuu-eCiiUzJPvz2U8aw@mail.gmail.com/ > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks 2026-03-03 10:05 ` Amir Goldstein 2026-03-03 10:28 ` NeilBrown @ 2026-03-03 14:24 ` Chuck Lever 1 sibling, 0 replies; 16+ messages in thread From: Chuck Lever @ 2026-03-03 14:24 UTC (permalink / raw) To: Amir Goldstein Cc: Jan Kara, Christian Brauner, Al Viro, Jeff Layton, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel, NeilBrown On 3/3/26 5:05 AM, Amir Goldstein wrote: > On Mon, Mar 2, 2026 at 11:28 PM Chuck Lever <chuck.lever@oracle.com> wrote: >> >> On 3/2/26 1:37 PM, Amir Goldstein wrote: >>> Add simple_end_creating() helper which combines fsnotify_create/mkdir() >>> hook and simple_done_creating(). >>> >>> Use the new helper to consolidate this pattern in several pseudo fs >>> which had open coded fsnotify_create/mkdir() hooks: >>> binderfs, debugfs, nfsctl, tracefs, rpc_pipefs. >>> >>> For those filesystems, the paired fsnotify_delete() hook is already >>> inside the library helper simple_recursive_removal(). >>> >>> Note that in debugfs_create_symlink(), the fsnotify hook was missing, >>> so the missing hook is fixed by this change. >>> >>> Signed-off-by: Amir Goldstein <amir73il@gmail.com> >> >>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c >>> index e9acd2cd602cb..6e600d52b66d0 100644 >>> --- a/fs/nfsd/nfsctl.c >>> +++ b/fs/nfsd/nfsctl.c >>> @@ -17,7 +17,6 @@ >>> #include <linux/sunrpc/rpc_pipe_fs.h> >>> #include <linux/sunrpc/svc.h> >>> #include <linux/module.h> >>> -#include <linux/fsnotify.h> >>> #include <linux/nfslocalio.h> >>> >>> #include "idmap.h" >>> @@ -1146,8 +1145,7 @@ static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *nc >>> } >>> d_make_persistent(dentry, inode); >>> inc_nlink(dir); >>> - fsnotify_mkdir(dir, dentry); >>> - simple_done_creating(dentry); >>> + simple_end_creating(dentry); >>> return dentry; // borrowed >>> } >>> >>> @@ -1178,8 +1176,7 @@ static void _nfsd_symlink(struct dentry *parent, const char *name, >>> inode->i_size = strlen(content); >>> >>> d_make_persistent(dentry, inode); >>> - fsnotify_create(dir, dentry); >>> - simple_done_creating(dentry); >>> + simple_end_creating(dentry); >>> } >>> #else >>> static inline void _nfsd_symlink(struct dentry *parent, const char *name, >>> @@ -1219,7 +1216,6 @@ static int nfsdfs_create_files(struct dentry *root, >>> struct nfsdfs_client *ncl, >>> struct dentry **fdentries) >>> { >>> - struct inode *dir = d_inode(root); >>> struct dentry *dentry; >>> >>> for (int i = 0; files->name && files->name[0]; i++, files++) { >>> @@ -1236,10 +1232,9 @@ static int nfsdfs_create_files(struct dentry *root, >>> inode->i_fop = files->ops; >>> inode->i_private = ncl; >>> d_make_persistent(dentry, inode); >>> - fsnotify_create(dir, dentry); >>> if (fdentries) >>> fdentries[i] = dentry; // borrowed >>> - simple_done_creating(dentry); >>> + simple_end_creating(dentry); >>> } >>> return 0; >>> } >> >> For the NFSD hunks: >> >> Acked-by: Chuck Lever <chuck.lever@oracle.com> > > FWIW, you are technically also CCed for the sunrpc hunk ;) rpc_pipe.c is client-side. On the next round, Cc: Trond, Anna, and linux-nfs@ . -- Chuck Lever ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks 2026-03-02 18:37 ` [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks Amir Goldstein 2026-03-02 22:28 ` Chuck Lever @ 2026-03-03 15:01 ` Alice Ryhl 1 sibling, 0 replies; 16+ messages in thread From: Alice Ryhl @ 2026-03-03 15:01 UTC (permalink / raw) To: Amir Goldstein Cc: Jan Kara, Christian Brauner, Al Viro, Chuck Lever, Jeff Layton, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel On Mon, Mar 02, 2026 at 07:37:41PM +0100, Amir Goldstein wrote: > Add simple_end_creating() helper which combines fsnotify_create/mkdir() > hook and simple_done_creating(). > > Use the new helper to consolidate this pattern in several pseudo fs > which had open coded fsnotify_create/mkdir() hooks: > binderfs, debugfs, nfsctl, tracefs, rpc_pipefs. > > For those filesystems, the paired fsnotify_delete() hook is already > inside the library helper simple_recursive_removal(). > > Note that in debugfs_create_symlink(), the fsnotify hook was missing, > so the missing hook is fixed by this change. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> Acked-by: Alice Ryhl <aliceryhl@google.com> # binderfs + rust_binderfs ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] fsnotify hooks consolidation 2026-03-02 18:37 [PATCH 0/2] fsnotify hooks consolidation Amir Goldstein 2026-03-02 18:37 ` [PATCH 1/2] fsnotify: make fsnotify_create() agnostic to file/dir Amir Goldstein 2026-03-02 18:37 ` [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks Amir Goldstein @ 2026-03-03 12:45 ` Jeff Layton 2026-03-03 13:18 ` Amir Goldstein 2 siblings, 1 reply; 16+ messages in thread From: Jeff Layton @ 2026-03-03 12:45 UTC (permalink / raw) To: Amir Goldstein, Jan Kara Cc: Christian Brauner, Al Viro, Chuck Lever, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel On Mon, 2026-03-02 at 19:37 +0100, Amir Goldstein wrote: > Jan, > > I've found this opportunity to reduce the amount of sprinkled > fsnotify hooks. > > There are still a few fsnotify hooks sprinkled in some pseudo fs, > but all those removed by this series are obvious boiler plate code > and for most of these fs, this removes all of the custom hooks. > > I could send a series to convert each fs with its own patch, > but to me that seems a bit unnecessary. > > WDYT? > Amir. > > Amir Goldstein (2): > fsnotify: make fsnotify_create() agnostic to file/dir > fs: use simple_end_creating helper to consolidate fsnotify hooks > > drivers/android/binder/rust_binderfs.c | 11 +++-------- > drivers/android/binderfs.c | 10 +++------- > fs/debugfs/inode.c | 5 +---- > fs/libfs.c | 14 ++++++++++++++ > fs/nfsd/nfsctl.c | 11 +++-------- > fs/tracefs/event_inode.c | 2 -- > fs/tracefs/inode.c | 5 +---- > include/linux/fs.h | 1 + > include/linux/fsnotify.h | 8 +++++++- > net/sunrpc/rpc_pipe.c | 10 +++------- > 10 files changed, 36 insertions(+), 41 deletions(-) Conceptually, this all seems fine. My only gripe is that there is nothing mnemonic about the names simple_done_creating() and simple_end_creating(). Since the only difference are the notification bits, maybe simple_done_creating_notify() is a better name? Reviewed-by: Jeff Layton <jlayton@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] fsnotify hooks consolidation 2026-03-03 12:45 ` [PATCH 0/2] fsnotify hooks consolidation Jeff Layton @ 2026-03-03 13:18 ` Amir Goldstein 2026-03-03 15:00 ` Christoph Hellwig 0 siblings, 1 reply; 16+ messages in thread From: Amir Goldstein @ 2026-03-03 13:18 UTC (permalink / raw) To: Jeff Layton Cc: Jan Kara, Christian Brauner, Al Viro, Chuck Lever, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel On Tue, Mar 3, 2026 at 1:45 PM Jeff Layton <jlayton@kernel.org> wrote: > > On Mon, 2026-03-02 at 19:37 +0100, Amir Goldstein wrote: > > Jan, > > > > I've found this opportunity to reduce the amount of sprinkled > > fsnotify hooks. > > > > There are still a few fsnotify hooks sprinkled in some pseudo fs, > > but all those removed by this series are obvious boiler plate code > > and for most of these fs, this removes all of the custom hooks. > > > > I could send a series to convert each fs with its own patch, > > but to me that seems a bit unnecessary. > > > > WDYT? > > Amir. > > > > Amir Goldstein (2): > > fsnotify: make fsnotify_create() agnostic to file/dir > > fs: use simple_end_creating helper to consolidate fsnotify hooks > > > > drivers/android/binder/rust_binderfs.c | 11 +++-------- > > drivers/android/binderfs.c | 10 +++------- > > fs/debugfs/inode.c | 5 +---- > > fs/libfs.c | 14 ++++++++++++++ > > fs/nfsd/nfsctl.c | 11 +++-------- > > fs/tracefs/event_inode.c | 2 -- > > fs/tracefs/inode.c | 5 +---- > > include/linux/fs.h | 1 + > > include/linux/fsnotify.h | 8 +++++++- > > net/sunrpc/rpc_pipe.c | 10 +++------- > > 10 files changed, 36 insertions(+), 41 deletions(-) > > Conceptually, this all seems fine. > > My only gripe is that there is nothing mnemonic about the names > simple_done_creating() and simple_end_creating(). Since the only > difference are the notification bits, maybe > simple_done_creating_notify() is a better name? I will go with simple_end_creating_notify() because what it does is: void simple_end_creating_notify(struct dentry *child) { fsnotify_create(child->d_parent->d_inode, child); end_creating(child); } > > Reviewed-by: Jeff Layton <jlayton@kernel.org> Thanks, Amir. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] fsnotify hooks consolidation 2026-03-03 13:18 ` Amir Goldstein @ 2026-03-03 15:00 ` Christoph Hellwig 2026-03-03 15:05 ` Amir Goldstein 0 siblings, 1 reply; 16+ messages in thread From: Christoph Hellwig @ 2026-03-03 15:00 UTC (permalink / raw) To: Amir Goldstein Cc: Jeff Layton, Jan Kara, Christian Brauner, Al Viro, Chuck Lever, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel On Tue, Mar 03, 2026 at 02:18:47PM +0100, Amir Goldstein wrote: > > simple_done_creating_notify() is a better name? > > I will go with simple_end_creating_notify() because what it does is: Shouldn't the notify case be the default one and the nonotify one stand out with a prefix like _nonotify? I.e., steer people to the more useful one by default. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] fsnotify hooks consolidation 2026-03-03 15:00 ` Christoph Hellwig @ 2026-03-03 15:05 ` Amir Goldstein 2026-03-03 15:07 ` Christoph Hellwig 0 siblings, 1 reply; 16+ messages in thread From: Amir Goldstein @ 2026-03-03 15:05 UTC (permalink / raw) To: Christoph Hellwig Cc: Jeff Layton, Jan Kara, Christian Brauner, Al Viro, Chuck Lever, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel On Tue, Mar 3, 2026 at 4:00 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Tue, Mar 03, 2026 at 02:18:47PM +0100, Amir Goldstein wrote: > > > simple_done_creating_notify() is a better name? > > > > I will go with simple_end_creating_notify() because what it does is: > > Shouldn't the notify case be the default one and the nonotify one > stand out with a prefix like _nonotify? I.e., steer people to the > more useful one by default. > It may sound like that, but in reality, it is quite hard to differentiate in pseduo fs code between creations that are auto initialized at mount time and creations that are user triggerred. Only the user triggerred ones should be notified and also we only have a handful of pseduo fs who opted in so far, so I think in this case staying with opt in is the right way, but open to hear what folks think. Thanks, Amir. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] fsnotify hooks consolidation 2026-03-03 15:05 ` Amir Goldstein @ 2026-03-03 15:07 ` Christoph Hellwig 2026-03-03 15:17 ` Amir Goldstein 0 siblings, 1 reply; 16+ messages in thread From: Christoph Hellwig @ 2026-03-03 15:07 UTC (permalink / raw) To: Amir Goldstein Cc: Christoph Hellwig, Jeff Layton, Jan Kara, Christian Brauner, Al Viro, Chuck Lever, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel On Tue, Mar 03, 2026 at 04:05:09PM +0100, Amir Goldstein wrote: > It may sound like that, but in reality, it is quite hard to differentiate > in pseduo fs code between creations that are auto initialized > at mount time and creations that are user triggerred. > > Only the user triggerred ones should be notified and also we only > have a handful of pseduo fs who opted in so far, so I think in this case > staying with opt in is the right way, but open to hear what folks think. Then maybe encode that in the name with _user and _kernel postfix, and also document it very well? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] fsnotify hooks consolidation 2026-03-03 15:07 ` Christoph Hellwig @ 2026-03-03 15:17 ` Amir Goldstein 0 siblings, 0 replies; 16+ messages in thread From: Amir Goldstein @ 2026-03-03 15:17 UTC (permalink / raw) To: Christoph Hellwig Cc: Jeff Layton, Jan Kara, Christian Brauner, Al Viro, Chuck Lever, Greg Kroah-Hartman, Steven Rostedt, linux-fsdevel On Tue, Mar 3, 2026 at 4:07 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Tue, Mar 03, 2026 at 04:05:09PM +0100, Amir Goldstein wrote: > > It may sound like that, but in reality, it is quite hard to differentiate > > in pseduo fs code between creations that are auto initialized > > at mount time and creations that are user triggerred. > > > > Only the user triggerred ones should be notified and also we only > > have a handful of pseduo fs who opted in so far, so I think in this case > > staying with opt in is the right way, but open to hear what folks think. > > Then maybe encode that in the name with _user and _kernel postfix, and > also document it very well? > Sure but _user vs. _kernel is maybe not the best terminology it is alreayd the case that dentries could be created from _user API via vfs syscalls. The code changed by this patch is specifically code that creates entries not via vfs syscalls, so vfs code refers to these changes and _kernel changes. However, they are trigerred by user writing something to some file etc. Calling the helper by what it does keeps things simple - if author wants a notification, author should use this helper. I honestly don't know why this fs opted for notifications and other fs did not - I assume it's just based on actual real world use cases, so I'd rather not change those decisions with this mechanical cleanup task or with an "alluring" helper name. Thanks, Amir. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-03-05 1:22 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-02 18:37 [PATCH 0/2] fsnotify hooks consolidation Amir Goldstein 2026-03-02 18:37 ` [PATCH 1/2] fsnotify: make fsnotify_create() agnostic to file/dir Amir Goldstein 2026-03-02 18:37 ` [PATCH 2/2] fs: use simple_end_creating helper to consolidate fsnotify hooks Amir Goldstein 2026-03-02 22:28 ` Chuck Lever 2026-03-03 10:05 ` Amir Goldstein 2026-03-03 10:28 ` NeilBrown 2026-03-03 11:27 ` Amir Goldstein 2026-03-05 1:22 ` NeilBrown 2026-03-03 14:24 ` Chuck Lever 2026-03-03 15:01 ` Alice Ryhl 2026-03-03 12:45 ` [PATCH 0/2] fsnotify hooks consolidation Jeff Layton 2026-03-03 13:18 ` Amir Goldstein 2026-03-03 15:00 ` Christoph Hellwig 2026-03-03 15:05 ` Amir Goldstein 2026-03-03 15:07 ` Christoph Hellwig 2026-03-03 15:17 ` Amir Goldstein
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox