All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Joonsoo Kim <js1304@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Mel Gorman <mgorman@suse.de>, Rik van Riel <riel@redhat.com>,
	David Rientjes <rientjes@google.com>,
	Minchan Kim <minchan@kernel.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>
Subject: Re: [PATCH v3 7/7] mm/compaction: replace compaction deferring with compaction limit
Date: Mon, 14 Dec 2015 10:55:00 +0100	[thread overview]
Message-ID: <566E91F4.3000709@suse.cz> (raw)
In-Reply-To: <1449126681-19647-8-git-send-email-iamjoonsoo.kim@lge.com>

On 12/03/2015 08:11 AM, Joonsoo Kim wrote:
> Compaction deferring effectively reduces compaction overhead if
> compaction success isn't expected. But, it is implemented that
> skipping a number of compaction requests until compaction is re-enabled.
> Due to this implementation, unfortunate compaction requestor will get
> whole compaction overhead unlike others have zero overhead. And, after
> deferring start to work, even if compaction success possibility is
> restored, we should skip to compaction in some number of times.
>
> This patch try to solve above problem by using compaction limit.
> Instead of imposing compaction overhead to one unfortunate requestor,
> compaction limit distributes overhead to all compaction requestors.
> All requestors have a chance to migrate some amount of pages and
> after limit is exhausted compaction will be stopped. This will fairly
> distributes overhead to all compaction requestors. And, because we don't
> defer compaction request, someone will succeed to compact as soon as
> possible if compaction success possiblility is restored.
>
> Following is whole workflow enabled by this change.
>
> - if sync compaction fails, compact_order_failed is set to current order
> - if it fails again, compact_defer_shift is adjusted
> - with positive compact_defer_shift, migration_scan_limit is assigned and
> compaction limit is activated
> - if compaction limit is activated, compaction would be stopped when
> migration_scan_limit is exhausted
> - when success, compact_defer_shift and compact_order_failed is reset and
> compaction limit is deactivated
> - compact_defer_shift can be grown up to COMPACT_MAX_DEFER_SHIFT
>
> Most of changes are mechanical ones to remove compact_considered which
> is not needed now. Note that, after restart, compact_defer_shift is
> subtracted by 1 to avoid invoking __reset_isolation_suitable()
> repeatedly.
>
> I tested this patch on my compaction benchmark and found that high-order
> allocation latency is evenly distributed and there is no latency spike
> in the situation where compaction success isn't possible.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Looks fine overal, looking forward to next version :) (due to changes 
expected in preceding patches, I didn't review the code fully now).

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

WARNING: multiple messages have this Message-ID (diff)
From: Vlastimil Babka <vbabka@suse.cz>
To: Joonsoo Kim <js1304@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Mel Gorman <mgorman@suse.de>, Rik van Riel <riel@redhat.com>,
	David Rientjes <rientjes@google.com>,
	Minchan Kim <minchan@kernel.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>
Subject: Re: [PATCH v3 7/7] mm/compaction: replace compaction deferring with compaction limit
Date: Mon, 14 Dec 2015 10:55:00 +0100	[thread overview]
Message-ID: <566E91F4.3000709@suse.cz> (raw)
In-Reply-To: <1449126681-19647-8-git-send-email-iamjoonsoo.kim@lge.com>

