On Mon, 2025-02-17 at 16:30 +1100, NeilBrown wrote:
> mkdir now allows a different dentry to be returned which is sometimes
> relevant for nfs.
>
> This patch changes the nfs_rpc_ops mkdir op to return a dentry, and
> passes that back to the caller.
>
> The mkdir nfs_rpc_op will return NULL if the original dentry should be
> used. This matches the mkdir inode_operation.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> fs/nfs/dir.c | 16 ++++++---------
> fs/nfs/nfs3proc.c | 7 ++++---
> fs/nfs/nfs4proc.c | 43 +++++++++++++++++++++++++++++------------
> fs/nfs/proc.c | 12 ++++++++----
> include/linux/nfs_xdr.h | 2 +-
> 5 files changed, 50 insertions(+), 30 deletions(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 5700f73d48bc..afb1afe0af8e 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -2426,7 +2426,7 @@ struct dentry *nfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> struct dentry *dentry, umode_t mode)
> {
> struct iattr attr;
> - int error;
> + struct dentry *ret;
>
> dfprintk(VFS, "NFS: mkdir(%s/%lu), %pd\n",
> dir->i_sb->s_id, dir->i_ino, dentry);
> @@ -2435,15 +2435,11 @@ struct dentry *nfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> attr.ia_mode = mode | S_IFDIR;
>
> trace_nfs_mkdir_enter(dir, dentry);
> - error = NFS_PROTO(dir)->mkdir(dir, dentry, &attr);
> - trace_nfs_mkdir_exit(dir, dentry, error);
> - if (error != 0)
> - goto out_err;
> - /* FIXME - ->mkdir might have used an alternate dentry */
> - return NULL;
> -out_err:
> - d_drop(dentry);
> - return ERR_PTR(error);
> + ret = NFS_PROTO(dir)->mkdir(dir, dentry, &attr);
> + trace_nfs_mkdir_exit(dir, dentry, PTR_ERR_OR_ZERO(ret));
FWIW, this should be fine even since the old dentry should have the
same d_name as the one in ret, if one is returned.
> + if (IS_ERR(ret))
> + d_drop(dentry);
> + return ret;
> }
> EXPORT_SYMBOL_GPL(nfs_mkdir);
>
> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
> index 0c3bc98cd999..cccb12ba19dc 100644
> --- a/fs/nfs/nfs3proc.c
> +++ b/fs/nfs/nfs3proc.c
> @@ -578,7 +578,7 @@ nfs3_proc_symlink(struct inode *dir, struct dentry *dentry, struct folio *folio,
> return status;
> }
>
> -static int
> +static struct dentry *
> nfs3_proc_mkdir(struct inode *dir, struct dentry *dentry, struct iattr *sattr)
> {
> struct posix_acl *default_acl, *acl;
> @@ -613,14 +613,15 @@ nfs3_proc_mkdir(struct inode *dir, struct dentry *dentry, struct iattr *sattr)
>
> status = nfs3_proc_setacls(d_inode(dentry), acl, default_acl);
>
> - dput(d_alias);
> out_release_acls:
> posix_acl_release(acl);
> posix_acl_release(default_acl);
> out:
> nfs3_free_createdata(data);
> dprintk("NFS reply mkdir: %d\n", status);
> - return status;
> + if (status)
> + return ERR_PTR(status);
> + return d_alias;
> }
>
> static int
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index df9669d4ded7..164c9f3f36c8 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5135,9 +5135,6 @@ static int nfs4_do_create(struct inode *dir, struct dentry *dentry, struct nfs4_
> &data->arg.seq_args, &data->res.seq_res, 1);
> if (status == 0) {
> spin_lock(&dir->i_lock);
> - /* Creating a directory bumps nlink in the parent */
> - if (data->arg.ftype == NF4DIR)
> - nfs4_inc_nlink_locked(dir);
> nfs4_update_changeattr_locked(dir, &data->res.dir_cinfo,
> data->res.fattr->time_start,
> NFS_INO_INVALID_DATA);
> @@ -5147,6 +5144,25 @@ static int nfs4_do_create(struct inode *dir, struct dentry *dentry, struct nfs4_
> return status;
> }
>
> +static struct dentry *nfs4_do_mkdir(struct inode *dir, struct dentry *dentry,
> + struct nfs4_createdata *data)
> +{
> + int status = nfs4_call_sync(NFS_SERVER(dir)->client, NFS_SERVER(dir), &data->msg,
> + &data->arg.seq_args, &data->res.seq_res, 1);
> +
> + if (status)
> + return ERR_PTR(status);
> +
> + spin_lock(&dir->i_lock);
> + /* Creating a directory bumps nlink in the parent */
> + nfs4_inc_nlink_locked(dir);
> + nfs4_update_changeattr_locked(dir, &data->res.dir_cinfo,
> + data->res.fattr->time_start,
> + NFS_INO_INVALID_DATA);
> + spin_unlock(&dir->i_lock);
> + return nfs_add_or_obtain(dentry, data->res.fh, data->res.fattr);
> +}
> +
> static void nfs4_free_createdata(struct nfs4_createdata *data)
> {
> nfs4_label_free(data->fattr.label);
> @@ -5203,32 +5219,34 @@ static int nfs4_proc_symlink(struct inode *dir, struct dentry *dentry,
> return err;
> }
>
> -static int _nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
> - struct iattr *sattr, struct nfs4_label *label)
> +static struct dentry *_nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
> + struct iattr *sattr,
> + struct nfs4_label *label)
> {
> struct nfs4_createdata *data;
> - int status = -ENOMEM;
> + struct dentry *ret = ERR_PTR(-ENOMEM);
>
> data = nfs4_alloc_createdata(dir, &dentry->d_name, sattr, NF4DIR);
> if (data == NULL)
> goto out;
>
> data->arg.label = label;
> - status = nfs4_do_create(dir, dentry, data);
> + ret = nfs4_do_mkdir(dir, dentry, data);
>
> nfs4_free_createdata(data);
> out:
> - return status;
> + return ret;
> }
>
> -static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
> - struct iattr *sattr)
> +static struct dentry *nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
> + struct iattr *sattr)
> {
> struct nfs_server *server = NFS_SERVER(dir);
> struct nfs4_exception exception = {
> .interruptible = true,
> };
> struct nfs4_label l, *label;
> + struct dentry *alias;
> int err;
>
> label = nfs4_label_init_security(dir, dentry, sattr, &l);
> @@ -5236,14 +5254,15 @@ static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
> if (!(server->attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK))
> sattr->ia_mode &= ~current_umask();
> do {
> - err = _nfs4_proc_mkdir(dir, dentry, sattr, label);
> + alias = _nfs4_proc_mkdir(dir, dentry, sattr, label);
> + err = PTR_ERR_OR_ZERO(alias);
> trace_nfs4_mkdir(dir, &dentry->d_name, err);
> err = nfs4_handle_exception(NFS_SERVER(dir), err,
> &exception);
> } while (exception.retry);
> nfs4_label_release_security(label);
>
> - return err;
> + return alias;
> }
>
> static int _nfs4_proc_readdir(struct nfs_readdir_arg *nr_arg,
> diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
> index 77920a2e3cef..63e71310b9f6 100644
> --- a/fs/nfs/proc.c
> +++ b/fs/nfs/proc.c
> @@ -446,13 +446,14 @@ nfs_proc_symlink(struct inode *dir, struct dentry *dentry, struct folio *folio,
> return status;
> }
>
> -static int
> +static struct dentry *
> nfs_proc_mkdir(struct inode *dir, struct dentry *dentry, struct iattr *sattr)
> {
> struct nfs_createdata *data;
> struct rpc_message msg = {
> .rpc_proc = &nfs_procedures[NFSPROC_MKDIR],
> };
> + struct dentry *alias = NULL;
> int status = -ENOMEM;
>
> dprintk("NFS call mkdir %pd\n", dentry);
> @@ -464,12 +465,15 @@ nfs_proc_mkdir(struct inode *dir, struct dentry *dentry, struct iattr *sattr)
>
> status = rpc_call_sync(NFS_CLIENT(dir), &msg, 0);
> nfs_mark_for_revalidate(dir);
> - if (status == 0)
> - status = nfs_instantiate(dentry, data->res.fh, data->res.fattr);
> + if (status == 0) {
> + alias = nfs_add_or_obtain(dentry, data->res.fh, data->res.fattr);
> + status = PTR_ERR_OR_ZERO(alias);
> + } else
> + alias = ERR_PTR(status);
> nfs_free_createdata(data);
> out:
> dprintk("NFS reply mkdir: %d\n", status);
> - return status;
> + return alias;
> }
>
> static int
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 9155a6ffc370..d66c61cbbd1d 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -1802,7 +1802,7 @@ struct nfs_rpc_ops {
> int (*link) (struct inode *, struct inode *, const struct qstr *);
> int (*symlink) (struct inode *, struct dentry *, struct folio *,
> unsigned int, struct iattr *);
> - int (*mkdir) (struct inode *, struct dentry *, struct iattr *);
> + struct dentry *(*mkdir) (struct inode *, struct dentry *, struct iattr *);
> int (*rmdir) (struct inode *, const struct qstr *);
> int (*readdir) (struct nfs_readdir_arg *, struct nfs_readdir_res *);
> int (*mknod) (struct inode *, struct dentry *, struct iattr *,
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
