All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@suse.de>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Rik van Riel <riel@redhat.com>
Subject: Re: [PATCH 5/5] mm: compaction: reset scanner positions immediately when they meet
Date: Tue, 26 Nov 2013 11:03:30 +0000	[thread overview]
Message-ID: <20131126110330.GJ5285@suse.de> (raw)
In-Reply-To: <1385389570-11393-6-git-send-email-vbabka@suse.cz>

On Mon, Nov 25, 2013 at 03:26:10PM +0100, Vlastimil Babka wrote:
> Compaction used to start its migrate and free page scaners at the zone's lowest
> and highest pfn, respectively. Later, caching was introduced to remember the
> scanners' progress across compaction attempts so that pageblocks are not
> re-scanned uselessly. Additionally, pageblocks where isolation failed are
> marked to be quickly skipped when encountered again in future compactions.
> 
> Currently, both the reset of cached pfn's and clearing of the pageblock skip
> information for a zone is done in __reset_isolation_suitable(). This function
> gets called when:
>  - compaction is restarting after being deferred
>  - compact_blockskip_flush flag is set in compact_finished() when the scanners
>    meet (and not again cleared when direct compaction succeeds in allocation)
>    and kswapd acts upon this flag before going to sleep
> 
> This behavior is suboptimal for several reasons:
>  - when direct sync compaction is called after async compaction fails (in the
>    allocation slowpath), it will effectively do nothing, unless kswapd
>    happens to process the compact_blockskip_flush flag meanwhile. This is racy
>    and goes against the purpose of sync compaction to more thoroughly retry
>    the compaction of a zone where async compaction has failed.
>    The restart-after-deferring path cannot help here as deferring happens only
>    after the sync compaction fails. It is also done only for the preferred
>    zone, while the compaction might be done for a fallback zone.
>  - the mechanism of marking pageblock to be skipped has little value since the
>    cached pfn's are reset only together with the pageblock skip flags. This
>    effectively limits pageblock skip usage to parallel compactions.
> 
> This patch changes compact_finished() so that cached pfn's are reset
> immediately when the scanners meet. Clearing pageblock skip flags is unchanged,
> as well as the other situations where cached pfn's are reset. This allows the
> sync-after-async compaction to retry pageblocks not marked as skipped, such as
> blocks !MIGRATE_MOVABLE blocks that async compactions now skips without
> marking them.
> 
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Rik van Riel <riel@redhat.com>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>

Acked-by: Mel Gorman <mgorman@suse.de>

-- 
Mel Gorman
SUSE Labs

--
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: Mel Gorman <mgorman@suse.de>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Rik van Riel <riel@redhat.com>
Subject: Re: [PATCH 5/5] mm: compaction: reset scanner positions immediately when they meet
Date: Tue, 26 Nov 2013 11:03:30 +0000	[thread overview]
Message-ID: <20131126110330.GJ5285@suse.de> (raw)
In-Reply-To: <1385389570-11393-6-git-send-email-vbabka@suse.cz>

On Mon, Nov 25, 2013 at 03:26:10PM +0100, Vlastimil Babka wrote:
> Compaction used to start its migrate and free page scaners at the zone's lowest
> and highest pfn, respectively. Later, caching was introduced to remember the
> scanners' progress across compaction attempts so that pageblocks are not
> re-scanned uselessly. Additionally, pageblocks where isolation failed are
> marked to be quickly skipped when encountered again in future compactions.
> 
> Currently, both the reset of cached pfn's and clearing of the pageblock skip
> information for a zone is done in __reset_isolation_suitable(). This function
> gets called when:
>  - compaction is restarting after being deferred
>  - compact_blockskip_flush flag is set in compact_finished() when the scanners
>    meet (and not again cleared when direct compaction succeeds in allocation)
>    and kswapd acts upon this flag before going to sleep
> 
> This behavior is suboptimal for several reasons:
>  - when direct sync compaction is called after async compaction fails (in the
>    allocation slowpath), it will effectively do nothing, unless kswapd
>    happens to process the compact_blockskip_flush flag meanwhile. This is racy
>    and goes against the purpose of sync compaction to more thoroughly retry
>    the compaction of a zone where async compaction has failed.
>    The restart-after-deferring path cannot help here as deferring happens only
>    after the sync compaction fails. It is also done only for the preferred
>    zone, while the compaction might be done for a fallback zone.
>  - the mechanism of marking pageblock to be skipped has little value since the
>    cached pfn's are reset only together with the pageblock skip flags. This
>    effectively limits pageblock skip usage to parallel compactions.
> 
> This patch changes compact_finished() so that cached pfn's are reset
> immediately when the scanners meet. Clearing pageblock skip flags is unchanged,
> as well as the other situations where cached pfn's are reset. This allows the
> sync-after-async compaction to retry pageblocks not marked as skipped, such as
> blocks !MIGRATE_MOVABLE blocks that async compactions now skips without
> marking them.
> 
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Rik van Riel <riel@redhat.com>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>

