2023-06-14 07:33:21

by Qi Zheng

[permalink] [raw]
Subject: [PATCH] NFSv4.2: fix wrong shrinker_id

From: Qi Zheng <[email protected]>

Currently, the list_lru::shrinker_id corresponding to the nfs4_xattr
shrinkers is wrong:

>>> prog["nfs4_xattr_cache_lru"].shrinker_id
(int)0
>>> prog["nfs4_xattr_entry_lru"].shrinker_id
(int)0
>>> prog["nfs4_xattr_large_entry_lru"].shrinker_id
(int)0
>>> prog["nfs4_xattr_cache_shrinker"].id
(int)18
>>> prog["nfs4_xattr_entry_shrinker"].id
(int)19
>>> prog["nfs4_xattr_large_entry_shrinker"].id
(int)20

This is not what we expect, which will cause these shrinkers
not to be found in shrink_slab_memcg().

We should assign shrinker::id before calling list_lru_init_memcg(),
so that the corresponding list_lru::shrinker_id will be assigned
the correct value like below:

>>> prog["nfs4_xattr_cache_lru"].shrinker_id
(int)16
>>> prog["nfs4_xattr_entry_lru"].shrinker_id
(int)17
>>> prog["nfs4_xattr_large_entry_lru"].shrinker_id
(int)18
>>> prog["nfs4_xattr_cache_shrinker"].id
(int)16
>>> prog["nfs4_xattr_entry_shrinker"].id
(int)17
>>> prog["nfs4_xattr_large_entry_shrinker"].id
(int)18

So just do it.

Fixes: 95ad37f90c33 ("NFSv4.2: add client side xattr caching.")
Signed-off-by: Qi Zheng <[email protected]>
---
fs/nfs/nfs42xattr.c | 83 ++++++++++++++++++++++++++-------------------
1 file changed, 48 insertions(+), 35 deletions(-)

diff --git a/fs/nfs/nfs42xattr.c b/fs/nfs/nfs42xattr.c
index 76ae11834206..e3dab0131e4c 100644
--- a/fs/nfs/nfs42xattr.c
+++ b/fs/nfs/nfs42xattr.c
@@ -991,6 +991,33 @@ static void nfs4_xattr_cache_init_once(void *p)
INIT_LIST_HEAD(&cache->dispose);
}

+static int nfs4_xattr_shrinker_init(struct shrinker *shrinker,
+ struct list_lru *lru, const char *name)
+{
+ int ret = 0;
+
+ ret = prealloc_shrinker(shrinker, name);
+ if (ret)
+ return ret;
+
+ ret = list_lru_init_memcg(lru, shrinker);
+ if (ret) {
+ free_prealloced_shrinker(shrinker);
+ return ret;
+ }
+
+ register_shrinker_prepared(shrinker);
+
+ return 0;
+}
+
+static void nfs4_xattr_shrinker_destroy(struct shrinker *shrinker,
+ struct list_lru *lru)
+{
+ unregister_shrinker(shrinker);
+ list_lru_destroy(lru);
+}
+
int __init nfs4_xattr_cache_init(void)
{
int ret = 0;
@@ -1002,44 +1029,30 @@ int __init nfs4_xattr_cache_init(void)
if (nfs4_xattr_cache_cachep == NULL)
return -ENOMEM;

- ret = list_lru_init_memcg(&nfs4_xattr_large_entry_lru,
- &nfs4_xattr_large_entry_shrinker);
- if (ret)
- goto out4;
-
- ret = list_lru_init_memcg(&nfs4_xattr_entry_lru,
- &nfs4_xattr_entry_shrinker);
- if (ret)
- goto out3;
-
- ret = list_lru_init_memcg(&nfs4_xattr_cache_lru,
- &nfs4_xattr_cache_shrinker);
- if (ret)
- goto out2;
-
- ret = register_shrinker(&nfs4_xattr_cache_shrinker, "nfs-xattr_cache");
+ ret = nfs4_xattr_shrinker_init(&nfs4_xattr_cache_shrinker,
+ &nfs4_xattr_cache_lru,
+ "nfs-xattr_cache");
if (ret)
goto out1;

- ret = register_shrinker(&nfs4_xattr_entry_shrinker, "nfs-xattr_entry");
+ ret = nfs4_xattr_shrinker_init(&nfs4_xattr_entry_shrinker,
+ &nfs4_xattr_entry_lru,
+ "nfs-xattr_entry");
if (ret)
- goto out;
+ goto out2;

- ret = register_shrinker(&nfs4_xattr_large_entry_shrinker,
- "nfs-xattr_large_entry");
+ ret = nfs4_xattr_shrinker_init(&nfs4_xattr_large_entry_shrinker,
+ &nfs4_xattr_large_entry_lru,
+ "nfs-xattr_large_entry");
if (!ret)
return 0;

- unregister_shrinker(&nfs4_xattr_entry_shrinker);
-out:
- unregister_shrinker(&nfs4_xattr_cache_shrinker);
-out1:
- list_lru_destroy(&nfs4_xattr_cache_lru);
+ nfs4_xattr_shrinker_destroy(&nfs4_xattr_entry_shrinker,
+ &nfs4_xattr_entry_lru);
out2:
- list_lru_destroy(&nfs4_xattr_entry_lru);
-out3:
- list_lru_destroy(&nfs4_xattr_large_entry_lru);
-out4:
+ nfs4_xattr_shrinker_destroy(&nfs4_xattr_cache_shrinker,
+ &nfs4_xattr_cache_lru);
+out1:
kmem_cache_destroy(nfs4_xattr_cache_cachep);

return ret;
@@ -1047,11 +1060,11 @@ int __init nfs4_xattr_cache_init(void)

void nfs4_xattr_cache_exit(void)
{
- unregister_shrinker(&nfs4_xattr_large_entry_shrinker);
- unregister_shrinker(&nfs4_xattr_entry_shrinker);
- unregister_shrinker(&nfs4_xattr_cache_shrinker);
- list_lru_destroy(&nfs4_xattr_large_entry_lru);
- list_lru_destroy(&nfs4_xattr_entry_lru);
- list_lru_destroy(&nfs4_xattr_cache_lru);
+ nfs4_xattr_shrinker_destroy(&nfs4_xattr_large_entry_shrinker,
+ &nfs4_xattr_large_entry_lru);
+ nfs4_xattr_shrinker_destroy(&nfs4_xattr_entry_shrinker,
+ &nfs4_xattr_entry_lru);
+ nfs4_xattr_shrinker_destroy(&nfs4_xattr_cache_shrinker,
+ &nfs4_xattr_cache_lru);
kmem_cache_destroy(nfs4_xattr_cache_cachep);
}
--
2.30.2



2023-06-14 18:13:00

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] NFSv4.2: fix wrong shrinker_id

