Re: [PATCH v3 -next 11/15] sunrpc: use vfs_pressure_ratio() helper
yukaixiong <[email protected]>
| Newsgroups | gmane.linux.ports.sh.devel,gmane.linux.kernel,gmane.linux.file-systems,gmane.linux.kernel.mm,gmane.linux.nfs,gmane.linux.network,gmane.linux.kernel.lsm |
|---|---|
| Message-ID | <[email protected]> |
On 2024/10/11 20:38, Jeff Layton wrote: > 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 <[email protected]> >>>> Reviewed-by: Kees Cook <[email protected]> >>>> Acked-by: Anna Schumaker <[email protected]> >>>> --- >>>> 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 <[email protected]> >>> >> 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. Thanks, I'll take NeilBrown's advice.