Acked-by: Mel Gorman <mgorman@suse.de>

-- 
Mel Gorman
SUSE Labs

  reply	other threads:[~2013-11-26 11:03 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-25 14:26 [RFC PATCH 0/5] Memory compaction efficiency improvements Vlastimil Babka
2013-11-25 14:26 ` Vlastimil Babka
2013-11-25 14:26 ` [PATCH 1/5] mm: compaction: encapsulate defer reset logic Vlastimil Babka
2013-11-25 14:26   ` Vlastimil Babka
2013-11-25 22:08   ` Rik van Riel
2013-11-25 22:08     ` Rik van Riel
2013-11-26 10:16   ` Mel Gorman
2013-11-26 10:16     ` Mel Gorman
2013-11-25 14:26 ` [PATCH 2/5] mm: compaction: reset cached scanner pfn's before reading them Vlastimil Babka
2013-11-25 14:26   ` Vlastimil Babka
2013-11-26 10:23   ` Mel Gorman
2013-11-26 10:23     ` Mel Gorman
2013-11-26 13:16   ` Rik van Riel
2013-11-26 13:16     ` Rik van Riel
2013-11-25 14:26 ` [PATCH 3/5] mm: compaction: detect when scanners meet in isolate_freepages Vlastimil Babka
2013-11-25 14:26   ` Vlastimil Babka
2013-11-26 10:45   ` Mel Gorman
2013-11-26 10:45     ` Mel Gorman
2013-11-26 16:44     ` Vlastimil Babka
2013-11-26 16:44       ` Vlastimil Babka
2013-11-25 14:26 ` [PATCH 4/5] mm: compaction: do not mark unmovable pageblocks as skipped in async compaction Vlastimil Babka
2013-11-25 14:26   ` Vlastimil Babka
2013-11-26 10:58   ` Mel Gorman
2013-11-26 10:58     ` Mel Gorman
2013-11-25 14:26 ` [PATCH 5/5] mm: compaction: reset scanner positions immediately when they meet Vlastimil Babka
2013-11-25 14:26   ` Vlastimil Babka
2013-11-26 11:03   ` Mel Gorman [this message]
2013-11-26 11:03     ` Mel Gorman
2013-12-04 14:30 ` [PATCH] mm: compaction: Trace compaction begin and end Mel Gorman
2013-12-04 14:30   ` Mel Gorman
2013-12-04 14:51   ` Vlastimil Babka
2013-12-04 14:51     ` Vlastimil Babka
2013-12-05  9:05     ` Mel Gorman
2013-12-05  9:05       ` Mel Gorman
2013-12-06  9:50       ` Vlastimil Babka
2013-12-06  9:50         ` Vlastimil Babka
2013-12-05  9:07     ` [PATCH] mm: compaction: Trace compaction begin and end v2 Mel Gorman
2013-12-05  9:07       ` Mel Gorman
2013-12-06  9:50       ` Vlastimil Babka
2013-12-06  9:50         ` 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=20131126110330.GJ5285@suse.de \
    --to=mgorman@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=riel@redhat.com \
    --cc=vbabka@suse.cz \
    /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.