linux-kernel - Re: [PATCHv4 2/9] zsmalloc: turn zspage order into runtime variable

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:   Fri, 11 Nov 2022 09:09:19 -0800
From:   Minchan Kim <minchan@...nel.org>
To:     Sergey Senozhatsky <senozhatsky@...omium.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Nitin Gupta <ngupta@...are.org>, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [PATCHv4 2/9] zsmalloc: turn zspage order into runtime variable

On Fri, Nov 11, 2022 at 07:38:10PM +0900, Sergey Senozhatsky wrote:
< snip >

> [..]
> > >  enum fullness_group {
> > >  	ZS_EMPTY,
> > > @@ -230,12 +221,15 @@ struct link_free {
> > >  struct zs_pool {
> > >  	const char *name;
> > >  
> > > -	struct size_class *size_class[ZS_SIZE_CLASSES];
> > > +	struct size_class **size_class;
> > >  	struct kmem_cache *handle_cachep;
> > >  	struct kmem_cache *zspage_cachep;
> > >  
> > >  	atomic_long_t pages_allocated;
> > >  
> > > +	u32 num_size_classes;
> > > +	u32 min_alloc_size;
> > 
> > Please use int.
> 
> OK. Any reason why we don't want u32? I thought that
> s16/u16/s32/u32/etc. is the new normal.

Oh, I didn't know the new normal.

# ag u32 mm/ | wc -l 
65

Then, I'd like to use int to be consistent with others.

> 
> > From this patch, I couldn't figure why we need
> > variable in the pool. Let's have the change in the patch where
> > you really need to have the usecase.
> 
> Let me take a look.
> 
> > > -static int get_pages_per_zspage(int class_size)
> > > +static int get_pages_per_zspage(u32 class_size, u32 num_pages)
> > 
> > Let's just use int instead of u32
> > 
> > Why do you need num_pages argument instead of using 1UL << ZS_DEFAULT_PAGE_ORDER?
> > It looks like static value.
> 
> It is static right now, but in the a couple of patches it'll change to
> dynamic.

Then, plase have the change in the patch you will use to review easier.