Re: [RFC][PATCH v3 3/5] mm/zsmalloc: introduce zs_huge_object()

[Minchan Kim]

On Mon, Mar 14, 2016 at 05:08:43PM +0900, Sergey Senozhatsky wrote:
> On (03/14/16 15:53), Minchan Kim wrote:
> [..]
> > On Thu, Mar 03, 2016 at 11:46:01PM +0900, Sergey Senozhatsky wrote:
> > > zsmalloc knows the watermark after which classes are considered
> > > to be ->huge -- every object stored consumes the entire zspage (which
> > > consist of a single order-0 page). On x86_64, PAGE_SHIFT 12 box, the
> > > first non-huge class size is 3264, so starting down from size 3264,
> > > objects share page(-s) and thus minimize memory wastage.
> > > 
> > > zram, however, has its own statically defined watermark for `bad'
> > > compression "3 * PAGE_SIZE / 4 = 3072", and stores every object
> > > larger than this watermark (3072) as a PAGE_SIZE, object, IOW,
> > > to a ->huge class, this results in increased memory consumption and
> > > memory wastage. (With a small exception: 3264 bytes class. zs_malloc()
> > > adds ZS_HANDLE_SIZE to the object's size, so some objects can pass
> > > 3072 bytes and get_size_class_index(size) will return 3264 bytes size
> > > class).
> > > 
> > > Introduce zs_huge_object() function which tells whether the supplied
> > > object's size belongs to a huge class; so zram now can store objects
> > > to ->huge clases only when those objects have sizes greater than
> > > huge_class_size_watermark.
> > 
> > I understand the problem you pointed out but I don't like this way.
> > 
> > Huge class is internal thing in zsmalloc so zram shouldn't be coupled
> > with it. Zram uses just zsmalloc to minimize meory wastage which is
> > all zram should know about zsmalloc.
> 
> well, zram already coupled with zsmalloc() and it has always been,
> that's the reality. there are zs_foo() calls, and not a single one
> zpool_foo() call. I'm not in love with zs_huge_object() either, but
> that's much better than forcing zsmalloc to be less efficient based
> on some pretty random expectations (no offense).
> 
> > Instead, how about changing max_zpage_size?
> > 
> >         static const size_t max_zpage_size = 4096;
> > 
> > So, if compression doesn't help memory efficiency, we don't
> > need to have decompress overhead. Only that case, we store
> > decompressed page.
> 
> hm, disabling this zram future entirely... this can do the trick,
> I think. zswap is quite happy not having any expectations on
> "how effectively an unknown compression algorithm will compress
> an unknown data set", and that's the "right" thing to do here,
> we can't count on anything.
> 
> 
> > For other huge size class(e.g., PAGE_SIZE / 4 * 3 ~ PAGE_SIZE),
> > you sent a patch to reduce waste memory as 5/5 so I think it's
> > a good justification between memory efficiency VS.
> > decompress overhead.
> 
> so the plan is to raise max_zpage_size to PAGE_SIZE and to increase
> the number of huge classes, so zsmalloc can be more helpful. sounds
> good to me.

nod.

Thanks!