Hi Qi,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on trondmy-nfs/linux-next v6.4-rc6 next-20230614]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/NFSv4-2-fix-wrong-shrinker_id/20230614-152853
base: linus/master
patch link: https://lore.kernel.org/r/20230614072443.3264264-1-qi.zheng%40linux.dev
patch subject: [PATCH] NFSv4.2: fix wrong shrinker_id
config: i386-debian-10.3 (https://download.01.org/0day-ci/archive/20230615/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build):
git checkout linus/master
b4 shazam https://lore.kernel.org/r/[email protected]
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "free_prealloced_shrinker" [fs/nfs/nfsv4.ko] undefined!
>> ERROR: modpost: "register_shrinker_prepared" [fs/nfs/nfsv4.ko] undefined!
>> ERROR: modpost: "prealloc_shrinker" [fs/nfs/nfsv4.ko] undefined!

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-06-15 02:59:10

by Qi Zheng

[permalink] [raw]
Subject: Re: [PATCH] NFSv4.2: fix wrong shrinker_id



On 2023/6/15 02:06, kernel test robot wrote:
> Hi Qi,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on trondmy-nfs/linux-next v6.4-rc6 next-20230614]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/NFSv4-2-fix-wrong-shrinker_id/20230614-152853
> base: linus/master
> patch link: https://lore.kernel.org/r/20230614072443.3264264-1-qi.zheng%40linux.dev
> patch subject: [PATCH] NFSv4.2: fix wrong shrinker_id
> config: i386-debian-10.3 (https://download.01.org/0day-ci/archive/20230615/[email protected]/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build):
> git checkout linus/master
> b4 shazam https://lore.kernel.org/r/[email protected]
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> make W=1 O=build_dir ARCH=i386 olddefconfig
> make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All errors (new ones prefixed by >>, old ones prefixed by <<):
>
>>> ERROR: modpost: "free_prealloced_shrinker" [fs/nfs/nfsv4.ko] undefined!
>>> ERROR: modpost: "register_shrinker_prepared" [fs/nfs/nfsv4.ko] undefined!
>>> ERROR: modpost: "prealloc_shrinker" [fs/nfs/nfsv4.ko] undefined!

Ah, these three functions need to be exported. Will fix it in the v2.

>

2023-06-15 11:07:41

by Qi Zheng

[permalink] [raw]
Subject: Re: [PATCH] NFSv4.2: fix wrong shrinker_id



On 2023/6/15 10:46, Qi Zheng wrote:
>
>
> On 2023/6/15 02:06, kernel test robot wrote:
>> Hi Qi,
>>
>> kernel test robot noticed the following build errors:
>>
>> [auto build test ERROR on linus/master]
>> [also build test ERROR on trondmy-nfs/linux-next v6.4-rc6 next-20230614]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>
>> url:
>> https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/NFSv4-2-fix-wrong-shrinker_id/20230614-152853
>> base:   linus/master
>> patch link:
>> https://lore.kernel.org/r/20230614072443.3264264-1-qi.zheng%40linux.dev
>> patch subject: [PATCH] NFSv4.2: fix wrong shrinker_id
>> config: i386-debian-10.3
>> (https://download.01.org/0day-ci/archive/20230615/[email protected]/config)
>> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
>> reproduce (this is a W=1 build):
>>          git checkout linus/master
>>          b4 shazam
>> https://lore.kernel.org/r/[email protected]
>>          # save the config file
>>          mkdir build_dir && cp config build_dir/.config
>>          make W=1 O=build_dir ARCH=i386 olddefconfig
>>          make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new
>> version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <[email protected]>
>> | Closes:
>> https://lore.kernel.org/oe-kbuild-all/[email protected]/
>>
>> All errors (new ones prefixed by >>, old ones prefixed by <<):
>>
>>>> ERROR: modpost: "free_prealloced_shrinker" [fs/nfs/nfsv4.ko] undefined!
>>>> ERROR: modpost: "register_shrinker_prepared" [fs/nfs/nfsv4.ko]
>>>> undefined!
>>>> ERROR: modpost: "prealloc_shrinker" [fs/nfs/nfsv4.ko] undefined!
>
> Ah, these three functions need to be exported. Will fix it in the v2.

Or we can just swap the order of register_shrinker() and
list_lru_init_memcg().

>
>>