On Fri, 2024-10-11 at 08:43 +1100, NeilBrown wrote:
> 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
>
-----------------8<------------------
* @count_objects should return the number of freeable items in the cache. If
* there are no objects to free, it should return SHRINK_EMPTY, while 0 is
* returned in cases of the number of freeable items cannot be determined
* or shrinker should skip this cache for this time (e.g., their number
* is below shrinkable limit)...
-----------------8<------------------
number_cred_unused does sound like a better way to report this.
--
Jeff Layton <jlayton@xxxxxxxxxx>
