Re: [PATCH v2 2/2] nfs: update inode ctime after removexattr operation

[Olga Kornievskaia]

On Tue, Mar 24, 2026 at 1:32 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> xfstest generic/728 fails with delegated timestamps. The client does a
> removexattr and then a stat to test the ctime, which doesn't change. The
> stat() doesn't trigger a GETATTR because of the delegated timestamps, so
> it relies on the cached ctime, which is wrong.
>
> The setxattr compound has a trailing GETATTR, which ensures that its
> ctime gets updated. Follow the same strategy with removexattr.

This approach relies on the fact that the server the serves delegated
attributes would update change_attr on operations which might now
necessarily happen (ie, linux server does not update change_attribute
on writes or clone). I propose an alternative fix for the failing
generic/728.

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 7b3ca68fb4bb..ede1835a45b3 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -1389,7 +1389,13 @@ static int _nfs42_proc_removexattr(struct inode
*inode, const char *name)
            &res.seq_res, 1);
        trace_nfs4_removexattr(inode, name, ret);
        if (!ret)
-               nfs4_update_changeattr(inode, &res.cinfo, timestamp, 0);
+               if (nfs_have_delegated_attributes(inode)) {
+                       nfs_update_delegated_mtime(inode);
+                       spin_lock(&inode->i_lock);
+                       nfs_set_cache_invalid(inode, NFS_INO_INVALID_BLOCKS);
+                       spin_unlock(&inode->i_lock);
+               } else
+                       nfs4_update_changeattr(inode, &res.cinfo, timestamp, 0);

        return ret;
 }

>
> Fixes: 3e1f02123fba ("NFSv4.2: add client side XDR handling for extended attributes")
> Reported-by: Olga Kornievskaia <aglo@xxxxxxxxx>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
>  fs/nfs/nfs42proc.c      | 18 ++++++++++++++++--
>  fs/nfs/nfs42xdr.c       | 10 ++++++++--
>  include/linux/nfs_xdr.h |  3 +++
>  3 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index 7b3ca68fb4bb3bee293f8621e5ed5fa596f5da3f..7e5c1172fc11c9d5a55b3621977ac83bb98f7c20 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -1372,11 +1372,15 @@ int nfs42_proc_clone(struct file *src_f, struct file *dst_f,
>  static int _nfs42_proc_removexattr(struct inode *inode, const char *name)
>  {
>         struct nfs_server *server = NFS_SERVER(inode);
> +       __u32 bitmask[NFS_BITMASK_SZ];
>         struct nfs42_removexattrargs args = {
>                 .fh = NFS_FH(inode),
> +               .bitmask = bitmask,
>                 .xattr_name = name,
>         };
> -       struct nfs42_removexattrres res;
> +       struct nfs42_removexattrres res = {
> +               .server = server,
> +       };
>         struct rpc_message msg = {
>                 .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_REMOVEXATTR],
>                 .rpc_argp = &args,
> @@ -1385,12 +1389,22 @@ static int _nfs42_proc_removexattr(struct inode *inode, const char *name)
>         int ret;
>         unsigned long timestamp = jiffies;
>
> +       res.fattr = nfs_alloc_fattr();
> +       if (!res.fattr)
> +               return -ENOMEM;
> +
> +       nfs4_bitmask_set(bitmask, server->cache_consistency_bitmask,
> +                        inode, NFS_INO_INVALID_CHANGE);
> +
>         ret = nfs4_call_sync(server->client, server, &msg, &args.seq_args,
>             &res.seq_res, 1);
>         trace_nfs4_removexattr(inode, name, ret);
> -       if (!ret)
> +       if (!ret) {
>                 nfs4_update_changeattr(inode, &res.cinfo, timestamp, 0);
> +               ret = nfs_post_op_update_inode(inode, res.fattr);
> +       }
>
> +       kfree(res.fattr);
>         return ret;
>  }
>
> diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> index 5c7452ce6e8ac94bd24bc3a33d4479d458a29907..ec105c62f721cfe01bfc60f5981396958084d627 100644
> --- a/fs/nfs/nfs42xdr.c
> +++ b/fs/nfs/nfs42xdr.c
> @@ -263,11 +263,13 @@
>  #define NFS4_enc_removexattr_sz                (compound_encode_hdr_maxsz + \
>                                          encode_sequence_maxsz + \
>                                          encode_putfh_maxsz + \
> -                                        encode_removexattr_maxsz)
> +                                        encode_removexattr_maxsz + \
> +                                        encode_getattr_maxsz)
>  #define NFS4_dec_removexattr_sz                (compound_decode_hdr_maxsz + \
>                                          decode_sequence_maxsz + \
>                                          decode_putfh_maxsz + \
> -                                        decode_removexattr_maxsz)
> +                                        decode_removexattr_maxsz + \
> +                                        decode_getattr_maxsz)
>
>  /*
>   * These values specify the maximum amount of data that is not
> @@ -869,6 +871,7 @@ static void nfs4_xdr_enc_removexattr(struct rpc_rqst *req,
>         encode_sequence(xdr, &args->seq_args, &hdr);
>         encode_putfh(xdr, args->fh, &hdr);
>         encode_removexattr(xdr, args->xattr_name, &hdr);
> +       encode_getfattr(xdr, args->bitmask, &hdr);
>         encode_nops(&hdr);
>  }
>
> @@ -1818,6 +1821,9 @@ static int nfs4_xdr_dec_removexattr(struct rpc_rqst *req,
>                 goto out;
>
>         status = decode_removexattr(xdr, &res->cinfo);
> +       if (status)
> +               goto out;
> +       status = decode_getfattr(xdr, res->fattr, res->server);
>  out:
>         return status;
>  }
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index ff1f12aa73d27b6fd874baf7019dd03195fc36e5..fcbd21b5685f46136a210c8e11c20a54d6ed9dad 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -1611,12 +1611,15 @@ struct nfs42_listxattrsres {
>  struct nfs42_removexattrargs {
>         struct nfs4_sequence_args       seq_args;
>         struct nfs_fh                   *fh;
> +       const u32                       *bitmask;
>         const char                      *xattr_name;
>  };
>
>  struct nfs42_removexattrres {
>         struct nfs4_sequence_res        seq_res;
>         struct nfs4_change_info         cinfo;
> +       struct nfs_fattr                *fattr;
> +       const struct nfs_server         *server;
>  };
>
>  #endif /* CONFIG_NFS_V4_2 */
>
> --
> 2.53.0
>