All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: Re: [RFC][PATCHv2 8/8] zsmalloc: register a shrinker to trigger auto-compaction
Date: Wed, 17 Jun 2015 00:45:29 +0900	[thread overview]
Message-ID: <20150616154529.GE20596@swordfish> (raw)
In-Reply-To: <20150616144730.GD31387@blaptop>

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@gmail.com>
> > Reported-by: Minchan Kim <minchan@kernel.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

WARNING: multiple messages have this Message-ID (diff)
From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: Re: [RFC][PATCHv2 8/8] zsmalloc: register a shrinker to trigger auto-compaction
Date: Wed, 17 Jun 2015 00:45:29 +0900	[thread overview]
Message-ID: <20150616154529.GE20596@swordfish> (raw)
In-Reply-To: <20150616144730.GD31387@blaptop>

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@gmail.com>
> > Reported-by: Minchan Kim <minchan@kernel.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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2015-06-16 15:46 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-05 12:03 [RFC][PATCHv2 0/8] introduce automatic pool compaction Sergey Senozhatsky
2015-06-05 12:03 ` Sergey Senozhatsky
2015-06-05 12:03 ` [RFC][PATCHv2 1/8] zsmalloc: drop unused variable `nr_to_migrate' Sergey Senozhatsky
2015-06-05 12:03   ` Sergey Senozhatsky
2015-06-05 12:03 ` [RFC][PATCHv2 2/8] zsmalloc: partial page ordering within a fullness_list Sergey Senozhatsky
2015-06-05 12:03   ` Sergey Senozhatsky
2015-06-16 13:19   ` Minchan Kim
2015-06-16 13:19     ` Minchan Kim
2015-06-16 14:30     ` Sergey Senozhatsky
2015-06-16 14:30       ` Sergey Senozhatsky
2015-06-05 12:03 ` [RFC][PATCHv2 3/8] zsmalloc: lower ZS_ALMOST_FULL waterline Sergey Senozhatsky
2015-06-05 12:03   ` Sergey Senozhatsky
2015-06-16 13:37   ` Minchan Kim
2015-06-16 13:37     ` Minchan Kim
2015-06-16 14:35     ` Sergey Senozhatsky
2015-06-16 14:35       ` Sergey Senozhatsky
2015-06-05 12:03 ` [RFC][PATCHv2 4/8] zsmalloc: always keep per-class stats Sergey Senozhatsky
2015-06-05 12:03   ` Sergey Senozhatsky
2015-06-05 12:03 ` [RFC][PATCHv2 5/8] zsmalloc: introduce zs_can_compact() function Sergey Senozhatsky
2015-06-05 12:03   ` Sergey Senozhatsky
2015-06-16 14:19   ` Minchan Kim
2015-06-16 14:19     ` Minchan Kim
2015-06-16 14:41     ` Sergey Senozhatsky
2015-06-16 14:41       ` Sergey Senozhatsky
2015-06-05 12:03 ` [RFC][PATCHv2 6/8] zsmalloc: cosmetic compaction code adjustments Sergey Senozhatsky
2015-06-05 12:03   ` Sergey Senozhatsky
2015-06-05 12:03 ` [RFC][PATCHv2 7/8] zsmalloc/zram: move `num_migrated' to zs_pool Sergey Senozhatsky
2015-06-05 12:03   ` Sergey Senozhatsky
2015-06-05 12:03 ` [RFC][PATCHv2 8/8] zsmalloc: register a shrinker to trigger auto-compaction Sergey Senozhatsky
2015-06-05 12:03   ` Sergey Senozhatsky
2015-06-16 14:47   ` Minchan Kim
2015-06-16 14:47     ` Minchan Kim
2015-06-16 15:45     ` Sergey Senozhatsky [this message]
2015-06-16 15:45       ` Sergey Senozhatsky
2015-06-18  1:50       ` Minchan Kim
2015-06-18  1:50         ` Minchan Kim
2015-06-18  2:41         ` Sergey Senozhatsky
2015-06-18  2:41           ` Sergey Senozhatsky
2015-06-18  3:01           ` Sergey Senozhatsky
2015-06-18  3:01             ` Sergey Senozhatsky
2015-06-18  3:46             ` Minchan Kim
2015-06-18  3:46               ` Minchan Kim
2015-06-18  3:39           ` Minchan Kim
2015-06-18  3:39             ` Minchan Kim
2015-06-18  3:58             ` Sergey Senozhatsky
2015-06-18  3:58               ` Sergey Senozhatsky
2015-06-17  7:11     ` Sergey Senozhatsky
2015-06-17  7:11       ` Sergey Senozhatsky
2015-06-10  0:04 ` [RFC][PATCHv2 0/8] introduce automatic pool compaction Minchan Kim
2015-06-10  0:04   ` Minchan Kim
2015-06-10  0:07   ` Sergey Senozhatsky
2015-06-10  0:07     ` Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150616154529.GE20596@swordfish \
    --to=sergey.senozhatsky@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.