Re: [PATCH 05/14] sunrpc: add a cache_notify callback

[Chuck Lever]

On 3/16/26 11:14 AM, Jeff Layton wrote:
> A later patch will be changing the kernel to send a netlink notification
> when there is a pending cache_request. Add a new cache_notify operation
> to struct cache_detail for this purpose.
> 
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
>  include/linux/sunrpc/cache.h | 3 +++
>  net/sunrpc/cache.c           | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
> index 80a3f17731d8fbc1c5252a830b202016faa41a18..c358151c23950ab48e83991c6138bb7d0e049ace 100644
> --- a/include/linux/sunrpc/cache.h
> +++ b/include/linux/sunrpc/cache.h
> @@ -80,6 +80,9 @@ struct cache_detail {
>  	int			(*cache_upcall)(struct cache_detail *,
>  						struct cache_head *);
>  
> +	int			(*cache_notify)(struct cache_detail *cd,
> +						struct cache_head *h);
> +
>  	void			(*cache_request)(struct cache_detail *cd,
>  						 struct cache_head *ch,
>  						 char **bpp, int *blen);
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 7081b6e0e9090d2ba7da68c1f36b4c170fb228cb..819f12add8f26562fdc6aaa200f55dec0180bfbc 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -33,6 +33,7 @@
>  #include <linux/sunrpc/cache.h>
>  #include <linux/sunrpc/stats.h>
>  #include <linux/sunrpc/rpc_pipe_fs.h>
> +#include <net/genetlink.h>

Nit: Adding this here might be premature. Should it be moved to a
subsequent patch in this series?


>  #include <trace/events/sunrpc.h>
>  
>  #include "netns.h"
> @@ -1239,6 +1240,8 @@ static int cache_do_upcall(struct cache_detail *detail, struct cache_head *h)
>  		/* Lost a race, no longer PENDING, so don't enqueue */
>  		ret = -EAGAIN;
>  	spin_unlock(&detail->queue_lock);
> +	if (detail->cache_notify)
> +		detail->cache_notify(detail, h);
>  	wake_up(&detail->queue_wait);
>  	if (ret == -EAGAIN) {
>  		kfree(buf);
> 

When ret == -EAGAIN, the CACHE_PENDING bit was already cleared by
another thread that won the race. Calling cache_notify in this case
sends a netlink notification for an entry that is no longer pending. The
winning thread already queued the request and will trigger its own
notification. If you agree this observation is not a false positive, a
possible fix might be:

  spin_unlock(&detail->queue_lock);
  if (ret != -EAGAIN && detail->cache_notify)
      detail->cache_notify(detail, h);
  wake_up(&detail->queue_wait);


-- 
Chuck Lever