On 12/03/2015 08:11 AM, Joonsoo Kim wrote:
> Compaction deferring effectively reduces compaction overhead if
> compaction success isn't expected. But, it is implemented that
> skipping a number of compaction requests until compaction is re-enabled.
> Due to this implementation, unfortunate compaction requestor will get
> whole compaction overhead unlike others have zero overhead. And, after
> deferring start to work, even if compaction success possibility is
> restored, we should skip to compaction in some number of times.
>
> This patch try to solve above problem by using compaction limit.
> Instead of imposing compaction overhead to one unfortunate requestor,
> compaction limit distributes overhead to all compaction requestors.
> All requestors have a chance to migrate some amount of pages and
> after limit is exhausted compaction will be stopped. This will fairly
> distributes overhead to all compaction requestors. And, because we don't
> defer compaction request, someone will succeed to compact as soon as
> possible if compaction success possiblility is restored.
>
> Following is whole workflow enabled by this change.
>
> - if sync compaction fails, compact_order_failed is set to current order
> - if it fails again, compact_defer_shift is adjusted
> - with positive compact_defer_shift, migration_scan_limit is assigned and
> compaction limit is activated
> - if compaction limit is activated, compaction would be stopped when
> migration_scan_limit is exhausted
> - when success, compact_defer_shift and compact_order_failed is reset and
> compaction limit is deactivated
> - compact_defer_shift can be grown up to COMPACT_MAX_DEFER_SHIFT
>
> Most of changes are mechanical ones to remove compact_considered which
> is not needed now. Note that, after restart, compact_defer_shift is
> subtracted by 1 to avoid invoking __reset_isolation_suitable()
> repeatedly.
>
> I tested this patch on my compaction benchmark and found that high-order
> allocation latency is evenly distributed and there is no latency spike
> in the situation where compaction success isn't possible.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Looks fine overal, looking forward to next version :) (due to changes 
expected in preceding patches, I didn't review the code fully now).


  reply	other threads:[~2015-12-14  9:55 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-03  7:11 [PATCH v3 0/7] mm/compaction: redesign compaction: part1 Joonsoo Kim
2015-12-03  7:11 ` Joonsoo Kim
2015-12-03  7:11 ` [PATCH v3 1/7] mm/compaction: skip useless pfn when updating cached pfn Joonsoo Kim
2015-12-03  7:11   ` Joonsoo Kim
2015-12-03 10:36   ` Vlastimil Babka
2015-12-03 10:36     ` Vlastimil Babka
2015-12-07  7:37     ` Joonsoo Kim
2015-12-07  7:37       ` Joonsoo Kim
2015-12-03  7:11 ` [PATCH v3 2/7] mm/compaction: remove unused defer_compaction() in compaction.h Joonsoo Kim
2015-12-03  7:11   ` Joonsoo Kim
2015-12-04 15:29   ` Vlastimil Babka
2015-12-04 15:29     ` Vlastimil Babka
2015-12-03  7:11 ` [PATCH v3 3/7] mm/compaction: initialize compact_order_failed to MAX_ORDER Joonsoo Kim
2015-12-03  7:11   ` Joonsoo Kim
2015-12-04 15:36   ` Vlastimil Babka
2015-12-04 15:36     ` Vlastimil Babka
2015-12-03  7:11 ` [PATCH v3 4/7] mm/compaction: update defer counter when allocation is expected to succeed Joonsoo Kim
2015-12-03  7:11   ` Joonsoo Kim
2015-12-04 16:52   ` Vlastimil Babka
2015-12-04 16:52     ` Vlastimil Babka
2015-12-07  8:03     ` Joonsoo Kim
2015-12-07  8:03       ` Joonsoo Kim
2015-12-03  7:11 ` [PATCH v3 5/7] mm/compaction: respect compaction order when updating defer counter Joonsoo Kim
2015-12-03  7:11   ` Joonsoo Kim
2015-12-04 17:15   ` Vlastimil Babka
2015-12-04 17:15     ` Vlastimil Babka
2015-12-07  8:04     ` Joonsoo Kim
2015-12-07  8:04       ` Joonsoo Kim
2015-12-03  7:11 ` [PATCH v3 6/7] mm/compaction: introduce migration scan limit Joonsoo Kim
2015-12-03  7:11   ` Joonsoo Kim
2015-12-14  9:34   ` Vlastimil Babka
2015-12-14  9:34     ` Vlastimil Babka
2015-12-16  5:39     ` Joonsoo Kim
2015-12-16  5:39       ` Joonsoo Kim
2015-12-03  7:11 ` [PATCH v3 7/7] mm/compaction: replace compaction deferring with compaction limit Joonsoo Kim
2015-12-03  7:11   ` Joonsoo Kim
2015-12-14  9:55   ` Vlastimil Babka [this message]
2015-12-14  9:55     ` Vlastimil Babka

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=566E91F4.3000709@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=js1304@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=minchan@kernel.org \
    --cc=riel@redhat.com \
    --cc=rientjes@google.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.