linux-kernel - Re: [PATCHv4 2/7] zsmalloc: promote to lib/

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] [day] [month] [year] [list]

Message-ID: <511BB8A7.9040305@linux.vnet.ibm.com>
Date:	Wed, 13 Feb 2013 10:00:39 -0600
From:	Seth Jennings <sjenning@...ux.vnet.ibm.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Nitin Gupta <ngupta@...are.org>,
	Minchan Kim <minchan@...nel.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Dan Magenheimer <dan.magenheimer@...cle.com>,
	Robert Jennings <rcj@...ux.vnet.ibm.com>,
	Jenifer Hopper <jhopper@...ibm.com>,
	Mel Gorman <mgorman@...e.de>,
	Johannes Weiner <jweiner@...hat.com>,
	Rik van Riel <riel@...hat.com>,
	Larry Woodman <lwoodman@...hat.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Dave Hansen <dave@...ux.vnet.ibm.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org
Subject: Re: [PATCHv4 2/7] zsmalloc: promote to lib/

On 01/29/2013 04:51 PM, Andrew Morton wrote:
> On Tue, 29 Jan 2013 15:40:22 -0600
> Seth Jennings <sjenning@...ux.vnet.ibm.com> wrote:
> 
>> This patch promotes the slab-based zsmalloc memory allocator
>> from the staging tree to lib/
> 
> Hate to rain on the parade, but...  we haven't reviewed zsmalloc
> yet.  At least, I haven't, and I haven't seen others do so.
> 
> So how's about we forget that zsmalloc was previously in staging and
> send the zsmalloc code out for review?  With a very good changelog
> explaining why it exists, what problems it solves, etc.
> 
> 
> I peeked.
> 
> Don't answer any of the below questions - they are examples of
> concepts which should be accessible to readers of the
> hopefully-forthcoming very-good-changelog.

I know you just said "don't answer", but I wanted to say why certain
points aren't included in the new patchset I'm about to post later today.

> 
> - kmap_atomic() returns a void* - there's no need to cast its return value.

In places where we do pointer arithmetic, we do the cast to avoid
incrementing a void *, which is acceptable for gcc, but not everyone.

> 
> - Remove private MAX(), use the (much better implemented) max().

We can't use max() or max_t() because ZS_SIZE_CLASSES, which uses
ZS_MIN_ALLOC_SIZE, is used to define and array size in struct zs_pool.
So the expression must be able to be fully evaluated by the precompiler.

> 
> - It says "This was one of the major issues with its predecessor
>   (xvmalloc)", but drivers/staging/ramster/xvmalloc.c is still in the tree.

Yes, this was a little mess.  I think you'll find that isn't the case
anymore.  Dan has removed those files from zcache/ramster.

> 
> - USE_PGTABLE_MAPPING should be done via Kconfig.

Minchan did this work and will be included in the next version of the
patchset.

I've added more documentation/comments in the new patchset.  Hopefully,
it will help with understanding the more complicated parts of the code.

Thanks,
Seth

--
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/