RE: [PATCH 2/2] zram: clean up handle

[Dan Magenheimer]

> From: Minchan Kim [mailto:minchan@xxxxxxxxxx]
> Subject: Re: [PATCH 2/2] zram: clean up handle
> 
> On 06/06/2012 02:04 PM, Nitin Gupta wrote:
> 
> > On 06/05/2012 12:23 AM, Minchan Kim wrote:
> >
> >> zram's handle variable can store handle of zsmalloc in case of
> >> compressing efficiently. Otherwise, it stores point of page descriptor.
> >> This patch clean up the mess by union struct.
> >>
> >> changelog
> >>   * from v1
> >> 	- none(new add in v2)
> >>
> >> Cc: Nitin Gupta <ngupta@xxxxxxxxxx>
> >> Acked-by: Seth Jennings <sjenning@xxxxxxxxxxxxxxxxxx>
> >> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> >> Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx>
> >> ---
> >>  drivers/staging/zram/zram_drv.c |   77 ++++++++++++++++++++-------------------
> >>  drivers/staging/zram/zram_drv.h |    5 ++-
> >>  2 files changed, 44 insertions(+), 38 deletions(-)
> >
> > I think page vs handle distinction was added since xvmalloc could not
> > handle full page allocation. Now that zsmalloc allows full page
> 
> I see. I didn't know that because I'm blind on xvmalloc.
> 
> > allocation, we can just use it for both cases. This would also allow
> > removing the ZRAM_UNCOMPRESSED flag. The only downside will be slightly
> > slower code path for full page allocation but this event is anyways
> > supposed to be rare, so should be fine.
> 
> Fair enough.
> It can remove many code of zram.
> Okay. Will look into that.

Nitin, can zsmalloc allow full page allocation by assigning
an actual physical pageframe (which is what zram does now)?
Or will it allocate PAGE_SIZE bytes which zsmalloc will allocate
crossing a page boundary which, presumably, will have much worse
impact on page allocator availability when these pages are
"reclaimed" via your swap notify callback.

Though this may be rare across all workloads, it may turn out
to be very common for certain workloads (e.g. if the workload
has many dirty anonymous pages that are already compressed
by userland).

It may not be worth cleaning up the code if it causes
performance issues with this case.

And anyway can zsmalloc handle and identify to the caller pages
that are both compressed and "native" (uncompressed)?  It
certainly has to handle both if you remove ZRAM_UNCOMPRESSED
as compressing some pages actually results in more than
PAGE_SIZE bytes.  So you need to record somewhere that
this "compressed page" is special and that must somehow
be communicated to the caller of your "get" routine.

(Just trying to save Minchan from removing all that code but
then needing to add it back again.)

Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/