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

[Minchan Kim]

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@xxxxxxxxxx>
> Reviewed-by: Jerome Marchand <jmarchan@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/