From: Mel Gorman <mgorman@suse.de>
To: Andrea Arcangeli <aarcange@redhat.com>
Cc: Minchan Kim <minchan@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Shaohua Li <shli@kernel.org>,
linux-mm@kvack.org
Subject: Re: [patch 1/2 v2]compaction: abort compaction loop if lock is contended or run too long
Date: Thu, 13 Sep 2012 17:31:51 +0100 [thread overview]
Message-ID: <20120913163151.GC11266@suse.de> (raw)
In-Reply-To: <20120913160432.GG3388@redhat.com>
On Thu, Sep 13, 2012 at 06:04:32PM +0200, Andrea Arcangeli wrote:
> On Thu, Sep 13, 2012 at 10:38:26AM +0100, Mel Gorman wrote:
> > I agree with Minchan. Andrea's patch ignores the fact that free page
> > isolation might have aborted due to lock contention. It's not necessarily
> > going to be isolating the pages it needs for migration.
>
> Actually I thought of calling putback_lru_pages first, but then I
> thought it was better to just complete the current slice.
>
Unfortunately that will end up calling compaction_alloc() ->
isolate_freepages and probably end up contending again.
> Note that putback_lru_pages can take the lru_lock immediately too when
True, but in that case there is no choice in the matter. We can't just
leak the pages.
> the pagevec gets full which won't work any better than if the
> cc->contended was set by the freepages isolation and we do
> migrate_pages.
>
> There's no way to abort lockless from that point, so I think it's
> better to take the last locks to finish the current slice of work and
> then abort if it's still contended (which confirms we're really
> trashing).
>
To me, that will just contend more than we have to. We're aborting compaction
and finishing off the current slice will not make any meaningful difference
to whether tha allocation succeeds or not.
> Skipping isolated pages without rewinding low_pfn would also reduce
> compaction reliability so that should be evaluated as well. And
> rewinding with the putback_lru_pages would risk livelocks.
>
> I agree Minchan's patch would fix the problem too, and this should be
> a fairly uncommon path so either ways shouldn't make a noticeable
> difference.
--
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>
next prev parent reply other threads:[~2012-09-13 16:31 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-10 1:18 [patch 1/2 v2]compaction: abort compaction loop if lock is contended or run too long Shaohua Li
2012-09-10 8:11 ` Mel Gorman
2012-09-11 1:45 ` Minchan Kim
2012-09-11 8:29 ` Shaohua Li
2012-09-11 8:40 ` Minchan Kim
2012-09-11 23:34 ` Andrew Morton
2012-09-12 0:48 ` Andrea Arcangeli
2012-09-12 21:20 ` Andrew Morton
2012-09-12 23:48 ` Andrea Arcangeli
2012-09-13 0:47 ` Minchan Kim
2012-09-13 2:49 ` Shaohua Li
2012-09-13 9:38 ` Mel Gorman
2012-09-13 10:13 ` Shaohua Li
2012-09-13 16:04 ` Andrea Arcangeli
2012-09-13 16:31 ` Mel Gorman [this message]
2012-09-13 17:11 ` Andrea Arcangeli
2012-09-13 10:03 ` Mel Gorman
2012-09-12 11:08 ` Mel Gorman
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=20120913163151.GC11266@suse.de \
--to=mgorman@suse.de \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=shli@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).