Message-ID: <20150616154529.GE20596@swordfish>
Date:	Wed, 17 Jun 2015 00:45:29 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
To:	Minchan Kim <minchan@...nel.org>
Cc:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: Re: [RFC][PATCHv2 8/8] zsmalloc: register a shrinker to trigger
 auto-compaction

On (06/16/15 23:47), Minchan Kim wrote:
[..]
> > 
> > Compaction now has a relatively quick pool scan so we are able to
> > estimate the number of pages that will be freed easily, which makes it
> > possible to call this function from a shrinker->count_objects() callback.
> > We also abort compaction as soon as we detect that we can't free any
> > pages any more, preventing wasteful objects migrations. In the example
> > above, "6074 objects were migrated" implies that we actually released
> > zspages back to system.
> > 
> > The initial patch was triggering compaction from zs_free() for
> > every ZS_ALMOST_EMPTY page. Minchan Kim proposed to use a slab
> > shrinker.
> 
> First of all, thanks for mentioning me as proposer.
> However, it's not a helpful comment for other reviewers and
> anonymous people who will review this in future.
> 
> At least, write why I suggested it so others can understand
> the pros/cons.

OK, this one is far from perfect. Will try to improve later.

> > 
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> > Reported-by: Minchan Kim <minchan@...nel.org>
> 
> I didn't report anything. ;-).

:-)

> 
> > ---

[..]

> 
> So should we hold class lock until finishing the compaction of the class?
> It would make horrible latency for other allocation from the class
> in parallel.

hm, what's the difference with the existing implementation?
The 'new one' aborts when (a) !zs_can_compact() and (b) !migrate_zspage().
It holds the class lock less time than current compaction.

> I will review remain parts tomorrow(I hope) but what I want to say
> before going sleep is:
> 
> I like the idea but still have a concern to lack of fragmented zspages
> during memory pressure because auto-compaction will prevent fragment
> most of time. Surely, using fragment space as buffer in heavy memory
> pressure is not intened design so it could be fragile but I'm afraid
> this feature might accelrate it and it ends up having a problem and
> change current behavior in zram as swap.

Well, it's nearly impossible to prove anything with the numbers obtained
during some particular case. I agree that fragmentation can be both
'good' (depending on IO pattern) and 'bad'.


Auto-compaction of IDLE zram devices certainly makes sense, when system
is getting low on memory. zram devices are not always 'busy', serving
heavy IO. There may be N idle zram devices simply sitting and wasting
memory; or being 'moderately' busy; so compaction will not cause any
significant slow down there.

Auto-compaction of BUSY zram devices is less `desired', of course;
but not entirely terrible I think (zs_can_compact() can help here a
lot).

Just an idea
we can move shrinker registration from zsmalloc to zram. zram will be
able to STOP (or forbid) any shrinker activities while it [zram] serves
IO requests (or has requests in its request_queue).

But, again, advocating fragmentation is tricky.


I'll quote from the cover letter

: zsmalloc in some cases can suffer from a notable fragmentation and
: compaction can release some considerable amount of memory. The problem
: here is that currently we fully rely on user space to perform compaction
: when needed. However, performing zsmalloc compaction is not always an
: obvious thing to do. For example, suppose we have a `idle' fragmented
: (compaction was never performed) zram device and system is getting low
: on memory due to some 3rd party user processes (gcc LTO, or firefox, etc.).
: It's quite unlikely that user space will issue zpool compaction in this
: case. Besides, user space cannot tell for sure how badly pool is
: fragmented; however, this info is known to zsmalloc and, hence, to a
: shrinker.


I find this case (a) interesting and (b) quite possible.
/* Besides, this happens on one of my old x86_64 boxen all the time.
 And I do like/appreciate that zram automatically releases some memory. */


> I hope you test this feature with considering my concern.
> Of course, I will test it with enough time.
> 
> Thanks.
> 

sure.

Thanks.

	-ss
--
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/

Powered by blists - more mailing lists