linux-kernel - Re: [PATCH] mm/zblock: use vmalloc for page allocations

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: <aBnZdLvgtz_54URF@pc636>
Date: Tue, 6 May 2025 11:42:12 +0200
From: Uladzislau Rezki <urezki@...il.com>
To: Johannes Weiner <hannes@...xchg.org>,
	Vitaly Wool <vitaly.wool@...sulko.se>
Cc: Vitaly Wool <vitaly.wool@...sulko.se>, linux-mm@...ck.org,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	Nhat Pham <nphamcs@...il.com>,
	Shakeel Butt <shakeel.butt@...ux.dev>,
	Yosry Ahmed <yosry.ahmed@...ux.dev>,
	Minchan Kim <minchan@...nel.org>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Igor Belousov <igor.b@...dev.am>
Subject: Re: [PATCH] mm/zblock: use vmalloc for page allocations

On Mon, May 05, 2025 at 10:29:49AM -0400, Johannes Weiner wrote:
> On Fri, May 02, 2025 at 10:01:56AM +0200, Vitaly Wool wrote:
> >  static struct zblock_block *alloc_block(struct zblock_pool *pool,
> >  					int block_type, gfp_t gfp,
> > -					unsigned long *handle)
> > +					unsigned long *handle,
> > +					unsigned int nid)
> >  {
> > +	struct block_list *block_list = &pool->block_lists[block_type];
> > +	unsigned int num_pages = block_desc[block_type].num_pages;
> >  	struct zblock_block *block;
> > -	struct block_list *block_list;
> > +	struct page *page = NULL;
> >  
> > -	block = (void *)__get_free_pages(gfp, block_desc[block_type].order);
> > -	if (!block)
> > -		return NULL;
> > +	if (!vmalloc_small_blocks && zblock_get_order(num_pages) >= 0) {
> > +		page = try_alloc_pages(nid, zblock_get_order(num_pages));
> 
> This is broken in several ways.
> 
> The function is meant for NMI contexts - the "try" refers to
> trylocking the freelists, in case whatever got interrupted was inside
> the allocator already. This will fall back to vmalloc unpredictably.
> 
> It also doesn't take a gfp parameter, which ignores the zswap ones,
> and substitutes a set that doesn't make any sense in this context:
> __GFP_NOMEMALLOC is counter productive inside reclaim; __GFP_ACCOUNT
> wreaks complete havoc on how compressed memory is charged to cgroups
> (double charging the wrong groups for shared blocks).
> 
+ "&& zblock_get_order(num_pages) >= 0" is always true?

A fallback makes sense to use when order > 0, IMO. Or just stick
fully to vmalloc. Another option is kvmalloc()/kvfree().

--
Uladzislau Rezki