Re: [PATCH v2] mm/zsmalloc: simplify shrinker init/destroy

[Andrew Morton <akpm@linux-foundation.org>]

On Tue, 19 Dec 2017 20:45:17 +0300 Aliaksei Karaliou <akaraliou.dev@gmail.com> wrote:

> >>> So what will happen if the pool is alive and used without any shrinker?
> >>> How do objects get freed?
> >> we use shrinker for "optional" de-fragmentation of zsmalloc pools. we
> >> don't free any objects from that path. just move them around within their
> >> size classes - to consolidate objects and to, may be, free unused pages
> >> [but we first need to make them "unused"]. it's not a mandatory thing for
> >> zsmalloc, we are just trying to be nice.
> > OK, it smells like an abuse of the API but please add a comment
> > clarifying that.
> >
> > Thanks!
> I can update the existing comment to be like that:
>          /*
>           * Not critical since shrinker is only used to trigger internal
>           * de-fragmentation of the pool which is pretty optional thing.
>           * If registration fails we still can use the pool normally and
>           * user can trigger compaction manually. Thus, ignore return code.
>           */
> 
> Sergey, does this sound well to you ? Or not clear enough, Michal ?

I did this:

From: Andrew Morton <akpm@linux-foundation.org>
Subject: mm-zsmalloc-simplify-shrinker-init-destroy-fix

update comment (Aliaksei), make zs_register_shrinker() return void

Cc: Aliaksei Karaliou <akaraliou.dev@gmail.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/zsmalloc.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff -puN mm/zsmalloc.c~mm-zsmalloc-simplify-shrinker-init-destroy-fix mm/zsmalloc.c
--- a/mm/zsmalloc.c~mm-zsmalloc-simplify-shrinker-init-destroy-fix
+++ a/mm/zsmalloc.c
@@ -2323,14 +2323,14 @@ static void zs_unregister_shrinker(struc
 	unregister_shrinker(&pool->shrinker);
 }
 
-static int zs_register_shrinker(struct zs_pool *pool)
+static void zs_register_shrinker(struct zs_pool *pool)
 {
 	pool->shrinker.scan_objects = zs_shrinker_scan;
 	pool->shrinker.count_objects = zs_shrinker_count;
 	pool->shrinker.batch = 0;
 	pool->shrinker.seeks = DEFAULT_SEEKS;
 
-	return register_shrinker(&pool->shrinker);
+	register_shrinker(&pool->shrinker);
 }
 
 /**
@@ -2419,10 +2419,12 @@ struct zs_pool *zs_create_pool(const cha
 		goto err;
 
 	/*
-	 * Not critical, we still can use the pool
-	 * and user can trigger compaction manually.
+	 * Not critical since shrinker is only used to trigger internal
+	 * defragmentation of the pool which is pretty optional thing.  If
+	 * registration fails we still can use the pool normally and user can
+	 * trigger compaction manually. Thus, ignore return code.
 	 */
-	(void) zs_register_shrinker(pool);
+	zs_register_shrinker(pool);
 
 	return pool;
 
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>