linux-kernel - Re: [PATCH 2/2] zram: clean up handle

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: <4FD11A3C.801@vflare.org>
Date:	Thu, 07 Jun 2012 14:16:44 -0700
From:	Nitin Gupta <ngupta@...are.org>
To:	Dan Magenheimer <dan.magenheimer@...cle.com>
CC:	Minchan Kim <minchan@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Konrad Wilk <konrad.wilk@...cle.com>,
	Seth Jennings <sjenning@...ux.vnet.ibm.com>
Subject: Re: [PATCH 2/2] zram: clean up handle

On 06/07/2012 01:47 PM, Dan Magenheimer wrote:

>> From: Minchan Kim [mailto:minchan@...nel.org]
>> 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@...are.org>
>>>> Acked-by: Seth Jennings <sjenning@...ux.vnet.ibm.com>
>>>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
>>>> Signed-off-by: Minchan Kim <minchan@...nel.org>
>>>> ---
>>>>  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.
> 


zsmalloc does not add any object headers, so when allocating PAGE_SIZE
you get a separate page from as if you did alloc_page(). So, it does not
span page boundaries.


> 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.)
> 


zsmalloc cannot identify compressed vs uncompressed pages. However, in
zram, we can tell if the page is uncompressed using table[i]->size which
is set to PAGE_SIZE for uncompressed pages.   Pages that compress to
more than PAGE_SIZE (i.e. expand on compression) are stored
as-is/uncompressed and thus will have size field set to PAGE_SIZE.

Thus, we do not require ZRAM_UNCOMPRESSED flag when using zsmalloc for
both compressed and uncompressed pages.

Thanks,
Nitin

Thanks,
Nitin
--
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/