linux-kernel - Re: [PATCH v2 2/2] zram: reduce metadata overhead

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, 30 Nov 2012 23:37:44 +0900
From:	Minchan Kim <minchan.kernel.2@...il.com>
To:	Nitin Gupta <ngupta@...are.org>
Cc:	Greg KH <greg@...ah.com>, Jerome Marchand <jmarchan@...hat.com>,
	Seth Jennings <sjenning@...ux.vnet.ibm.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	Sam Hansen <solid.se7en@...il.com>,
	Linux Driver Project <devel@...uxdriverproject.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] zram: reduce metadata overhead

On Thu, Nov 29, 2012 at 10:54:49PM -0800, Nitin Gupta wrote:
> Changelog v2 vs v1:
>  - Use is_zero_page() instead of direct handle comparison
>  - Use 1 as invalid handle value instead of -1 since handle
> is unsigned and thus -1 may refer to a valid object. While 1
> is guaranteed to be invalid since <pfn:0,offset:1> can never
> refer to (end of) a valid object.

Ho, Hmm, another coupling between zram and zsmalloc.
The zram knows internal of zsmalloc very well. Sigh.
I don't like it really. Nonetheless, if you really want it,
please put "#define ZS_INVALID_HANDLE 1" in zsmalloc.h and use it.
But the concern about my suggestion is that user can imagine
it's equal to 0 so they might try to use it instead of 0.
Maybe we need more clear name.

Off-topic:
Anyway, my assumption about user's mistake is only vaild in case of
general allocator but zsmalloc already wasn't general allocator by
following as.

zs_map_object
zs_get_objsize
ZS_INVALID_HANDLE

Now I'm sure we shouldn't put it in under /lib. :(

>  - Remove references to 'table' in comments and messages since
> we just have a plain array of handles now.
> 
> For every allocated object, zram maintains the the handle, size,
> flags and count fields. Of these, only the handle is required
> since zsmalloc now provides the object size given the handle.
> The flags field was needed only to mark a given page as zero-filled.
> Instead of this field, we now use an invalid value (-1) to mark such

                                                      ZS_INAVLID_HANDLE or something.

> pages. Lastly, the count field was unused, so was simply removed.
> 
> Signed-off-by: Nitin Gupta <ngupta@...are.org>
> Reviewed-by: Jerome Marchand <jmarchan@...hat.com>
> ---
>  drivers/staging/zram/zram_drv.c |   97 ++++++++++++++++-----------------------
>  drivers/staging/zram/zram_drv.h |   20 ++------
>  2 files changed, 43 insertions(+), 74 deletions(-)
> 

Otherwise, looks good to me.
-- 
Kind regards,
Minchan Kim
--
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/