linux-kernel - Re: [PATCH v2 5/5] zsmalloc: Implement writeback mechanism for zsmalloc

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]

Date:   Thu, 3 Nov 2022 12:45:53 -0400
From:   Johannes Weiner <hannes@...xchg.org>
To:     Sergey Senozhatsky <senozhatsky@...omium.org>
Cc:     Nhat Pham <nphamcs@...il.com>, akpm@...ux-foundation.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        minchan@...nel.org, ngupta@...are.org, sjenning@...hat.com,
        ddstreet@...e.org, vitaly.wool@...sulko.com
Subject: Re: [PATCH v2 5/5] zsmalloc: Implement writeback mechanism for
 zsmalloc

On Wed, Nov 02, 2022 at 01:13:58PM +0900, Sergey Senozhatsky wrote:
> On (22/10/27 11:27), Nhat Pham wrote:
> > +
> > +static int zs_zpool_shrink(void *pool, unsigned int pages,
> > +			unsigned int *reclaimed)
> > +{
> > +	unsigned int total = 0;
> > +	int ret = -EINVAL;
> > +
> > +	while (total < pages) {
> > +		ret = zs_reclaim_page(pool, 8);
> > +		if (ret < 0)
> > +			break;
> > +		total++;
> > +	}
> > +
> > +	if (reclaimed)
> > +		*reclaimed = total;
> > +
> > +	return ret;
> > +}
> 
> The name collides with shrinker callbacks (compaction). That's a bit
> confusing, took me some time.

Yeah this tripped us up too.

Unfortunately, this is inherited from the zpool API:

> > @@ -482,6 +504,7 @@ static struct zpool_driver zs_zpool_driver = {
> >  	.malloc_support_movable = true,
> >  	.malloc =		  zs_zpool_malloc,
> >  	.free =			  zs_zpool_free,
> > +	.shrink =     zs_zpool_shrink,

There is another terminology collision around "compaction" and
"migration": There is zs_page_migrate() which is called from physical
page migration and compaction to switch out backing struct pages of
the zspage. Then there is migrate_zspage() which is called from a
*shrinker* through *zs_compact()*, and it consolidates the objects
from partially used zspages into full ones.

We're collecting these issues and want to clean them up separately. It
seems this codebase hasn't had a lot of TLC recently, which makes it
harder to make needed changes :/ We try to clean up as we go, but some
of these issues (like the .shrink callback) run through all backends,
zpool and zswap, so they'd be better off in separate cleanup patches.

The out-of-control locking complexity in zsmalloc is another one of
these issues that we hoped we could clean up along the way, as you
have noticed...