* [PATCH 02/17] new helper: simple_start_creating()
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 18:31 ` Jeff Layton
2025-06-13 7:34 ` [PATCH 03/17] rpc_pipe: clean failure exits in fill_super Al Viro
` (15 subsequent siblings)
16 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
Set the things up for kernel-initiated creation of object in
a tree-in-dcache filesystem. With respect to locking it's
an equivalent of filename_create() - we either get a negative
dentry with locked parent, or ERR_PTR() and no locks taken.
tracefs and debugfs had that open-coded as part of their
object creation machinery; switched to calling new helper.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/debugfs/inode.c | 21 ++-------------------
fs/libfs.c | 25 +++++++++++++++++++++++++
fs/tracefs/inode.c | 15 ++-------------
include/linux/fs.h | 1 +
4 files changed, 30 insertions(+), 32 deletions(-)
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 30c4944e1862..08638e39bd12 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -384,26 +384,9 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
if (!parent)
parent = debugfs_mount->mnt_root;
- inode_lock(d_inode(parent));
- if (unlikely(IS_DEADDIR(d_inode(parent))))
- dentry = ERR_PTR(-ENOENT);
- else
- dentry = lookup_noperm(&QSTR(name), parent);
- if (!IS_ERR(dentry) && d_really_is_positive(dentry)) {
- if (d_is_dir(dentry))
- pr_err("Directory '%s' with parent '%s' already present!\n",
- name, parent->d_name.name);
- else
- pr_err("File '%s' in directory '%s' already present!\n",
- name, parent->d_name.name);
- dput(dentry);
- dentry = ERR_PTR(-EEXIST);
- }
-
- if (IS_ERR(dentry)) {
- inode_unlock(d_inode(parent));
+ dentry = simple_start_creating(parent, name);
+ if (IS_ERR(dentry))
simple_release_fs(&debugfs_mount, &debugfs_mount_count);
- }
return dentry;
}
diff --git a/fs/libfs.c b/fs/libfs.c
index 42e226af6095..833ad5ed10f5 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -2260,3 +2260,28 @@ void stashed_dentry_prune(struct dentry *dentry)
*/
cmpxchg(stashed, dentry, NULL);
}
+
+/* parent must be held exclusive */
+struct dentry *simple_start_creating(struct dentry *parent, const char *name)
+{
+ struct dentry *dentry;
+ struct inode *dir = d_inode(parent);
+
+ inode_lock(dir);
+ if (unlikely(IS_DEADDIR(dir))) {
+ inode_unlock(dir);
+ return ERR_PTR(-ENOENT);
+ }
+ dentry = lookup_noperm(&QSTR(name), parent);
+ if (IS_ERR(dentry)) {
+ inode_unlock(dir);
+ return dentry;
+ }
+ if (dentry->d_inode) {
+ dput(dentry);
+ inode_unlock(dir);
+ return ERR_PTR(-EEXIST);
+ }
+ return dentry;
+}
+EXPORT_SYMBOL(simple_start_creating);
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index a3fd3cc591bd..4e5d091e9263 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -551,20 +551,9 @@ struct dentry *tracefs_start_creating(const char *name, struct dentry *parent)
if (!parent)
parent = tracefs_mount->mnt_root;
- inode_lock(d_inode(parent));
- if (unlikely(IS_DEADDIR(d_inode(parent))))
- dentry = ERR_PTR(-ENOENT);
- else
- dentry = lookup_noperm(&QSTR(name), parent);
- if (!IS_ERR(dentry) && d_inode(dentry)) {
- dput(dentry);
- dentry = ERR_PTR(-EEXIST);
- }
-
- if (IS_ERR(dentry)) {
- inode_unlock(d_inode(parent));
+ dentry = simple_start_creating(parent, name);
+ if (IS_ERR(dentry))
simple_release_fs(&tracefs_mount, &tracefs_mount_count);
- }
return dentry;
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 96c7925a6551..9f75f8836bbd 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3619,6 +3619,7 @@ extern int simple_fill_super(struct super_block *, unsigned long,
const struct tree_descr *);
extern int simple_pin_fs(struct file_system_type *, struct vfsmount **mount, int *count);
extern void simple_release_fs(struct vfsmount **mount, int *count);
+struct dentry *simple_start_creating(struct dentry *, const char *);
extern ssize_t simple_read_from_buffer(void __user *to, size_t count,
loff_t *ppos, const void *from, size_t available);
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 02/17] new helper: simple_start_creating()
2025-06-13 7:34 ` [PATCH 02/17] new helper: simple_start_creating() Al Viro
@ 2025-06-13 18:31 ` Jeff Layton
2025-06-13 22:36 ` Al Viro
0 siblings, 1 reply; 25+ messages in thread
From: Jeff Layton @ 2025-06-13 18:31 UTC (permalink / raw)
To: Al Viro, linux-fsdevel; +Cc: chuck.lever, linux-nfs, neil, torvalds, trondmy
On Fri, 2025-06-13 at 08:34 +0100, Al Viro wrote:
> Set the things up for kernel-initiated creation of object in
> a tree-in-dcache filesystem. With respect to locking it's
> an equivalent of filename_create() - we either get a negative
> dentry with locked parent, or ERR_PTR() and no locks taken.
>
> tracefs and debugfs had that open-coded as part of their
> object creation machinery; switched to calling new helper.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> fs/debugfs/inode.c | 21 ++-------------------
> fs/libfs.c | 25 +++++++++++++++++++++++++
> fs/tracefs/inode.c | 15 ++-------------
> include/linux/fs.h | 1 +
> 4 files changed, 30 insertions(+), 32 deletions(-)
>
> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
> index 30c4944e1862..08638e39bd12 100644
> --- a/fs/debugfs/inode.c
> +++ b/fs/debugfs/inode.c
> @@ -384,26 +384,9 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
> if (!parent)
> parent = debugfs_mount->mnt_root;
>
> - inode_lock(d_inode(parent));
> - if (unlikely(IS_DEADDIR(d_inode(parent))))
> - dentry = ERR_PTR(-ENOENT);
> - else
> - dentry = lookup_noperm(&QSTR(name), parent);
> - if (!IS_ERR(dentry) && d_really_is_positive(dentry)) {
> - if (d_is_dir(dentry))
> - pr_err("Directory '%s' with parent '%s' already present!\n",
> - name, parent->d_name.name);
> - else
> - pr_err("File '%s' in directory '%s' already present!\n",
> - name, parent->d_name.name);
Any chance we could keep a pr_err() for this case? I was doing some
debugfs work recently, and found it helpful.
> - dput(dentry);
> - dentry = ERR_PTR(-EEXIST);
> - }
> -
> - if (IS_ERR(dentry)) {
> - inode_unlock(d_inode(parent));
> + dentry = simple_start_creating(parent, name);
> + if (IS_ERR(dentry))
> simple_release_fs(&debugfs_mount, &debugfs_mount_count);
> - }
>
> return dentry;
> }
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 42e226af6095..833ad5ed10f5 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -2260,3 +2260,28 @@ void stashed_dentry_prune(struct dentry *dentry)
> */
> cmpxchg(stashed, dentry, NULL);
> }
> +
> +/* parent must be held exclusive */
> +struct dentry *simple_start_creating(struct dentry *parent, const char *name)
> +{
> + struct dentry *dentry;
> + struct inode *dir = d_inode(parent);
> +
> + inode_lock(dir);
> + if (unlikely(IS_DEADDIR(dir))) {
> + inode_unlock(dir);
> + return ERR_PTR(-ENOENT);
> + }
> + dentry = lookup_noperm(&QSTR(name), parent);
> + if (IS_ERR(dentry)) {
> + inode_unlock(dir);
> + return dentry;
> + }
> + if (dentry->d_inode) {
> + dput(dentry);
> + inode_unlock(dir);
> + return ERR_PTR(-EEXIST);
> + }
> + return dentry;
> +}
> +EXPORT_SYMBOL(simple_start_creating);
> diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
> index a3fd3cc591bd..4e5d091e9263 100644
> --- a/fs/tracefs/inode.c
> +++ b/fs/tracefs/inode.c
> @@ -551,20 +551,9 @@ struct dentry *tracefs_start_creating(const char *name, struct dentry *parent)
> if (!parent)
> parent = tracefs_mount->mnt_root;
>
> - inode_lock(d_inode(parent));
> - if (unlikely(IS_DEADDIR(d_inode(parent))))
> - dentry = ERR_PTR(-ENOENT);
> - else
> - dentry = lookup_noperm(&QSTR(name), parent);
> - if (!IS_ERR(dentry) && d_inode(dentry)) {
> - dput(dentry);
> - dentry = ERR_PTR(-EEXIST);
> - }
> -
> - if (IS_ERR(dentry)) {
> - inode_unlock(d_inode(parent));
> + dentry = simple_start_creating(parent, name);
> + if (IS_ERR(dentry))
> simple_release_fs(&tracefs_mount, &tracefs_mount_count);
> - }
>
> return dentry;
> }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 96c7925a6551..9f75f8836bbd 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3619,6 +3619,7 @@ extern int simple_fill_super(struct super_block *, unsigned long,
> const struct tree_descr *);
> extern int simple_pin_fs(struct file_system_type *, struct vfsmount **mount, int *count);
> extern void simple_release_fs(struct vfsmount **mount, int *count);
> +struct dentry *simple_start_creating(struct dentry *, const char *);
>
> extern ssize_t simple_read_from_buffer(void __user *to, size_t count,
> loff_t *ppos, const void *from, size_t available);
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 02/17] new helper: simple_start_creating()
2025-06-13 18:31 ` Jeff Layton
@ 2025-06-13 22:36 ` Al Viro
2025-06-13 23:46 ` Jeff Layton
0 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2025-06-13 22:36 UTC (permalink / raw)
To: Jeff Layton
Cc: linux-fsdevel, chuck.lever, linux-nfs, neil, torvalds, trondmy
On Fri, Jun 13, 2025 at 02:31:56PM -0400, Jeff Layton wrote:
> > - if (unlikely(IS_DEADDIR(d_inode(parent))))
> > - dentry = ERR_PTR(-ENOENT);
> > - else
> > - dentry = lookup_noperm(&QSTR(name), parent);
> > - if (!IS_ERR(dentry) && d_really_is_positive(dentry)) {
> > - if (d_is_dir(dentry))
> > - pr_err("Directory '%s' with parent '%s' already present!\n",
> > - name, parent->d_name.name);
> > - else
> > - pr_err("File '%s' in directory '%s' already present!\n",
> > - name, parent->d_name.name);
>
> Any chance we could keep a pr_err() for this case? I was doing some
> debugfs work recently, and found it helpful.
Umm... Not in simple_start_creating(), obviously, but...
Would something like
dentry = simple_start_creating(parent, name);
if (IS_ERR(dentry)) {
if (dentry == ERR_PTR(-EEXIST))
pr_err("'%s' already exists in '%pd'\n", name, parent);
simple_release_fs(&debugfs_mount, &debugfs_mount_count);
}
work for you?
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 02/17] new helper: simple_start_creating()
2025-06-13 22:36 ` Al Viro
@ 2025-06-13 23:46 ` Jeff Layton
0 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2025-06-13 23:46 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, chuck.lever, linux-nfs, neil, torvalds, trondmy
On Fri, 2025-06-13 at 23:36 +0100, Al Viro wrote:
> On Fri, Jun 13, 2025 at 02:31:56PM -0400, Jeff Layton wrote:
> > > - if (unlikely(IS_DEADDIR(d_inode(parent))))
> > > - dentry = ERR_PTR(-ENOENT);
> > > - else
> > > - dentry = lookup_noperm(&QSTR(name), parent);
> > > - if (!IS_ERR(dentry) && d_really_is_positive(dentry)) {
> > > - if (d_is_dir(dentry))
> > > - pr_err("Directory '%s' with parent '%s' already present!\n",
> > > - name, parent->d_name.name);
> > > - else
> > > - pr_err("File '%s' in directory '%s' already present!\n",
> > > - name, parent->d_name.name);
> >
> > Any chance we could keep a pr_err() for this case? I was doing some
> > debugfs work recently, and found it helpful.
>
> Umm... Not in simple_start_creating(), obviously, but...
> Would something like
> dentry = simple_start_creating(parent, name);
> if (IS_ERR(dentry)) {
> if (dentry == ERR_PTR(-EEXIST))
> pr_err("'%s' already exists in '%pd'\n", name, parent);
> simple_release_fs(&debugfs_mount, &debugfs_mount_count);
> }
> work for you?
That's exactly what I was thinking.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 03/17] rpc_pipe: clean failure exits in fill_super
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
2025-06-13 7:34 ` [PATCH 02/17] new helper: simple_start_creating() Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 04/17] rpc_{rmdir_,}depopulate(): use simple_recursive_removal() instead Al Viro
` (14 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
->kill_sb() will be called immediately after a failure return
anyway, so we don't need to bother with notifier chain and
rpc_gssd_dummy_depopulate(). What's more, rpc_gssd_dummy_populate()
failure exits do not need to bother with __rpc_depopulate() -
anything added to the tree will be taken out by ->kill_sb().
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 67 +++++++++++--------------------------------
1 file changed, 16 insertions(+), 51 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 98f78cd55905..580e078e49a3 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -1292,7 +1292,7 @@ static const struct rpc_filelist gssd_dummy_info_file[] = {
* Create a dummy set of directories and a pipe that gssd can hold open to
* indicate that it is up and running.
*/
-static struct dentry *
+static int
rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data)
{
int ret = 0;
@@ -1303,58 +1303,37 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data)
/* We should never get this far if "gssd" doesn't exist */
gssd_dentry = try_lookup_noperm(&QSTR(files[RPCAUTH_gssd].name), root);
if (!gssd_dentry)
- return ERR_PTR(-ENOENT);
+ return -ENOENT;
ret = rpc_populate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1, NULL);
if (ret) {
- pipe_dentry = ERR_PTR(ret);
- goto out;
+ dput(gssd_dentry);
+ return ret;
}
clnt_dentry = try_lookup_noperm(&QSTR(gssd_dummy_clnt_dir[0].name),
gssd_dentry);
- if (!clnt_dentry) {
- __rpc_depopulate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1);
- pipe_dentry = ERR_PTR(-ENOENT);
- goto out;
- }
+ dput(gssd_dentry);
+ if (!clnt_dentry)
+ return -ENOENT;
ret = rpc_populate(clnt_dentry, gssd_dummy_info_file, 0, 1, NULL);
if (ret) {
- __rpc_depopulate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1);
- pipe_dentry = ERR_PTR(ret);
- goto out;
+ dput(clnt_dentry);
+ return ret;
}
-
pipe_dentry = rpc_mkpipe_dentry(clnt_dentry, "gssd", NULL, pipe_data);
- if (IS_ERR(pipe_dentry)) {
- __rpc_depopulate(clnt_dentry, gssd_dummy_info_file, 0, 1);
- __rpc_depopulate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1);
- }
-out:
dput(clnt_dentry);
- dput(gssd_dentry);
- return pipe_dentry;
-}
-
-static void
-rpc_gssd_dummy_depopulate(struct dentry *pipe_dentry)
-{
- struct dentry *clnt_dir = pipe_dentry->d_parent;
- struct dentry *gssd_dir = clnt_dir->d_parent;
-
- dget(pipe_dentry);
- __rpc_rmpipe(d_inode(clnt_dir), pipe_dentry);
- __rpc_depopulate(clnt_dir, gssd_dummy_info_file, 0, 1);
- __rpc_depopulate(gssd_dir, gssd_dummy_clnt_dir, 0, 1);
- dput(pipe_dentry);
+ if (IS_ERR(pipe_dentry))
+ ret = PTR_ERR(pipe_dentry);
+ return ret;
}
static int
rpc_fill_super(struct super_block *sb, struct fs_context *fc)
{
struct inode *inode;
- struct dentry *root, *gssd_dentry;
+ struct dentry *root;
struct net *net = sb->s_fs_info;
struct sunrpc_net *sn = net_generic(net, sunrpc_net_id);
int err;
@@ -1373,11 +1352,9 @@ rpc_fill_super(struct super_block *sb, struct fs_context *fc)
if (rpc_populate(root, files, RPCAUTH_lockd, RPCAUTH_RootEOF, NULL))
return -ENOMEM;
- gssd_dentry = rpc_gssd_dummy_populate(root, sn->gssd_dummy);
- if (IS_ERR(gssd_dentry)) {
- __rpc_depopulate(root, files, RPCAUTH_lockd, RPCAUTH_RootEOF);
- return PTR_ERR(gssd_dentry);
- }
+ err = rpc_gssd_dummy_populate(root, sn->gssd_dummy);
+ if (err)
+ return err;
dprintk("RPC: sending pipefs MOUNT notification for net %x%s\n",
net->ns.inum, NET_NAME(net));
@@ -1386,18 +1363,6 @@ rpc_fill_super(struct super_block *sb, struct fs_context *fc)
err = blocking_notifier_call_chain(&rpc_pipefs_notifier_list,
RPC_PIPEFS_MOUNT,
sb);
- if (err)
- goto err_depopulate;
- mutex_unlock(&sn->pipefs_sb_lock);
- return 0;
-
-err_depopulate:
- rpc_gssd_dummy_depopulate(gssd_dentry);
- blocking_notifier_call_chain(&rpc_pipefs_notifier_list,
- RPC_PIPEFS_UMOUNT,
- sb);
- sn->pipefs_sb = NULL;
- __rpc_depopulate(root, files, RPCAUTH_lockd, RPCAUTH_RootEOF);
mutex_unlock(&sn->pipefs_sb_lock);
return err;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 04/17] rpc_{rmdir_,}depopulate(): use simple_recursive_removal() instead
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
2025-06-13 7:34 ` [PATCH 02/17] new helper: simple_start_creating() Al Viro
2025-06-13 7:34 ` [PATCH 03/17] rpc_pipe: clean failure exits in fill_super Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 05/17] rpc_unlink(): use simple_recursive_removal() Al Viro
` (13 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
no need to give an exact list of objects to be remove when it's
simply every child of the victim directory.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 44 +++----------------------------------------
1 file changed, 3 insertions(+), 41 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 580e078e49a3..9571cbd91305 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -678,17 +678,6 @@ static void __rpc_depopulate(struct dentry *parent,
}
}
-static void rpc_depopulate(struct dentry *parent,
- const struct rpc_filelist *files,
- int start, int eof)
-{
- struct inode *dir = d_inode(parent);
-
- inode_lock_nested(dir, I_MUTEX_CHILD);
- __rpc_depopulate(parent, files, start, eof);
- inode_unlock(dir);
-}
-
static int rpc_populate(struct dentry *parent,
const struct rpc_filelist *files,
int start, int eof,
@@ -762,24 +751,6 @@ static struct dentry *rpc_mkdir_populate(struct dentry *parent,
goto out;
}
-static int rpc_rmdir_depopulate(struct dentry *dentry,
- void (*depopulate)(struct dentry *))
-{
- struct dentry *parent;
- struct inode *dir;
- int error;
-
- parent = dget_parent(dentry);
- dir = d_inode(parent);
- inode_lock_nested(dir, I_MUTEX_PARENT);
- if (depopulate != NULL)
- depopulate(dentry);
- error = __rpc_rmdir(dir, dentry);
- inode_unlock(dir);
- dput(parent);
- return error;
-}
-
/**
* rpc_mkpipe_dentry - make an rpc_pipefs file for kernel<->userspace
* communication
@@ -1030,11 +1001,6 @@ static int rpc_clntdir_populate(struct dentry *dentry, void *private)
private);
}
-static void rpc_clntdir_depopulate(struct dentry *dentry)
-{
- rpc_depopulate(dentry, authfiles, RPCAUTH_info, RPCAUTH_EOF);
-}
-
/**
* rpc_create_client_dir - Create a new rpc_client directory in rpc_pipefs
* @dentry: the parent of new directory
@@ -1073,7 +1039,8 @@ int rpc_remove_client_dir(struct rpc_clnt *rpc_client)
return 0;
rpc_destroy_pipe_dir_objects(&rpc_client->cl_pipedir_objects);
rpc_client->cl_pipedir_objects.pdh_dentry = NULL;
- return rpc_rmdir_depopulate(dentry, rpc_clntdir_depopulate);
+ simple_recursive_removal(dentry, NULL);
+ return 0;
}
static const struct rpc_filelist cache_pipefs_files[3] = {
@@ -1101,11 +1068,6 @@ static int rpc_cachedir_populate(struct dentry *dentry, void *private)
private);
}
-static void rpc_cachedir_depopulate(struct dentry *dentry)
-{
- rpc_depopulate(dentry, cache_pipefs_files, 0, 3);
-}
-
struct dentry *rpc_create_cache_dir(struct dentry *parent, const char *name,
umode_t umode, struct cache_detail *cd)
{
@@ -1115,7 +1077,7 @@ struct dentry *rpc_create_cache_dir(struct dentry *parent, const char *name,
void rpc_remove_cache_dir(struct dentry *dentry)
{
- rpc_rmdir_depopulate(dentry, rpc_cachedir_depopulate);
+ simple_recursive_removal(dentry, NULL);
}
/*
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 05/17] rpc_unlink(): use simple_recursive_removal()
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (2 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 04/17] rpc_{rmdir_,}depopulate(): use simple_recursive_removal() instead Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 06/17] rpc_populate(): lift cleanup into callers Al Viro
` (12 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
note that the callback of simple_recursive_removal() is called with
the parent locked; the victim isn't locked by the caller.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 24 ++++--------------------
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 9571cbd91305..67621a94f67b 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -168,8 +168,9 @@ rpc_inode_setowner(struct inode *inode, void *private)
}
static void
-rpc_close_pipes(struct inode *inode)
+rpc_close_pipes(struct dentry *dentry)
{
+ struct inode *inode = dentry->d_inode;
struct rpc_pipe *pipe = RPC_I(inode)->pipe;
int need_release;
LIST_HEAD(free_list);
@@ -619,14 +620,6 @@ static int __rpc_unlink(struct inode *dir, struct dentry *dentry)
return ret;
}
-static int __rpc_rmpipe(struct inode *dir, struct dentry *dentry)
-{
- struct inode *inode = d_inode(dentry);
-
- rpc_close_pipes(inode);
- return __rpc_unlink(dir, dentry);
-}
-
static struct dentry *__rpc_lookup_create_exclusive(struct dentry *parent,
const char *name)
{
@@ -814,17 +807,8 @@ EXPORT_SYMBOL_GPL(rpc_mkpipe_dentry);
int
rpc_unlink(struct dentry *dentry)
{
- struct dentry *parent;
- struct inode *dir;
- int error = 0;
-
- parent = dget_parent(dentry);
- dir = d_inode(parent);
- inode_lock_nested(dir, I_MUTEX_PARENT);
- error = __rpc_rmpipe(dir, dentry);
- inode_unlock(dir);
- dput(parent);
- return error;
+ simple_recursive_removal(dentry, rpc_close_pipes);
+ return 0;
}
EXPORT_SYMBOL_GPL(rpc_unlink);
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 06/17] rpc_populate(): lift cleanup into callers
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (3 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 05/17] rpc_unlink(): use simple_recursive_removal() Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 07/17] rpc_unlink(): saner calling conventions Al Viro
` (11 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
rpc_populate() is called either from fill_super (where we don't
need to remove any files on failure - rpc_kill_sb() will take
them all out anyway) or from rpc_mkdir_populate(), where we need
to remove the directory we'd been trying to populate along with
whatever we'd put into it before we failed. Simpler to combine
that into simple_recursive_removal() there.
Note that rpc_pipe is overlocking directories quite a bit -
locked parent is no obstacle to finding a child in dcache, so
keeping it locked won't prevent userland observing a partially
built subtree.
All we need is to follow minimal VFS requirements; it's not
as if clients used directory locking for exclusion - tree
changes are serialized, but that's done on ->pipefs_sb_lock.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 71 +++----------------------------------------
1 file changed, 5 insertions(+), 66 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 67621a94f67b..46fa00ac5e0e 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -594,32 +594,6 @@ static int __rpc_mkpipe_dentry(struct inode *dir, struct dentry *dentry,
return 0;
}
-static int __rpc_rmdir(struct inode *dir, struct dentry *dentry)
-{
- int ret;
-
- dget(dentry);
- ret = simple_rmdir(dir, dentry);
- d_drop(dentry);
- if (!ret)
- fsnotify_rmdir(dir, dentry);
- dput(dentry);
- return ret;
-}
-
-static int __rpc_unlink(struct inode *dir, struct dentry *dentry)
-{
- int ret;
-
- dget(dentry);
- ret = simple_unlink(dir, dentry);
- d_drop(dentry);
- if (!ret)
- fsnotify_unlink(dir, dentry);
- dput(dentry);
- return ret;
-}
-
static struct dentry *__rpc_lookup_create_exclusive(struct dentry *parent,
const char *name)
{
@@ -636,41 +610,6 @@ static struct dentry *__rpc_lookup_create_exclusive(struct dentry *parent,
return ERR_PTR(-EEXIST);
}
-/*
- * FIXME: This probably has races.
- */
-static void __rpc_depopulate(struct dentry *parent,
- const struct rpc_filelist *files,
- int start, int eof)
-{
- struct inode *dir = d_inode(parent);
- struct dentry *dentry;
- struct qstr name;
- int i;
-
- for (i = start; i < eof; i++) {
- name.name = files[i].name;
- name.len = strlen(files[i].name);
- dentry = try_lookup_noperm(&name, parent);
-
- if (dentry == NULL)
- continue;
- if (d_really_is_negative(dentry))
- goto next;
- switch (d_inode(dentry)->i_mode & S_IFMT) {
- default:
- BUG();
- case S_IFREG:
- __rpc_unlink(dir, dentry);
- break;
- case S_IFDIR:
- __rpc_rmdir(dir, dentry);
- }
-next:
- dput(dentry);
- }
-}
-
static int rpc_populate(struct dentry *parent,
const struct rpc_filelist *files,
int start, int eof,
@@ -707,7 +646,6 @@ static int rpc_populate(struct dentry *parent,
inode_unlock(dir);
return 0;
out_bad:
- __rpc_depopulate(parent, files, start, eof);
inode_unlock(dir);
printk(KERN_WARNING "%s: %s failed to populate directory %pd\n",
__FILE__, __func__, parent);
@@ -731,14 +669,15 @@ static struct dentry *rpc_mkdir_populate(struct dentry *parent,
goto out_err;
if (populate != NULL) {
error = populate(dentry, args_populate);
- if (error)
- goto err_rmdir;
+ if (error) {
+ inode_unlock(dir);
+ simple_recursive_removal(dentry, NULL);
+ return ERR_PTR(error);
+ }
}
out:
inode_unlock(dir);
return dentry;
-err_rmdir:
- __rpc_rmdir(dir, dentry);
out_err:
dentry = ERR_PTR(error);
goto out;
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 07/17] rpc_unlink(): saner calling conventions
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (4 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 06/17] rpc_populate(): lift cleanup into callers Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 08/17] rpc_mkpipe_dentry(): " Al Viro
` (10 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
1) pass it pipe instead of pipe->dentry
2) zero pipe->dentry afterwards
3) it always returns 0; why bother?
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/nfs/blocklayout/rpc_pipefs.c | 12 ++----------
fs/nfs/nfs4idmap.c | 6 +-----
fs/nfsd/nfs4recover.c | 12 ++----------
include/linux/sunrpc/rpc_pipe_fs.h | 2 +-
net/sunrpc/auth_gss/auth_gss.c | 6 +-----
net/sunrpc/rpc_pipe.c | 12 +++++++-----
6 files changed, 14 insertions(+), 36 deletions(-)
diff --git a/fs/nfs/blocklayout/rpc_pipefs.c b/fs/nfs/blocklayout/rpc_pipefs.c
index d8d50a88de04..25d429e44eb4 100644
--- a/fs/nfs/blocklayout/rpc_pipefs.c
+++ b/fs/nfs/blocklayout/rpc_pipefs.c
@@ -154,13 +154,6 @@ static struct dentry *nfs4blocklayout_register_sb(struct super_block *sb,
return dentry;
}
-static void nfs4blocklayout_unregister_sb(struct super_block *sb,
- struct rpc_pipe *pipe)
-{
- if (pipe->dentry)
- rpc_unlink(pipe->dentry);
-}
-
static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
void *ptr)
{
@@ -188,8 +181,7 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
nn->bl_device_pipe->dentry = dentry;
break;
case RPC_PIPEFS_UMOUNT:
- if (nn->bl_device_pipe->dentry)
- nfs4blocklayout_unregister_sb(sb, nn->bl_device_pipe);
+ rpc_unlink(nn->bl_device_pipe);
break;
default:
ret = -ENOTSUPP;
@@ -224,7 +216,7 @@ static void nfs4blocklayout_unregister_net(struct net *net,
pipefs_sb = rpc_get_sb_net(net);
if (pipefs_sb) {
- nfs4blocklayout_unregister_sb(pipefs_sb, pipe);
+ rpc_unlink(pipe);
rpc_put_sb_net(net);
}
}
diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index 25a7c771cfd8..adc03232b851 100644
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -424,12 +424,8 @@ static void nfs_idmap_pipe_destroy(struct dentry *dir,
struct rpc_pipe_dir_object *pdo)
{
struct idmap *idmap = pdo->pdo_data;
- struct rpc_pipe *pipe = idmap->idmap_pipe;
- if (pipe->dentry) {
- rpc_unlink(pipe->dentry);
- pipe->dentry = NULL;
- }
+ rpc_unlink(idmap->idmap_pipe);
}
static int nfs_idmap_pipe_create(struct dentry *dir,
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 82785db730d9..bbd29b3b573f 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -963,13 +963,6 @@ nfsd4_cld_register_sb(struct super_block *sb, struct rpc_pipe *pipe)
return dentry;
}
-static void
-nfsd4_cld_unregister_sb(struct rpc_pipe *pipe)
-{
- if (pipe->dentry)
- rpc_unlink(pipe->dentry);
-}
-
static struct dentry *
nfsd4_cld_register_net(struct net *net, struct rpc_pipe *pipe)
{
@@ -991,7 +984,7 @@ nfsd4_cld_unregister_net(struct net *net, struct rpc_pipe *pipe)
sb = rpc_get_sb_net(net);
if (sb) {
- nfsd4_cld_unregister_sb(pipe);
+ rpc_unlink(pipe);
rpc_put_sb_net(net);
}
}
@@ -2142,8 +2135,7 @@ rpc_pipefs_event(struct notifier_block *nb, unsigned long event, void *ptr)
cn->cn_pipe->dentry = dentry;
break;
case RPC_PIPEFS_UMOUNT:
- if (cn->cn_pipe->dentry)
- nfsd4_cld_unregister_sb(cn->cn_pipe);
+ rpc_unlink(cn->cn_pipe);
break;
default:
ret = -ENOTSUPP;
diff --git a/include/linux/sunrpc/rpc_pipe_fs.h b/include/linux/sunrpc/rpc_pipe_fs.h
index 3b35b6f6533a..a8c0a500d55c 100644
--- a/include/linux/sunrpc/rpc_pipe_fs.h
+++ b/include/linux/sunrpc/rpc_pipe_fs.h
@@ -129,7 +129,7 @@ struct rpc_pipe *rpc_mkpipe_data(const struct rpc_pipe_ops *ops, int flags);
void rpc_destroy_pipe_data(struct rpc_pipe *pipe);
extern struct dentry *rpc_mkpipe_dentry(struct dentry *, const char *, void *,
struct rpc_pipe *);
-extern int rpc_unlink(struct dentry *);
+extern void rpc_unlink(struct rpc_pipe *);
extern int register_rpc_pipefs(void);
extern void unregister_rpc_pipefs(void);
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 0fa244f16876..f2a44d589cfb 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -887,12 +887,8 @@ static void gss_pipe_dentry_destroy(struct dentry *dir,
struct rpc_pipe_dir_object *pdo)
{
struct gss_pipe *gss_pipe = pdo->pdo_data;
- struct rpc_pipe *pipe = gss_pipe->pipe;
- if (pipe->dentry != NULL) {
- rpc_unlink(pipe->dentry);
- pipe->dentry = NULL;
- }
+ rpc_unlink(gss_pipe->pipe);
}
static int gss_pipe_dentry_create(struct dentry *dir,
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 46fa00ac5e0e..2046582c4f35 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -737,17 +737,19 @@ EXPORT_SYMBOL_GPL(rpc_mkpipe_dentry);
/**
* rpc_unlink - remove a pipe
- * @dentry: dentry for the pipe, as returned from rpc_mkpipe
+ * @pipe: the pipe to be removed
*
* After this call, lookups will no longer find the pipe, and any
* attempts to read or write using preexisting opens of the pipe will
* return -EPIPE.
*/
-int
-rpc_unlink(struct dentry *dentry)
+void
+rpc_unlink(struct rpc_pipe *pipe)
{
- simple_recursive_removal(dentry, rpc_close_pipes);
- return 0;
+ if (pipe->dentry) {
+ simple_recursive_removal(pipe->dentry, rpc_close_pipes);
+ pipe->dentry = NULL;
+ }
}
EXPORT_SYMBOL_GPL(rpc_unlink);
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 08/17] rpc_mkpipe_dentry(): saner calling conventions
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (5 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 07/17] rpc_unlink(): saner calling conventions Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 09/17] rpc_pipe: don't overdo directory locking Al Viro
` (9 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
Instead of returning a dentry or ERR_PTR(-E...), return 0 and store
dentry into pipe->dentry on success and return -E... on failure.
Callers are happier that way...
NOTE: dummy rpc_pipe is getting ->dentry set; we never access that,
since we
1) never call rpc_unlink() for it (dentry is taken out by
->kill_sb())
2) never call rpc_queue_upcall() for it (writing to that
sucker fails; no downcalls are ever submitted, so no replies are
going to arrive)
IOW, having that ->dentry set (and left dangling) is harmless,
if ugly; cleaner solution will take more massage.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/nfs/blocklayout/rpc_pipefs.c | 41 ++++++++++++------------------
fs/nfs/nfs4idmap.c | 8 +-----
fs/nfsd/nfs4recover.c | 37 ++++++++++-----------------
include/linux/sunrpc/rpc_pipe_fs.h | 2 +-
net/sunrpc/auth_gss/auth_gss.c | 7 +----
net/sunrpc/rpc_pipe.c | 29 +++++++++------------
6 files changed, 45 insertions(+), 79 deletions(-)
diff --git a/fs/nfs/blocklayout/rpc_pipefs.c b/fs/nfs/blocklayout/rpc_pipefs.c
index 25d429e44eb4..d526f5ba7887 100644
--- a/fs/nfs/blocklayout/rpc_pipefs.c
+++ b/fs/nfs/blocklayout/rpc_pipefs.c
@@ -141,17 +141,18 @@ static const struct rpc_pipe_ops bl_upcall_ops = {
.destroy_msg = bl_pipe_destroy_msg,
};
-static struct dentry *nfs4blocklayout_register_sb(struct super_block *sb,
+static int nfs4blocklayout_register_sb(struct super_block *sb,
struct rpc_pipe *pipe)
{
- struct dentry *dir, *dentry;
+ struct dentry *dir;
+ int err;
dir = rpc_d_lookup_sb(sb, NFS_PIPE_DIRNAME);
if (dir == NULL)
- return ERR_PTR(-ENOENT);
- dentry = rpc_mkpipe_dentry(dir, "blocklayout", NULL, pipe);
+ return -ENOENT;
+ err = rpc_mkpipe_dentry(dir, "blocklayout", NULL, pipe);
dput(dir);
- return dentry;
+ return err;
}
static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
@@ -160,7 +161,6 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
struct super_block *sb = ptr;
struct net *net = sb->s_fs_info;
struct nfs_net *nn = net_generic(net, nfs_net_id);
- struct dentry *dentry;
int ret = 0;
if (!try_module_get(THIS_MODULE))
@@ -173,12 +173,7 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
switch (event) {
case RPC_PIPEFS_MOUNT:
- dentry = nfs4blocklayout_register_sb(sb, nn->bl_device_pipe);
- if (IS_ERR(dentry)) {
- ret = PTR_ERR(dentry);
- break;
- }
- nn->bl_device_pipe->dentry = dentry;
+ ret = nfs4blocklayout_register_sb(sb, nn->bl_device_pipe);
break;
case RPC_PIPEFS_UMOUNT:
rpc_unlink(nn->bl_device_pipe);
@@ -195,18 +190,17 @@ static struct notifier_block nfs4blocklayout_block = {
.notifier_call = rpc_pipefs_event,
};
-static struct dentry *nfs4blocklayout_register_net(struct net *net,
- struct rpc_pipe *pipe)
+static int nfs4blocklayout_register_net(struct net *net, struct rpc_pipe *pipe)
{
struct super_block *pipefs_sb;
- struct dentry *dentry;
+ int ret;
pipefs_sb = rpc_get_sb_net(net);
if (!pipefs_sb)
- return NULL;
- dentry = nfs4blocklayout_register_sb(pipefs_sb, pipe);
+ return 0;
+ ret = nfs4blocklayout_register_sb(pipefs_sb, pipe);
rpc_put_sb_net(net);
- return dentry;
+ return ret;
}
static void nfs4blocklayout_unregister_net(struct net *net,
@@ -224,20 +218,17 @@ static void nfs4blocklayout_unregister_net(struct net *net,
static int nfs4blocklayout_net_init(struct net *net)
{
struct nfs_net *nn = net_generic(net, nfs_net_id);
- struct dentry *dentry;
+ int err;
mutex_init(&nn->bl_mutex);
init_waitqueue_head(&nn->bl_wq);
nn->bl_device_pipe = rpc_mkpipe_data(&bl_upcall_ops, 0);
if (IS_ERR(nn->bl_device_pipe))
return PTR_ERR(nn->bl_device_pipe);
- dentry = nfs4blocklayout_register_net(net, nn->bl_device_pipe);
- if (IS_ERR(dentry)) {
+ err = nfs4blocklayout_register_net(net, nn->bl_device_pipe);
+ if (unlikely(err))
rpc_destroy_pipe_data(nn->bl_device_pipe);
- return PTR_ERR(dentry);
- }
- nn->bl_device_pipe->dentry = dentry;
- return 0;
+ return err;
}
static void nfs4blocklayout_net_exit(struct net *net)
diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index adc03232b851..00932500fce4 100644
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -432,14 +432,8 @@ static int nfs_idmap_pipe_create(struct dentry *dir,
struct rpc_pipe_dir_object *pdo)
{
struct idmap *idmap = pdo->pdo_data;
- struct rpc_pipe *pipe = idmap->idmap_pipe;
- struct dentry *dentry;
- dentry = rpc_mkpipe_dentry(dir, "idmap", idmap, pipe);
- if (IS_ERR(dentry))
- return PTR_ERR(dentry);
- pipe->dentry = dentry;
- return 0;
+ return rpc_mkpipe_dentry(dir, "idmap", idmap, idmap->idmap_pipe);
}
static const struct rpc_pipe_dir_object_ops nfs_idmap_pipe_dir_object_ops = {
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index bbd29b3b573f..2231192ec33f 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -950,31 +950,32 @@ static const struct rpc_pipe_ops cld_upcall_ops = {
.destroy_msg = cld_pipe_destroy_msg,
};
-static struct dentry *
+static int
nfsd4_cld_register_sb(struct super_block *sb, struct rpc_pipe *pipe)
{
- struct dentry *dir, *dentry;
+ struct dentry *dir;
+ int err;
dir = rpc_d_lookup_sb(sb, NFSD_PIPE_DIR);
if (dir == NULL)
- return ERR_PTR(-ENOENT);
- dentry = rpc_mkpipe_dentry(dir, NFSD_CLD_PIPE, NULL, pipe);
+ return -ENOENT;
+ err = rpc_mkpipe_dentry(dir, NFSD_CLD_PIPE, NULL, pipe);
dput(dir);
- return dentry;
+ return err;
}
-static struct dentry *
+static int
nfsd4_cld_register_net(struct net *net, struct rpc_pipe *pipe)
{
struct super_block *sb;
- struct dentry *dentry;
+ int err;
sb = rpc_get_sb_net(net);
if (!sb)
- return NULL;
- dentry = nfsd4_cld_register_sb(sb, pipe);
+ return 0;
+ err = nfsd4_cld_register_sb(sb, pipe);
rpc_put_sb_net(net);
- return dentry;
+ return err;
}
static void
@@ -994,7 +995,6 @@ static int
__nfsd4_init_cld_pipe(struct net *net)
{
int ret;
- struct dentry *dentry;
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
struct cld_net *cn;
@@ -1015,13 +1015,10 @@ __nfsd4_init_cld_pipe(struct net *net)
spin_lock_init(&cn->cn_lock);
INIT_LIST_HEAD(&cn->cn_list);
- dentry = nfsd4_cld_register_net(net, cn->cn_pipe);
- if (IS_ERR(dentry)) {
- ret = PTR_ERR(dentry);
+ ret = nfsd4_cld_register_net(net, cn->cn_pipe);
+ if (unlikely(ret))
goto err_destroy_data;
- }
- cn->cn_pipe->dentry = dentry;
#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
cn->cn_has_legacy = false;
#endif
@@ -2114,7 +2111,6 @@ rpc_pipefs_event(struct notifier_block *nb, unsigned long event, void *ptr)
struct net *net = sb->s_fs_info;
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
struct cld_net *cn = nn->cld_net;
- struct dentry *dentry;
int ret = 0;
if (!try_module_get(THIS_MODULE))
@@ -2127,12 +2123,7 @@ rpc_pipefs_event(struct notifier_block *nb, unsigned long event, void *ptr)
switch (event) {
case RPC_PIPEFS_MOUNT:
- dentry = nfsd4_cld_register_sb(sb, cn->cn_pipe);
- if (IS_ERR(dentry)) {
- ret = PTR_ERR(dentry);
- break;
- }
- cn->cn_pipe->dentry = dentry;
+ ret = nfsd4_cld_register_sb(sb, cn->cn_pipe);
break;
case RPC_PIPEFS_UMOUNT:
rpc_unlink(cn->cn_pipe);
diff --git a/include/linux/sunrpc/rpc_pipe_fs.h b/include/linux/sunrpc/rpc_pipe_fs.h
index a8c0a500d55c..8cc3a5df9801 100644
--- a/include/linux/sunrpc/rpc_pipe_fs.h
+++ b/include/linux/sunrpc/rpc_pipe_fs.h
@@ -127,7 +127,7 @@ extern void rpc_remove_cache_dir(struct dentry *);
struct rpc_pipe *rpc_mkpipe_data(const struct rpc_pipe_ops *ops, int flags);
void rpc_destroy_pipe_data(struct rpc_pipe *pipe);
-extern struct dentry *rpc_mkpipe_dentry(struct dentry *, const char *, void *,
+extern int rpc_mkpipe_dentry(struct dentry *, const char *, void *,
struct rpc_pipe *);
extern void rpc_unlink(struct rpc_pipe *);
extern int register_rpc_pipefs(void);
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index f2a44d589cfb..6c23d46a1dcc 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -895,13 +895,8 @@ static int gss_pipe_dentry_create(struct dentry *dir,
struct rpc_pipe_dir_object *pdo)
{
struct gss_pipe *p = pdo->pdo_data;
- struct dentry *dentry;
- dentry = rpc_mkpipe_dentry(dir, p->name, p->clnt, p->pipe);
- if (IS_ERR(dentry))
- return PTR_ERR(dentry);
- p->pipe->dentry = dentry;
- return 0;
+ return rpc_mkpipe_dentry(dir, p->name, p->clnt, p->pipe);
}
static const struct rpc_pipe_dir_object_ops gss_pipe_dir_object_ops = {
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 2046582c4f35..dac1c35a642f 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -702,7 +702,7 @@ static struct dentry *rpc_mkdir_populate(struct dentry *parent,
* The @private argument passed here will be available to all these methods
* from the file pointer, via RPC_I(file_inode(file))->private.
*/
-struct dentry *rpc_mkpipe_dentry(struct dentry *parent, const char *name,
+int rpc_mkpipe_dentry(struct dentry *parent, const char *name,
void *private, struct rpc_pipe *pipe)
{
struct dentry *dentry;
@@ -717,21 +717,19 @@ struct dentry *rpc_mkpipe_dentry(struct dentry *parent, const char *name,
inode_lock_nested(dir, I_MUTEX_PARENT);
dentry = __rpc_lookup_create_exclusive(parent, name);
- if (IS_ERR(dentry))
- goto out;
+ if (IS_ERR(dentry)) {
+ inode_unlock(dir);
+ return PTR_ERR(dentry);
+ }
err = __rpc_mkpipe_dentry(dir, dentry, umode, &rpc_pipe_fops,
private, pipe);
- if (err)
- goto out_err;
-out:
+ if (unlikely(err))
+ pr_warn("%s() failed to create pipe %pd/%s (errno = %d)\n",
+ __func__, parent, name, err);
+ else
+ pipe->dentry = dentry;
inode_unlock(dir);
- return dentry;
-out_err:
- dentry = ERR_PTR(err);
- printk(KERN_WARNING "%s: %s() failed to create pipe %pd/%s (errno = %d)\n",
- __FILE__, __func__, parent, name,
- err);
- goto out;
+ return err;
}
EXPORT_SYMBOL_GPL(rpc_mkpipe_dentry);
@@ -1185,7 +1183,6 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data)
int ret = 0;
struct dentry *gssd_dentry;
struct dentry *clnt_dentry = NULL;
- struct dentry *pipe_dentry = NULL;
/* We should never get this far if "gssd" doesn't exist */
gssd_dentry = try_lookup_noperm(&QSTR(files[RPCAUTH_gssd].name), root);
@@ -1209,10 +1206,8 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data)
dput(clnt_dentry);
return ret;
}
- pipe_dentry = rpc_mkpipe_dentry(clnt_dentry, "gssd", NULL, pipe_data);
+ ret = rpc_mkpipe_dentry(clnt_dentry, "gssd", NULL, pipe_data);
dput(clnt_dentry);
- if (IS_ERR(pipe_dentry))
- ret = PTR_ERR(pipe_dentry);
return ret;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 09/17] rpc_pipe: don't overdo directory locking
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (6 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 08/17] rpc_mkpipe_dentry(): " Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 10/17] rpc_pipe: saner primitive for creating subdirectories Al Viro
` (8 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
Don't try to hold directories locked more than VFS requires;
lock just before getting a child to be made positive (using
simple_start_creating()) and unlock as soon as the child is
created. There's no benefit in keeping the parent locked
while populating the child - it won't stop dcache lookups anyway.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 44 +++++++++----------------------------------
1 file changed, 9 insertions(+), 35 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index dac1c35a642f..a61c1173738c 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -594,22 +594,6 @@ static int __rpc_mkpipe_dentry(struct inode *dir, struct dentry *dentry,
return 0;
}
-static struct dentry *__rpc_lookup_create_exclusive(struct dentry *parent,
- const char *name)
-{
- struct qstr q = QSTR(name);
- struct dentry *dentry = try_lookup_noperm(&q, parent);
- if (!dentry) {
- dentry = d_alloc(parent, &q);
- if (!dentry)
- return ERR_PTR(-ENOMEM);
- }
- if (d_really_is_negative(dentry))
- return dentry;
- dput(dentry);
- return ERR_PTR(-EEXIST);
-}
-
static int rpc_populate(struct dentry *parent,
const struct rpc_filelist *files,
int start, int eof,
@@ -619,9 +603,8 @@ static int rpc_populate(struct dentry *parent,
struct dentry *dentry;
int i, err;
- inode_lock(dir);
for (i = start; i < eof; i++) {
- dentry = __rpc_lookup_create_exclusive(parent, files[i].name);
+ dentry = simple_start_creating(parent, files[i].name);
err = PTR_ERR(dentry);
if (IS_ERR(dentry))
goto out_bad;
@@ -633,20 +616,20 @@ static int rpc_populate(struct dentry *parent,
files[i].mode,
files[i].i_fop,
private);
+ inode_unlock(dir);
break;
case S_IFDIR:
err = __rpc_mkdir(dir, dentry,
files[i].mode,
NULL,
private);
+ inode_unlock(dir);
}
if (err != 0)
goto out_bad;
}
- inode_unlock(dir);
return 0;
out_bad:
- inode_unlock(dir);
printk(KERN_WARNING "%s: %s failed to populate directory %pd\n",
__FILE__, __func__, parent);
return err;
@@ -660,27 +643,21 @@ static struct dentry *rpc_mkdir_populate(struct dentry *parent,
struct inode *dir = d_inode(parent);
int error;
- inode_lock_nested(dir, I_MUTEX_PARENT);
- dentry = __rpc_lookup_create_exclusive(parent, name);
+ dentry = simple_start_creating(parent, name);
if (IS_ERR(dentry))
- goto out;
+ return dentry;
error = __rpc_mkdir(dir, dentry, mode, NULL, private);
+ inode_unlock(dir);
if (error != 0)
- goto out_err;
+ return ERR_PTR(error);
if (populate != NULL) {
error = populate(dentry, args_populate);
if (error) {
- inode_unlock(dir);
simple_recursive_removal(dentry, NULL);
return ERR_PTR(error);
}
}
-out:
- inode_unlock(dir);
return dentry;
-out_err:
- dentry = ERR_PTR(error);
- goto out;
}
/**
@@ -715,12 +692,9 @@ int rpc_mkpipe_dentry(struct dentry *parent, const char *name,
if (pipe->ops->downcall == NULL)
umode &= ~0222;
- inode_lock_nested(dir, I_MUTEX_PARENT);
- dentry = __rpc_lookup_create_exclusive(parent, name);
- if (IS_ERR(dentry)) {
- inode_unlock(dir);
+ dentry = simple_start_creating(parent, name);
+ if (IS_ERR(dentry))
return PTR_ERR(dentry);
- }
err = __rpc_mkpipe_dentry(dir, dentry, umode, &rpc_pipe_fops,
private, pipe);
if (unlikely(err))
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 10/17] rpc_pipe: saner primitive for creating subdirectories
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (7 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 09/17] rpc_pipe: don't overdo directory locking Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 11/17] rpc_pipe: saner primitive for creating regular files Al Viro
` (7 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
All users of __rpc_mkdir() have the same form - start_creating(),
followed, in case of success, by __rpc_mkdir() and unlocking parent.
Combine that into a single helper, expanding __rpc_mkdir() into it,
along with the call of __rpc_create_common() in it.
Don't mess with d_drop() + d_add() - just d_instantiate() and be
done with that. The reason __rpc_create_common() goes for that
dance is that dentry it gets might or might not be hashed; here
we know it's hashed.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 67 +++++++++++++++++++++++++------------------
1 file changed, 39 insertions(+), 28 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index a61c1173738c..c3f269aadcaf 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -524,21 +524,6 @@ static int __rpc_create(struct inode *dir, struct dentry *dentry,
return 0;
}
-static int __rpc_mkdir(struct inode *dir, struct dentry *dentry,
- umode_t mode,
- const struct file_operations *i_fop,
- void *private)
-{
- int err;
-
- err = __rpc_create_common(dir, dentry, S_IFDIR | mode, i_fop, private);
- if (err)
- return err;
- inc_nlink(dir);
- fsnotify_mkdir(dir, dentry);
- return 0;
-}
-
static void
init_pipe(struct rpc_pipe *pipe)
{
@@ -594,6 +579,35 @@ static int __rpc_mkpipe_dentry(struct inode *dir, struct dentry *dentry,
return 0;
}
+static struct dentry *rpc_new_dir(struct dentry *parent,
+ const char *name,
+ umode_t mode,
+ void *private)
+{
+ struct dentry *dentry = simple_start_creating(parent, name);
+ struct inode *dir = parent->d_inode;
+ struct inode *inode;
+
+ if (IS_ERR(dentry))
+ return dentry;
+
+ inode = rpc_get_inode(dir->i_sb, S_IFDIR | mode);
+ if (unlikely(!inode)) {
+ dput(dentry);
+ inode_unlock(dir);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ inode->i_ino = iunique(dir->i_sb, 100);
+ rpc_inode_setowner(inode, private);
+ inc_nlink(dir);
+ d_instantiate(dentry, inode);
+ fsnotify_mkdir(dir, dentry);
+ inode_unlock(dir);
+
+ return dentry;
+}
+
static int rpc_populate(struct dentry *parent,
const struct rpc_filelist *files,
int start, int eof,
@@ -604,14 +618,14 @@ static int rpc_populate(struct dentry *parent,
int i, err;
for (i = start; i < eof; i++) {
- dentry = simple_start_creating(parent, files[i].name);
- err = PTR_ERR(dentry);
- if (IS_ERR(dentry))
- goto out_bad;
switch (files[i].mode & S_IFMT) {
default:
BUG();
case S_IFREG:
+ dentry = simple_start_creating(parent, files[i].name);
+ err = PTR_ERR(dentry);
+ if (IS_ERR(dentry))
+ goto out_bad;
err = __rpc_create(dir, dentry,
files[i].mode,
files[i].i_fop,
@@ -619,11 +633,13 @@ static int rpc_populate(struct dentry *parent,
inode_unlock(dir);
break;
case S_IFDIR:
- err = __rpc_mkdir(dir, dentry,
+ dentry = rpc_new_dir(parent,
+ files[i].name,
files[i].mode,
- NULL,
private);
- inode_unlock(dir);
+ err = PTR_ERR(dentry);
+ if (IS_ERR(dentry))
+ goto out_bad;
}
if (err != 0)
goto out_bad;
@@ -640,16 +656,11 @@ static struct dentry *rpc_mkdir_populate(struct dentry *parent,
int (*populate)(struct dentry *, void *), void *args_populate)
{
struct dentry *dentry;
- struct inode *dir = d_inode(parent);
int error;
- dentry = simple_start_creating(parent, name);
+ dentry = rpc_new_dir(parent, name, mode, private);
if (IS_ERR(dentry))
return dentry;
- error = __rpc_mkdir(dir, dentry, mode, NULL, private);
- inode_unlock(dir);
- if (error != 0)
- return ERR_PTR(error);
if (populate != NULL) {
error = populate(dentry, args_populate);
if (error) {
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 11/17] rpc_pipe: saner primitive for creating regular files
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (8 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 10/17] rpc_pipe: saner primitive for creating subdirectories Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 12/17] rpc_mkpipe_dentry(): switch to start_creating() Al Viro
` (6 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
rpc_new_file(); similar to rpc_new_dir(), except that here we pass
file_operations as well. Callers don't care about dentry, just
return 0 or -E...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 61 +++++++++++++++++++++++++------------------
1 file changed, 36 insertions(+), 25 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index c3f269aadcaf..8f88c75c9b30 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -510,20 +510,6 @@ static int __rpc_create_common(struct inode *dir, struct dentry *dentry,
return -ENOMEM;
}
-static int __rpc_create(struct inode *dir, struct dentry *dentry,
- umode_t mode,
- const struct file_operations *i_fop,
- void *private)
-{
- int err;
-
- err = __rpc_create_common(dir, dentry, S_IFREG | mode, i_fop, private);
- if (err)
- return err;
- fsnotify_create(dir, dentry);
- return 0;
-}
-
static void
init_pipe(struct rpc_pipe *pipe)
{
@@ -579,6 +565,35 @@ static int __rpc_mkpipe_dentry(struct inode *dir, struct dentry *dentry,
return 0;
}
+static int rpc_new_file(struct dentry *parent,
+ const char *name,
+ umode_t mode,
+ const struct file_operations *i_fop,
+ void *private)
+{
+ struct dentry *dentry = simple_start_creating(parent, name);
+ struct inode *dir = parent->d_inode;
+ struct inode *inode;
+
+ if (IS_ERR(dentry))
+ return PTR_ERR(dentry);
+
+ inode = rpc_get_inode(dir->i_sb, S_IFREG | mode);
+ if (unlikely(!inode)) {
+ dput(dentry);
+ inode_unlock(dir);
+ return -ENOMEM;
+ }
+ inode->i_ino = iunique(dir->i_sb, 100);
+ if (i_fop)
+ inode->i_fop = i_fop;
+ rpc_inode_setowner(inode, private);
+ d_instantiate(dentry, inode);
+ fsnotify_create(dir, dentry);
+ inode_unlock(dir);
+ return 0;
+}
+
static struct dentry *rpc_new_dir(struct dentry *parent,
const char *name,
umode_t mode,
@@ -613,7 +628,6 @@ static int rpc_populate(struct dentry *parent,
int start, int eof,
void *private)
{
- struct inode *dir = d_inode(parent);
struct dentry *dentry;
int i, err;
@@ -622,27 +636,24 @@ static int rpc_populate(struct dentry *parent,
default:
BUG();
case S_IFREG:
- dentry = simple_start_creating(parent, files[i].name);
- err = PTR_ERR(dentry);
- if (IS_ERR(dentry))
- goto out_bad;
- err = __rpc_create(dir, dentry,
+ err = rpc_new_file(parent,
+ files[i].name,
files[i].mode,
files[i].i_fop,
private);
- inode_unlock(dir);
+ if (err)
+ goto out_bad;
break;
case S_IFDIR:
dentry = rpc_new_dir(parent,
files[i].name,
files[i].mode,
private);
- err = PTR_ERR(dentry);
- if (IS_ERR(dentry))
+ if (IS_ERR(dentry)) {
+ err = PTR_ERR(dentry);
goto out_bad;
+ }
}
- if (err != 0)
- goto out_bad;
}
return 0;
out_bad:
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 12/17] rpc_mkpipe_dentry(): switch to start_creating()
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (9 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 11/17] rpc_pipe: saner primitive for creating regular files Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 19:27 ` Jeff Layton
2025-06-13 7:34 ` [PATCH 13/17] rpc_gssd_dummy_populate(): don't bother with rpc_populate() Al Viro
` (5 subsequent siblings)
16 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
... and make sure we set the rpc_pipe-private part of inode up before
attaching it to dentry.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 83 +++++++++++++++----------------------------
1 file changed, 29 insertions(+), 54 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 8f88c75c9b30..a52fe3bbf9dc 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -485,31 +485,6 @@ rpc_get_inode(struct super_block *sb, umode_t mode)
return inode;
}
-static int __rpc_create_common(struct inode *dir, struct dentry *dentry,
- umode_t mode,
- const struct file_operations *i_fop,
- void *private)
-{
- struct inode *inode;
-
- d_drop(dentry);
- inode = rpc_get_inode(dir->i_sb, mode);
- if (!inode)
- goto out_err;
- inode->i_ino = iunique(dir->i_sb, 100);
- if (i_fop)
- inode->i_fop = i_fop;
- if (private)
- rpc_inode_setowner(inode, private);
- d_add(dentry, inode);
- return 0;
-out_err:
- printk(KERN_WARNING "%s: %s failed to allocate inode for dentry %pd\n",
- __FILE__, __func__, dentry);
- dput(dentry);
- return -ENOMEM;
-}
-
static void
init_pipe(struct rpc_pipe *pipe)
{
@@ -546,25 +521,6 @@ struct rpc_pipe *rpc_mkpipe_data(const struct rpc_pipe_ops *ops, int flags)
}
EXPORT_SYMBOL_GPL(rpc_mkpipe_data);
-static int __rpc_mkpipe_dentry(struct inode *dir, struct dentry *dentry,
- umode_t mode,
- const struct file_operations *i_fop,
- void *private,
- struct rpc_pipe *pipe)
-{
- struct rpc_inode *rpci;
- int err;
-
- err = __rpc_create_common(dir, dentry, S_IFIFO | mode, i_fop, private);
- if (err)
- return err;
- rpci = RPC_I(d_inode(dentry));
- rpci->private = private;
- rpci->pipe = pipe;
- fsnotify_create(dir, dentry);
- return 0;
-}
-
static int rpc_new_file(struct dentry *parent,
const char *name,
umode_t mode,
@@ -704,8 +660,10 @@ static struct dentry *rpc_mkdir_populate(struct dentry *parent,
int rpc_mkpipe_dentry(struct dentry *parent, const char *name,
void *private, struct rpc_pipe *pipe)
{
- struct dentry *dentry;
struct inode *dir = d_inode(parent);
+ struct dentry *dentry;
+ struct inode *inode;
+ struct rpc_inode *rpci;
umode_t umode = S_IFIFO | 0600;
int err;
@@ -715,16 +673,33 @@ int rpc_mkpipe_dentry(struct dentry *parent, const char *name,
umode &= ~0222;
dentry = simple_start_creating(parent, name);
- if (IS_ERR(dentry))
- return PTR_ERR(dentry);
- err = __rpc_mkpipe_dentry(dir, dentry, umode, &rpc_pipe_fops,
- private, pipe);
- if (unlikely(err))
- pr_warn("%s() failed to create pipe %pd/%s (errno = %d)\n",
- __func__, parent, name, err);
- else
- pipe->dentry = dentry;
+ if (IS_ERR(dentry)) {
+ err = PTR_ERR(dentry);
+ goto failed;
+ }
+
+ inode = rpc_get_inode(dir->i_sb, umode);
+ if (unlikely(!inode)) {
+ dput(dentry);
+ inode_unlock(dir);
+ err = -ENOMEM;
+ goto failed;
+ }
+ inode->i_ino = iunique(dir->i_sb, 100);
+ inode->i_fop = &rpc_pipe_fops;
+ rpci = RPC_I(inode);
+ rpci->private = private;
+ rpci->pipe = pipe;
+ rpc_inode_setowner(inode, private);
+ d_instantiate(dentry, inode);
+ pipe->dentry = dentry;
+ fsnotify_create(dir, dentry);
inode_unlock(dir);
+ return 0;
+
+failed:
+ pr_warn("%s() failed to create pipe %pd/%s (errno = %d)\n",
+ __func__, parent, name, err);
return err;
}
EXPORT_SYMBOL_GPL(rpc_mkpipe_dentry);
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 12/17] rpc_mkpipe_dentry(): switch to start_creating()
2025-06-13 7:34 ` [PATCH 12/17] rpc_mkpipe_dentry(): switch to start_creating() Al Viro
@ 2025-06-13 19:27 ` Jeff Layton
2025-06-16 19:26 ` Al Viro
0 siblings, 1 reply; 25+ messages in thread
From: Jeff Layton @ 2025-06-13 19:27 UTC (permalink / raw)
To: Al Viro, linux-fsdevel; +Cc: chuck.lever, linux-nfs, neil, torvalds, trondmy
On Fri, 2025-06-13 at 08:34 +0100, Al Viro wrote:
> ... and make sure we set the rpc_pipe-private part of inode up before
> attaching it to dentry.
>
"rpc_pipe->private"
nit: subject should say "...switch to simple_start_creating()".
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> net/sunrpc/rpc_pipe.c | 83 +++++++++++++++----------------------------
> 1 file changed, 29 insertions(+), 54 deletions(-)
>
> diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
> index 8f88c75c9b30..a52fe3bbf9dc 100644
> --- a/net/sunrpc/rpc_pipe.c
> +++ b/net/sunrpc/rpc_pipe.c
> @@ -485,31 +485,6 @@ rpc_get_inode(struct super_block *sb, umode_t mode)
> return inode;
> }
>
> -static int __rpc_create_common(struct inode *dir, struct dentry *dentry,
> - umode_t mode,
> - const struct file_operations *i_fop,
> - void *private)
> -{
> - struct inode *inode;
> -
> - d_drop(dentry);
> - inode = rpc_get_inode(dir->i_sb, mode);
> - if (!inode)
> - goto out_err;
> - inode->i_ino = iunique(dir->i_sb, 100);
> - if (i_fop)
> - inode->i_fop = i_fop;
> - if (private)
> - rpc_inode_setowner(inode, private);
> - d_add(dentry, inode);
> - return 0;
> -out_err:
> - printk(KERN_WARNING "%s: %s failed to allocate inode for dentry %pd\n",
> - __FILE__, __func__, dentry);
> - dput(dentry);
> - return -ENOMEM;
> -}
> -
> static void
> init_pipe(struct rpc_pipe *pipe)
> {
> @@ -546,25 +521,6 @@ struct rpc_pipe *rpc_mkpipe_data(const struct rpc_pipe_ops *ops, int flags)
> }
> EXPORT_SYMBOL_GPL(rpc_mkpipe_data);
>
> -static int __rpc_mkpipe_dentry(struct inode *dir, struct dentry *dentry,
> - umode_t mode,
> - const struct file_operations *i_fop,
> - void *private,
> - struct rpc_pipe *pipe)
> -{
> - struct rpc_inode *rpci;
> - int err;
> -
> - err = __rpc_create_common(dir, dentry, S_IFIFO | mode, i_fop, private);
> - if (err)
> - return err;
> - rpci = RPC_I(d_inode(dentry));
> - rpci->private = private;
> - rpci->pipe = pipe;
> - fsnotify_create(dir, dentry);
> - return 0;
> -}
> -
> static int rpc_new_file(struct dentry *parent,
> const char *name,
> umode_t mode,
> @@ -704,8 +660,10 @@ static struct dentry *rpc_mkdir_populate(struct dentry *parent,
> int rpc_mkpipe_dentry(struct dentry *parent, const char *name,
> void *private, struct rpc_pipe *pipe)
> {
> - struct dentry *dentry;
> struct inode *dir = d_inode(parent);
> + struct dentry *dentry;
> + struct inode *inode;
> + struct rpc_inode *rpci;
> umode_t umode = S_IFIFO | 0600;
> int err;
>
> @@ -715,16 +673,33 @@ int rpc_mkpipe_dentry(struct dentry *parent, const char *name,
> umode &= ~0222;
>
> dentry = simple_start_creating(parent, name);
> - if (IS_ERR(dentry))
> - return PTR_ERR(dentry);
> - err = __rpc_mkpipe_dentry(dir, dentry, umode, &rpc_pipe_fops,
> - private, pipe);
> - if (unlikely(err))
> - pr_warn("%s() failed to create pipe %pd/%s (errno = %d)\n",
> - __func__, parent, name, err);
> - else
> - pipe->dentry = dentry;
> + if (IS_ERR(dentry)) {
> + err = PTR_ERR(dentry);
> + goto failed;
> + }
> +
> + inode = rpc_get_inode(dir->i_sb, umode);
> + if (unlikely(!inode)) {
> + dput(dentry);
> + inode_unlock(dir);
> + err = -ENOMEM;
> + goto failed;
> + }
> + inode->i_ino = iunique(dir->i_sb, 100);
> + inode->i_fop = &rpc_pipe_fops;
> + rpci = RPC_I(inode);
> + rpci->private = private;
> + rpci->pipe = pipe;
> + rpc_inode_setowner(inode, private);
> + d_instantiate(dentry, inode);
> + pipe->dentry = dentry;
> + fsnotify_create(dir, dentry);
> inode_unlock(dir);
> + return 0;
> +
> +failed:
> + pr_warn("%s() failed to create pipe %pd/%s (errno = %d)\n",
> + __func__, parent, name, err);
> return err;
> }
> EXPORT_SYMBOL_GPL(rpc_mkpipe_dentry);
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 12/17] rpc_mkpipe_dentry(): switch to start_creating()
2025-06-13 19:27 ` Jeff Layton
@ 2025-06-16 19:26 ` Al Viro
0 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-16 19:26 UTC (permalink / raw)
To: Jeff Layton
Cc: linux-fsdevel, chuck.lever, linux-nfs, neil, torvalds, trondmy
On Fri, Jun 13, 2025 at 03:27:39PM -0400, Jeff Layton wrote:
> On Fri, 2025-06-13 at 08:34 +0100, Al Viro wrote:
> > ... and make sure we set the rpc_pipe-private part of inode up before
> > attaching it to dentry.
> >
>
> "rpc_pipe->private"
Nope; fs-private, if anything. That, or rpc_pipefs-private...
> nit: subject should say "...switch to simple_start_creating()".
D'oh...
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 13/17] rpc_gssd_dummy_populate(): don't bother with rpc_populate()
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (10 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 12/17] rpc_mkpipe_dentry(): switch to start_creating() Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 14/17] rpc_pipe: expand the calls of rpc_mkdir_populate() Al Viro
` (4 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
Just have it create gssd (in root), clntXX in gssd, then info and gssd in clntXX
- all with explicit rpc_new_dir()/rpc_new_file()/rpc_mkpipe_dentry().
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 55 +++++++++----------------------------------
1 file changed, 11 insertions(+), 44 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index a52fe3bbf9dc..9051842228ec 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -997,7 +997,6 @@ enum {
RPCAUTH_nfsd4_cb,
RPCAUTH_cache,
RPCAUTH_nfsd,
- RPCAUTH_gssd,
RPCAUTH_RootEOF
};
@@ -1034,10 +1033,6 @@ static const struct rpc_filelist files[] = {
.name = "nfsd",
.mode = S_IFDIR | 0555,
},
- [RPCAUTH_gssd] = {
- .name = "gssd",
- .mode = S_IFDIR | 0555,
- },
};
/*
@@ -1097,13 +1092,6 @@ void rpc_put_sb_net(const struct net *net)
}
EXPORT_SYMBOL_GPL(rpc_put_sb_net);
-static const struct rpc_filelist gssd_dummy_clnt_dir[] = {
- [0] = {
- .name = "clntXX",
- .mode = S_IFDIR | 0555,
- },
-};
-
static ssize_t
dummy_downcall(struct file *filp, const char __user *src, size_t len)
{
@@ -1132,14 +1120,6 @@ rpc_dummy_info_show(struct seq_file *m, void *v)
}
DEFINE_SHOW_ATTRIBUTE(rpc_dummy_info);
-static const struct rpc_filelist gssd_dummy_info_file[] = {
- [0] = {
- .name = "info",
- .i_fop = &rpc_dummy_info_fops,
- .mode = S_IFREG | 0400,
- },
-};
-
/**
* rpc_gssd_dummy_populate - create a dummy gssd pipe
* @root: root of the rpc_pipefs filesystem
@@ -1151,35 +1131,22 @@ static const struct rpc_filelist gssd_dummy_info_file[] = {
static int
rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data)
{
- int ret = 0;
- struct dentry *gssd_dentry;
- struct dentry *clnt_dentry = NULL;
+ struct dentry *gssd_dentry, *clnt_dentry;
+ int err;
- /* We should never get this far if "gssd" doesn't exist */
- gssd_dentry = try_lookup_noperm(&QSTR(files[RPCAUTH_gssd].name), root);
- if (!gssd_dentry)
+ gssd_dentry = rpc_new_dir(root, "gssd", 0555, NULL);
+ if (IS_ERR(gssd_dentry))
return -ENOENT;
- ret = rpc_populate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1, NULL);
- if (ret) {
- dput(gssd_dentry);
- return ret;
- }
-
- clnt_dentry = try_lookup_noperm(&QSTR(gssd_dummy_clnt_dir[0].name),
- gssd_dentry);
- dput(gssd_dentry);
- if (!clnt_dentry)
+ clnt_dentry = rpc_new_dir(gssd_dentry, "clntXX", 0555, NULL);
+ if (IS_ERR(clnt_dentry))
return -ENOENT;
- ret = rpc_populate(clnt_dentry, gssd_dummy_info_file, 0, 1, NULL);
- if (ret) {
- dput(clnt_dentry);
- return ret;
- }
- ret = rpc_mkpipe_dentry(clnt_dentry, "gssd", NULL, pipe_data);
- dput(clnt_dentry);
- return ret;
+ err = rpc_new_file(clnt_dentry, "info", 0400,
+ &rpc_dummy_info_fops, NULL);
+ if (!err)
+ err = rpc_mkpipe_dentry(clnt_dentry, "gssd", NULL, pipe_data);
+ return err;
}
static int
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 14/17] rpc_pipe: expand the calls of rpc_mkdir_populate()
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (11 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 13/17] rpc_gssd_dummy_populate(): don't bother with rpc_populate() Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 15/17] rpc_new_dir(): the last argument is always NULL Al Viro
` (3 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
... and get rid of convoluted callbacks.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 63 +++++++++++++++----------------------------
1 file changed, 22 insertions(+), 41 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 9051842228ec..15ec770bb7fb 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -618,26 +618,6 @@ static int rpc_populate(struct dentry *parent,
return err;
}
-static struct dentry *rpc_mkdir_populate(struct dentry *parent,
- const char *name, umode_t mode, void *private,
- int (*populate)(struct dentry *, void *), void *args_populate)
-{
- struct dentry *dentry;
- int error;
-
- dentry = rpc_new_dir(parent, name, mode, private);
- if (IS_ERR(dentry))
- return dentry;
- if (populate != NULL) {
- error = populate(dentry, args_populate);
- if (error) {
- simple_recursive_removal(dentry, NULL);
- return ERR_PTR(error);
- }
- }
- return dentry;
-}
-
/**
* rpc_mkpipe_dentry - make an rpc_pipefs file for kernel<->userspace
* communication
@@ -888,13 +868,6 @@ static const struct rpc_filelist authfiles[] = {
},
};
-static int rpc_clntdir_populate(struct dentry *dentry, void *private)
-{
- return rpc_populate(dentry,
- authfiles, RPCAUTH_info, RPCAUTH_EOF,
- private);
-}
-
/**
* rpc_create_client_dir - Create a new rpc_client directory in rpc_pipefs
* @dentry: the parent of new directory
@@ -911,13 +884,19 @@ struct dentry *rpc_create_client_dir(struct dentry *dentry,
struct rpc_clnt *rpc_client)
{
struct dentry *ret;
+ int error;
- ret = rpc_mkdir_populate(dentry, name, 0555, NULL,
- rpc_clntdir_populate, rpc_client);
- if (!IS_ERR(ret)) {
- rpc_client->cl_pipedir_objects.pdh_dentry = ret;
- rpc_create_pipe_dir_objects(&rpc_client->cl_pipedir_objects);
+ ret = rpc_new_dir(dentry, name, 0555, NULL);
+ if (IS_ERR(ret))
+ return ret;
+ error = rpc_populate(ret, authfiles, RPCAUTH_info, RPCAUTH_EOF,
+ rpc_client);
+ if (unlikely(error)) {
+ simple_recursive_removal(ret, NULL);
+ return ERR_PTR(error);
}
+ rpc_client->cl_pipedir_objects.pdh_dentry = ret;
+ rpc_create_pipe_dir_objects(&rpc_client->cl_pipedir_objects);
return ret;
}
@@ -955,18 +934,20 @@ static const struct rpc_filelist cache_pipefs_files[3] = {
},
};
-static int rpc_cachedir_populate(struct dentry *dentry, void *private)
-{
- return rpc_populate(dentry,
- cache_pipefs_files, 0, 3,
- private);
-}
-
struct dentry *rpc_create_cache_dir(struct dentry *parent, const char *name,
umode_t umode, struct cache_detail *cd)
{
- return rpc_mkdir_populate(parent, name, umode, NULL,
- rpc_cachedir_populate, cd);
+ struct dentry *dentry;
+
+ dentry = rpc_new_dir(parent, name, umode, NULL);
+ if (!IS_ERR(dentry)) {
+ int error = rpc_populate(dentry, cache_pipefs_files, 0, 3, cd);
+ if (error) {
+ simple_recursive_removal(dentry, NULL);
+ return ERR_PTR(error);
+ }
+ }
+ return dentry;
}
void rpc_remove_cache_dir(struct dentry *dentry)
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 15/17] rpc_new_dir(): the last argument is always NULL
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (12 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 14/17] rpc_pipe: expand the calls of rpc_mkdir_populate() Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 16/17] rpc_create_client_dir(): don't bother with rpc_populate() Al Viro
` (2 subsequent siblings)
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
All callers except the one in rpc_populate() pass explicit NULL
there; rpc_populate() passes its last argument ('private') instead,
but in the only call of rpc_populate() that creates any subdirectories
(when creating fixed subdirectories of root) private itself is NULL.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 15ec770bb7fb..c14425d2d0d3 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -552,8 +552,7 @@ static int rpc_new_file(struct dentry *parent,
static struct dentry *rpc_new_dir(struct dentry *parent,
const char *name,
- umode_t mode,
- void *private)
+ umode_t mode)
{
struct dentry *dentry = simple_start_creating(parent, name);
struct inode *dir = parent->d_inode;
@@ -570,7 +569,6 @@ static struct dentry *rpc_new_dir(struct dentry *parent,
}
inode->i_ino = iunique(dir->i_sb, 100);
- rpc_inode_setowner(inode, private);
inc_nlink(dir);
d_instantiate(dentry, inode);
fsnotify_mkdir(dir, dentry);
@@ -603,8 +601,7 @@ static int rpc_populate(struct dentry *parent,
case S_IFDIR:
dentry = rpc_new_dir(parent,
files[i].name,
- files[i].mode,
- private);
+ files[i].mode);
if (IS_ERR(dentry)) {
err = PTR_ERR(dentry);
goto out_bad;
@@ -886,7 +883,7 @@ struct dentry *rpc_create_client_dir(struct dentry *dentry,
struct dentry *ret;
int error;
- ret = rpc_new_dir(dentry, name, 0555, NULL);
+ ret = rpc_new_dir(dentry, name, 0555);
if (IS_ERR(ret))
return ret;
error = rpc_populate(ret, authfiles, RPCAUTH_info, RPCAUTH_EOF,
@@ -939,7 +936,7 @@ struct dentry *rpc_create_cache_dir(struct dentry *parent, const char *name,
{
struct dentry *dentry;
- dentry = rpc_new_dir(parent, name, umode, NULL);
+ dentry = rpc_new_dir(parent, name, umode);
if (!IS_ERR(dentry)) {
int error = rpc_populate(dentry, cache_pipefs_files, 0, 3, cd);
if (error) {
@@ -1115,11 +1112,11 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data)
struct dentry *gssd_dentry, *clnt_dentry;
int err;
- gssd_dentry = rpc_new_dir(root, "gssd", 0555, NULL);
+ gssd_dentry = rpc_new_dir(root, "gssd", 0555);
if (IS_ERR(gssd_dentry))
return -ENOENT;
- clnt_dentry = rpc_new_dir(gssd_dentry, "clntXX", 0555, NULL);
+ clnt_dentry = rpc_new_dir(gssd_dentry, "clntXX", 0555);
if (IS_ERR(clnt_dentry))
return -ENOENT;
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 16/17] rpc_create_client_dir(): don't bother with rpc_populate()
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (13 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 15/17] rpc_new_dir(): the last argument is always NULL Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 7:34 ` [PATCH 17/17] rpc_create_client_dir(): return 0 or -E Al Viro
2025-06-13 8:19 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Amir Goldstein
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
not for a single file...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sunrpc/rpc_pipe.c | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index c14425d2d0d3..e4b53530eb1b 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -852,19 +852,6 @@ rpc_destroy_pipe_dir_objects(struct rpc_pipe_dir_head *pdh)
pdo->pdo_ops->destroy(dir, pdo);
}
-enum {
- RPCAUTH_info,
- RPCAUTH_EOF
-};
-
-static const struct rpc_filelist authfiles[] = {
- [RPCAUTH_info] = {
- .name = "info",
- .i_fop = &rpc_info_operations,
- .mode = S_IFREG | 0400,
- },
-};
-
/**
* rpc_create_client_dir - Create a new rpc_client directory in rpc_pipefs
* @dentry: the parent of new directory
@@ -881,16 +868,18 @@ struct dentry *rpc_create_client_dir(struct dentry *dentry,
struct rpc_clnt *rpc_client)
{
struct dentry *ret;
- int error;
+ int err;
ret = rpc_new_dir(dentry, name, 0555);
if (IS_ERR(ret))
return ret;
- error = rpc_populate(ret, authfiles, RPCAUTH_info, RPCAUTH_EOF,
- rpc_client);
- if (unlikely(error)) {
+ err = rpc_new_file(ret, "info", S_IFREG | 0400,
+ &rpc_info_operations, rpc_client);
+ if (err) {
+ pr_warn("%s failed to populate directory %pd\n",
+ __func__, ret);
simple_recursive_removal(ret, NULL);
- return ERR_PTR(error);
+ return ERR_PTR(err);
}
rpc_client->cl_pipedir_objects.pdh_dentry = ret;
rpc_create_pipe_dir_objects(&rpc_client->cl_pipedir_objects);
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 17/17] rpc_create_client_dir(): return 0 or -E...
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (14 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 16/17] rpc_create_client_dir(): don't bother with rpc_populate() Al Viro
@ 2025-06-13 7:34 ` Al Viro
2025-06-13 8:19 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Amir Goldstein
16 siblings, 0 replies; 25+ messages in thread
From: Al Viro @ 2025-06-13 7:34 UTC (permalink / raw)
To: linux-fsdevel; +Cc: chuck.lever, jlayton, linux-nfs, neil, torvalds, trondmy
Callers couldn't care less which dentry did we get - anything
valid is treated as success.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
include/linux/sunrpc/rpc_pipe_fs.h | 2 +-
net/sunrpc/clnt.c | 36 ++++++++++++------------------
net/sunrpc/rpc_pipe.c | 12 +++++-----
3 files changed, 21 insertions(+), 29 deletions(-)
diff --git a/include/linux/sunrpc/rpc_pipe_fs.h b/include/linux/sunrpc/rpc_pipe_fs.h
index 8cc3a5df9801..2cb406f8ff4e 100644
--- a/include/linux/sunrpc/rpc_pipe_fs.h
+++ b/include/linux/sunrpc/rpc_pipe_fs.h
@@ -98,7 +98,7 @@ static inline bool rpc_msg_is_inflight(const struct rpc_pipe_msg *msg) {
}
struct rpc_clnt;
-extern struct dentry *rpc_create_client_dir(struct dentry *, const char *, struct rpc_clnt *);
+extern int rpc_create_client_dir(struct dentry *, const char *, struct rpc_clnt *);
extern int rpc_remove_client_dir(struct rpc_clnt *);
extern void rpc_init_pipe_dir_head(struct rpc_pipe_dir_head *pdh);
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 21426c3049d3..8ca354ecfd02 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -112,47 +112,46 @@ static void rpc_clnt_remove_pipedir(struct rpc_clnt *clnt)
}
}
-static struct dentry *rpc_setup_pipedir_sb(struct super_block *sb,
+static int rpc_setup_pipedir_sb(struct super_block *sb,
struct rpc_clnt *clnt)
{
static uint32_t clntid;
const char *dir_name = clnt->cl_program->pipe_dir_name;
char name[15];
- struct dentry *dir, *dentry;
+ struct dentry *dir;
+ int err;
dir = rpc_d_lookup_sb(sb, dir_name);
if (dir == NULL) {
pr_info("RPC: pipefs directory doesn't exist: %s\n", dir_name);
- return dir;
+ return -ENOENT;
}
for (;;) {
snprintf(name, sizeof(name), "clnt%x", (unsigned int)clntid++);
name[sizeof(name) - 1] = '\0';
- dentry = rpc_create_client_dir(dir, name, clnt);
- if (!IS_ERR(dentry))
+ err = rpc_create_client_dir(dir, name, clnt);
+ if (!err)
break;
- if (dentry == ERR_PTR(-EEXIST))
+ if (err == -EEXIST)
continue;
printk(KERN_INFO "RPC: Couldn't create pipefs entry"
- " %s/%s, error %ld\n",
- dir_name, name, PTR_ERR(dentry));
+ " %s/%s, error %d\n",
+ dir_name, name, err);
break;
}
dput(dir);
- return dentry;
+ return err;
}
static int
rpc_setup_pipedir(struct super_block *pipefs_sb, struct rpc_clnt *clnt)
{
- struct dentry *dentry;
-
clnt->pipefs_sb = pipefs_sb;
if (clnt->cl_program->pipe_dir_name != NULL) {
- dentry = rpc_setup_pipedir_sb(pipefs_sb, clnt);
- if (IS_ERR(dentry))
- return PTR_ERR(dentry);
+ int err = rpc_setup_pipedir_sb(pipefs_sb, clnt);
+ if (err && err != -ENOENT)
+ return err;
}
return 0;
}
@@ -180,16 +179,9 @@ static int rpc_clnt_skip_event(struct rpc_clnt *clnt, unsigned long event)
static int __rpc_clnt_handle_event(struct rpc_clnt *clnt, unsigned long event,
struct super_block *sb)
{
- struct dentry *dentry;
-
switch (event) {
case RPC_PIPEFS_MOUNT:
- dentry = rpc_setup_pipedir_sb(sb, clnt);
- if (!dentry)
- return -ENOENT;
- if (IS_ERR(dentry))
- return PTR_ERR(dentry);
- break;
+ return rpc_setup_pipedir_sb(sb, clnt);
case RPC_PIPEFS_UMOUNT:
__rpc_clnt_remove_pipedir(clnt);
break;
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index e4b53530eb1b..a12ec709c445 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -863,27 +863,27 @@ rpc_destroy_pipe_dir_objects(struct rpc_pipe_dir_head *pdh)
* information about the client, together with any "pipes" that may
* later be created using rpc_mkpipe().
*/
-struct dentry *rpc_create_client_dir(struct dentry *dentry,
- const char *name,
- struct rpc_clnt *rpc_client)
+int rpc_create_client_dir(struct dentry *dentry,
+ const char *name,
+ struct rpc_clnt *rpc_client)
{
struct dentry *ret;
int err;
ret = rpc_new_dir(dentry, name, 0555);
if (IS_ERR(ret))
- return ret;
+ return PTR_ERR(ret);
err = rpc_new_file(ret, "info", S_IFREG | 0400,
&rpc_info_operations, rpc_client);
if (err) {
pr_warn("%s failed to populate directory %pd\n",
__func__, ret);
simple_recursive_removal(ret, NULL);
- return ERR_PTR(err);
+ return err;
}
rpc_client->cl_pipedir_objects.pdh_dentry = ret;
rpc_create_pipe_dir_objects(&rpc_client->cl_pipedir_objects);
- return ret;
+ return 0;
}
/**
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify
2025-06-13 7:34 ` [PATCH 01/17] simple_recursive_removal(): saner interaction with fsnotify Al Viro
` (15 preceding siblings ...)
2025-06-13 7:34 ` [PATCH 17/17] rpc_create_client_dir(): return 0 or -E Al Viro
@ 2025-06-13 8:19 ` Amir Goldstein
16 siblings, 0 replies; 25+ messages in thread
From: Amir Goldstein @ 2025-06-13 8:19 UTC (permalink / raw)
To: Al Viro
Cc: linux-fsdevel, chuck.lever, jlayton, linux-nfs, neil, torvalds,
trondmy
On Fri, Jun 13, 2025 at 9:35 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Make it match the real unlink(2)/rmdir(2) - notify *after* the
> operation. And use fsnotify_delete() instead of messing with
> fsnotify_unlink()/fsnotify_rmdir().
>
> Currently the only caller that cares is the one in debugfs, and
> there the order matching the normal syscalls makes more sense;
> it'll get more serious for users introduced later in the series.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Makes sense.
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
> fs/libfs.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 9ea0ecc325a8..42e226af6095 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -628,12 +628,9 @@ void simple_recursive_removal(struct dentry *dentry,
> inode_lock(inode);
> if (simple_positive(victim)) {
> d_invalidate(victim); // avoid lost mounts
> - if (d_is_dir(victim))
> - fsnotify_rmdir(inode, victim);
> - else
> - fsnotify_unlink(inode, victim);
> if (callback)
> callback(victim);
> + fsnotify_delete(inode, d_inode(victim), victim);
> dput(victim); // unpin it
> }
> if (victim == dentry) {
> --
> 2.39.5
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread