Re: [PATCH v2 2/4] NFS: Add support for CB_NOTIFY4_REMOVE_ENTRY

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2026-08-25 at 11:02 -0400, Anna Schumaker wrote:
> From: Anna Schumaker <anna.schumaker@oracle.com>
> 
> When the server tells us that a directory entry has been removed then we
> need to take that as an indication that our knowledge of the directory
> has changed and needs to be refreshed.
> 
> Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
> Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
> ---
> v2:
>   * Initialize 'res' to 0 in nfs4_callback_notify()
>   * Add missing error handling
>   * Remove nfs4_cb_notify_lookup()
> ---
>  fs/nfs/callback.h      | 16 ++++++++++-
>  fs/nfs/callback_proc.c | 63 +++++++++++++++++++++++++++++++++++++++++-
>  fs/nfs/callback_xdr.c  | 62 +++++++++++++++++++++++++++++++++++++++++
>  fs/nfs/nfs4xdr.c       |  3 +-
>  include/linux/nfs4.h   |  1 +
>  5 files changed, 142 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfs/callback.h b/fs/nfs/callback.h
> index f7cc5b6931bf..3740c999bb82 100644
> --- a/fs/nfs/callback.h
> +++ b/fs/nfs/callback.h
> @@ -143,8 +143,22 @@ struct cb_layoutrecallargs {
>  extern __be32 nfs4_callback_layoutrecall(void *argp, void *resp,
>  		struct cb_process_state *cps);
>  
> +struct cb_notify_entry {
> +	u32			ne_namelen;
> +	const char		*ne_name;
> +	struct nfs_fattr	ne_attrs;
> +};
> +
> +struct cb_notify_remove {
> +	struct cb_notify_entry	nrm_old_entry;
> +	u64			nrm_old_entry_cookie;
> +};
> +
>  struct cb_notify_changes {
> -	u32 notify_mask;
> +	u32	notify_mask;
> +	union {
> +		struct cb_notify_remove notify_remove;
> +	};
>  };
>  
>  struct cb_notifyargs {
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index 01600d75f2d9..d8ad3cd792df 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -9,6 +9,7 @@
>  
>  #include <linux/errno.h>
>  #include <linux/math.h>
> +#include <linux/namei.h>
>  #include <linux/nfs4.h>
>  #include <linux/nfs_fs.h>
>  #include <linux/slab.h>
> @@ -369,13 +370,73 @@ static void pnfs_recall_all_layouts(struct nfs_client *clp,
>  	do_callback_layoutrecall(clp, &args, cps);
>  }
>  
> +static __be32 nfs4_cb_notify_remove(struct cb_process_state *cps,
> +				    struct dentry *parent,
> +				    struct cb_notify_remove *cb_remove)
> +{
> +	struct qstr filename = QSTR_INIT(cb_remove->nrm_old_entry.ne_name,
> +					 cb_remove->nrm_old_entry.ne_namelen);
> +	struct dentry *child;
> +
> +	filename.hash = full_name_hash(parent, filename.name, filename.len);
> +	child = d_lookup(parent, &filename);
> +	if (IS_ERR_OR_NULL(child))
> +		return htonl(NFS4ERR_BADHANDLE);
> +
> +	d_drop(child);

Should this instead make this a negative dentry?

I think this will just drop the existing dentry and it'll trigger a
lookup next time we end up trying to hit it.

> +	dput(child);
> +
> +	nfs_set_cache_invalid(parent->d_inode, NFS_INO_INVALID_DATA);
> +	return 0;
> +}
> +
>  __be32 nfs4_callback_notify(void *argp, void *resp,
>  			    struct cb_process_state *cps)
>  {
>  	struct cb_notifyargs *args = argp;
> +	struct dentry *parent;
> +	struct inode *inode;
> +	unsigned int i;
> +	__be32 res = 0;
>  
> +	if (!cps->clp) {
> +		res = htonl(NFS4ERR_OP_NOT_IN_SESSION);
> +		goto out;
> +	}
> +
> +	inode = nfs_delegation_find_inode(cps->clp, &args->cna_fh);
> +	if (IS_ERR(inode)) {
> +		res = htonl(NFS4ERR_BADHANDLE);
> +		goto out;
> +	}
> +	parent = d_find_alias(inode);
> +	if (!parent)
> +		goto out_iput;
> +
> +	for (i = 0; i < args->cna_n_changes; i++) {
> +		struct cb_notify_changes *change = &args->cna_changes[i];
> +
> +		switch (change->notify_mask) {
> +		case CB_NOTIFY4_REMOVE_ENTRY:
> +			res = nfs4_cb_notify_remove(cps, parent,
> +						    &change->notify_remove);
> +			break;
> +		default:
> +			res = htonl(NFS4ERR_NOTSUPP);
> +			goto out_dput;
> +		}
> +
> +		if (unlikely(res != 0))
> +			break;
> +	}
> +
> +out_dput:
> +	dput(parent);
> +out_iput:
> +	nfs_iput_and_deactive(inode);
> +out:
>  	kfree(args->cna_changes);
> -	return 0;
> +	return res;
>  }
>  
>  __be32 nfs4_callback_devicenotify(void *argp, void *resp,
> diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
> index 8d822a3ee248..612d215ac66e 100644
> --- a/fs/nfs/callback_xdr.c
> +++ b/fs/nfs/callback_xdr.c
> @@ -252,6 +252,50 @@ static __be32 decode_layoutrecall_args(struct svc_rqst *rqstp,
>  	return 0;
>  }
>  
> +static __be32 decode_notify_entry(struct xdr_stream *xdr,
> +				  struct cb_notify_entry *args)
> +{
> +	uint32_t bitmap[3] = {0};
> +	__be32 status;
> +	u32 attrlen;
> +	__be32 *p;
> +
> +	status = decode_string(xdr, &args->ne_namelen, &args->ne_name,
> +				NFS4_OPAQUE_LIMIT);
> +	if (unlikely(status != 0))
> +		return status;
> +
> +	status = decode_bitmap(xdr, bitmap);
> +	if (unlikely(status != 0))
> +		return status;
> +
> +	p = xdr_inline_decode(xdr, 4);
> +	if (unlikely(!p))
> +		return htonl(NFS4ERR_BADXDR);
> +
> +	attrlen = be32_to_cpup(p);
> +	if (attrlen != 0)
> +		return htonl(NFS4ERR_BADXDR);
> +	return 0;
> +}
> +
> +static __be32 decode_notify_remove(struct xdr_stream *xdr,
> +				   struct cb_notify_remove *args)
> +{
> +	__be32 status;
> +	__be32 *p;
> +
> +	status = decode_notify_entry(xdr, &args->nrm_old_entry);
> +	if (unlikely(status != 0))
> +		return status;
> +
> +	p = xdr_inline_decode(xdr, 8);
> +	if (unlikely(!p))
> +		return htonl(NFS4ERR_BADXDR);
> +	xdr_decode_hyper(p, &args->nrm_old_entry_cookie);
> +	return 0;
> +}
> +
>  static
>  __be32 decode_notify_args(struct svc_rqst *rqstp,
>  			  struct xdr_stream *xdr,
> @@ -289,6 +333,24 @@ __be32 decode_notify_args(struct svc_rqst *rqstp,
>  						     &change->notify_mask, 1);
>  		if (unlikely(res < 0))
>  			goto err;
> +
> +		/* Decode opaque size */
> +		p = xdr_inline_decode(xdr, 4);
> +		if (unlikely(!p))
> +			goto err;
> +		res = ntohl(*p);
> +
> +		switch (change->notify_mask) {
> +		case CB_NOTIFY4_REMOVE_ENTRY:
> +			status = decode_notify_remove(xdr,
> +						      &change->notify_remove);
> +			break;
> +		default:
> +			goto err;
> +		}
> +
> +		if (unlikely(status != 0))
> +			goto err;
>  	}
>  
>  	return 0;
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 5c51691fff03..e275fbfd395e 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -2012,7 +2012,8 @@ static void
>  encode_get_dir_delegation(struct xdr_stream *xdr, struct compound_hdr *hdr)
>  {
>  	struct timespec64 ts = { 0, 0 };
> -	u32 notifications[1] = { CB_NOTIFY4_GFLAG_EXTEND };
> +	u32 notifications[1] = { CB_NOTIFY4_REMOVE_ENTRY |
> +				 CB_NOTIFY4_GFLAG_EXTEND };
>  	u32 attributes[1] = { 0 };
>  	__be32 *p;
>  
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index 2e33f2ffdc31..8de3ea283767 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -492,6 +492,7 @@ enum {
>  #define THRESHOLD_WR_IO                 (1UL << 3)
>  
>  /* Directory Delegation / CB_NOTIFY bits */
> +#define CB_NOTIFY4_REMOVE_ENTRY		(1UL << 2)
>  #define CB_NOTIFY4_GFLAG_EXTEND		(1UL << 6)
>  
>  #define NFSPROC4_NULL 0

-- 
Jeff Layton <jlayton@kernel.org>