lkml.org 
[lkml]   [2013]   [Feb]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@linux.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



\
 
 \ /
  Last update: 2013-02-13 17:42    [W:0.080 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site