Re: [PATCH v3 -next 11/15] sunrpc: use vfs_pressure_ratio() helper

[NeilBrown]

On Fri, 11 Oct 2024, Jeff Layton wrote:
> On Thu, 2024-10-10 at 23:22 +0800, Kaixiong Yu wrote:
> > Use vfs_pressure_ratio() to simplify code.
> > 
> > Signed-off-by: Kaixiong Yu <yukaixiong@xxxxxxxxxx>
> > Reviewed-by: Kees Cook <kees@xxxxxxxxxx>
> > Acked-by: Anna Schumaker <anna.schumaker@xxxxxxxxxx>
> > ---
> >  net/sunrpc/auth.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
> > index 04534ea537c8..3d2b51d7e934 100644
> > --- a/net/sunrpc/auth.c
> > +++ b/net/sunrpc/auth.c
> > @@ -489,7 +489,7 @@ static unsigned long
> >  rpcauth_cache_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
> >  
> >  {
> > -	return number_cred_unused * sysctl_vfs_cache_pressure / 100;
> > +	return vfs_pressure_ratio(number_cred_unused);
> >  }
> >  
> >  static void
> 
> Acked-by: Jeff Layton <jlayton@xxxxxxxxxx>
> 

I realise this is a bit of a tangent, and I'm not objecting to this
patch, but I wonder what the justification is for using
vfs_cache_pressure here.  The sysctl is documented as

   This percentage value controls the tendency of the kernel to reclaim
   the memory which is used for caching of directory and inode objects.

So it can sensibly be used for dentries and inode, and for anything
directly related like the nfs access cache (which is attached to inodes)
and the nfs xattr cache.

But the sunrpc cred cache scales with the number of active users, not
the number of inodes/dentries.

So I think this should simply "return number_cred_unused;".

What do others think?

NeilBrown