On Thu 28-07-22 17:13:41, tujinjiang@xxxxxxxxxxxxx wrote:
> From: Jinjiang Tu <tujinjiang@xxxxxxxxxxxxx>
>
> When a memcg aware shrinker is registered by register_shrinker(),
> shrinker->nr_deferred will not be initialized. But when the shrinker
> is unregistered by unregister_shrinker(), shrinker->nr_deferred
> will be freed.
>
> Luckily, the memcg aware shrinkers in the current kernel are pre-zeroed.
> But a new memcg aware shrinker may be added in the future, and we should
> not assume the shrinker is pre-zeroed.
>
> Another unregister API free_prealloced_shrinker() does not assume the
> shrinker is pre-zered and free shrinker->nr_deferred only if it is
> not memcg aware. So unregister_shrinker() should do like
> free_prealloced_shrinker().
>
> Signed-off-by: Jinjiang Tu <tujinjiang@xxxxxxxxxxxxx>
> ---
> mm/vmscan.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f7d9a683e3a7..f8a9a5349b6e 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -675,8 +675,11 @@ void unregister_shrinker(struct shrinker *shrinker)
> down_write(&shrinker_rwsem);
> list_del(&shrinker->list);
> shrinker->flags &= ~SHRINKER_REGISTERED;
> - if (shrinker->flags & SHRINKER_MEMCG_AWARE)
> + if (shrinker->flags & SHRINKER_MEMCG_AWARE) {
> unregister_memcg_shrinker(shrinker);
> + up_write(&shrinker_rwsem);
> + return;
> + }
> up_write(&shrinker_rwsem);
>
> kfree(shrinker->nr_deferred);
Can we get rid of the code duplication?
---
diff --git a/mm/vmscan.c b/mm/vmscan.c
index f7d9a683e3a7..308279414fe8 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -632,12 +632,10 @@ int prealloc_shrinker(struct shrinker *shrinker)
return 0;
}
-void free_prealloced_shrinker(struct shrinker *shrinker)
+static void __free_shrinker(struct shrinker *shrinker)
{
if (shrinker->flags & SHRINKER_MEMCG_AWARE) {
- down_write(&shrinker_rwsem);
unregister_memcg_shrinker(shrinker);
- up_write(&shrinker_rwsem);
return;
}
@@ -645,6 +643,13 @@ void free_prealloced_shrinker(struct shrinker *shrinker)
shrinker->nr_deferred = NULL;
}
+void free_prealloced_shrinker(struct shrinker *shrinker)
+{
+ down_write(&shrinker_rwsem);
+ __free_shrinker(shrinker);
+ up_write(&shrinker_rwsem);
+}
+
void register_shrinker_prepared(struct shrinker *shrinker)
{
down_write(&shrinker_rwsem);
@@ -675,12 +680,9 @@ void unregister_shrinker(struct shrinker *shrinker)
down_write(&shrinker_rwsem);
list_del(&shrinker->list);
shrinker->flags &= ~SHRINKER_REGISTERED;
- if (shrinker->flags & SHRINKER_MEMCG_AWARE)
- unregister_memcg_shrinker(shrinker);
- up_write(&shrinker_rwsem);
- kfree(shrinker->nr_deferred);
- shrinker->nr_deferred = NULL;
+ __free_shrinker(shrinker);
+ up_write(&shrinker_rwsem);
}
EXPORT_SYMBOL(unregister_shrinker);
--
Michal Hocko
SUSE Labs
