lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20151015035317.GF1735@swordfish>
Date:	Thu, 15 Oct 2015 12:53:17 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:	Minchan Kim <minchan@...nel.org>
Cc:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [PATCH] zsmalloc: don't test shrinker_enabled in
 zs_shrinker_count()

On (10/15/15 11:29), Minchan Kim wrote:
[..]
> I'm in favor of removing shrinker disable feature with this patch(
> although we didn't implement it yet) because if there is some problem
> of compaction, we should reveal and fix it without hiding with the
> feature.
> 

sure.

> One thing I want is if we decide it, let's remove all things
> about shrinker_enabled(ie, variable).
> If we might need it later, we could introduce it easily.

well, do we really want to make the shrinker a vital part of zsmalloc?

it's not that we will tighten the dependency between zsmalloc and
shrinker, we will introduce it instead. in a sense that, at the moment,
zsmalloc is, let's say, ignorant to shrinker registration errors
(shrinker registration implementation is internal to shrinker), because
there is no direct impact on zsmalloc functionality -- zsmalloc will not
be able to release some pages (there are if-s here: first, zsmalloc
shrinker callback may even not be called; second, zsmalloc may not be
albe to migrate objects and release objects).

no really strong opinion against, but at the same time zsmalloc will
have another point of failure (again, zsmalloc should not be aware of
shrinker registration implementation and why it may fail).

so... I can prepare a new patch later today.

	-ss

> 
> > 
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> > ---
> >  mm/zsmalloc.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> > index 7ad5e54..8ba247d 100644
> > --- a/mm/zsmalloc.c
> > +++ b/mm/zsmalloc.c
> > @@ -1822,9 +1822,6 @@ static unsigned long zs_shrinker_count(struct shrinker *shrinker,
> >  	struct zs_pool *pool = container_of(shrinker, struct zs_pool,
> >  			shrinker);
> >  
> > -	if (!pool->shrinker_enabled)
> > -		return 0;
> > -
> >  	for (i = zs_size_classes - 1; i >= 0; i--) {
> >  		class = pool->size_class[i];
> >  		if (!class)
> > -- 
> > 2.6.1.134.g4b1fd35
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