linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrea Arcangeli <aarcange@redhat.com>
To: Mel Gorman <mgorman@suse.de>
Cc: Linux-MM <linux-mm@kvack.org>,
	Minchan Kim <minchan.kim@gmail.com>, Jan Kara <jack@suse.cz>,
	Andy Isaacson <adi@hexapodia.org>,
	Johannes Weiner <jweiner@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/5] mm: compaction: Determine if dirty pages can be migreated without blocking within ->migratepage
Date: Fri, 18 Nov 2011 22:35:30 +0100	[thread overview]
Message-ID: <20111118213530.GA6323@redhat.com> (raw)
In-Reply-To: <1321635524-8586-5-git-send-email-mgorman@suse.de>

On Fri, Nov 18, 2011 at 04:58:43PM +0000, Mel Gorman wrote:
> +	/* async case, we cannot block on lock_buffer so use trylock_buffer */
> +	do {
> +		get_bh(bh);
> +		if (!trylock_buffer(bh)) {
> +			/*
> +			 * We failed to lock the buffer and cannot stall in
> +			 * async migration. Release the taken locks
> +			 */
> +			struct buffer_head *failed_bh = bh;
> +			bh = head;
> +			do {
> +				unlock_buffer(bh);
> +				put_bh(bh);
> +				bh = bh->b_this_page;
> +			} while (bh != failed_bh);
> +			return false;

here if blocksize is < PAGE_SIZE you're leaking one get_bh
(memleak). If blocksize is PAGE_SIZE (common) you're unlocking a
locked bh leading to fs corruption.
> +	if (!buffer_migrate_lock_buffers(head, sync)) {
> +		/*
> +		 * We have to revert the radix tree update. If this returns
> +		 * non-zero, it either means that the page count changed
> +		 * which "can't happen" or the slot changed from underneath
> +		 * us in which case someone operated on a page that did not
> +		 * have buffers fully migrated which is alarming so warn
> +		 * that it happened.
> +		 */
> +		WARN_ON(migrate_page_move_mapping(mapping, page, newpage));

speculative pagecache lookups can actually increase the count, the
freezing is released before returning from
migrate_page_move_mapping. It's not alarming that pagecache lookup
flips bit all over the place. The only way to stop them is the
page_freeze_refs.

folks who wants low latency or no memory overhead should simply
disable compaction. In my tests these "lowlatency" changes, notably
the change in vmscan that is already upstream breaks thp allocation
reliability, the __GFP_NO_KSWAPD check too should be dropped I think,
it's good thing we dropped it because the sync migrate is needed or
the above pages with bh to migrate would become "unmovable" despite
they're allocated in "movable" pageblocks.

The workload to test is:

cp /dev/sda /dev/null &
cp /dev/zero /media/someusb/zero &
wait free memory to reach minimum level
./largepage (allocate some gigabyte of hugepages)
grep thp /proc/vmstat

Anything that leads to a thp allocation failure rate of this workload
of 50% should be banned and all compaction patches (including vmscan
changes) should go through the above workload.

I got back to the previous state and there's <10% of failures even in
the above workload (and close to 100% in normal load but it's harder
to define normal load while the above is pretty easy to define).

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-11-18 21:35 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-18 16:58 [RFC PATCH 0/5] Reduce compaction-related stalls and improve asynchronous migration of dirty pages v3 Mel Gorman
2011-11-18 16:58 ` [PATCH 1/5] mm: compaction: Allow compaction to isolate dirty pages Mel Gorman
2011-11-18 17:28   ` Andrea Arcangeli
2011-11-21 17:16   ` Rik van Riel
2011-11-18 16:58 ` [PATCH 2/5] mm: compaction: Use synchronous compaction for /proc/sys/vm/compact_memory Mel Gorman
2011-11-18 17:27   ` Andrea Arcangeli
2011-11-21 21:46   ` Rik van Riel
2011-11-18 16:58 ` [PATCH 3/5] mm: Do not stall in synchronous compaction for THP allocations Mel Gorman
2011-11-18 17:34   ` Andrea Arcangeli
2011-11-18 16:58 ` [PATCH 4/5] mm: compaction: Determine if dirty pages can be migreated without blocking within ->migratepage Mel Gorman
2011-11-18 21:35   ` Andrea Arcangeli [this message]
2011-11-21 11:17     ` Mel Gorman
2011-11-21 22:45       ` Andrea Arcangeli
2011-11-22  0:55         ` [PATCH] mm: compaction: make buffer cache __GFP_MOVABLE Rik van Riel
2011-11-22 12:59         ` [PATCH 4/5] mm: compaction: Determine if dirty pages can be migreated without blocking within ->migratepage Mel Gorman
2011-11-24  1:19           ` Andrea Arcangeli
2011-11-24 12:21             ` Mel Gorman
2011-11-26  6:51               ` Andy Isaacson
2011-11-27 20:50               ` Rik van Riel
2011-11-19  8:59   ` Nai Xia
2011-11-19  9:48     ` Nai Xia
2011-11-21 11:19     ` Mel Gorman
2011-11-18 16:58 ` [PATCH 5/5] mm: compaction: make isolate_lru_page() filter-aware again Mel Gorman
2011-11-19 19:54 ` [RFC PATCH 0/5] Reduce compaction-related stalls Andrea Arcangeli
2011-11-19 19:54 ` [PATCH 1/8] mm: compaction: Allow compaction to isolate dirty pages Andrea Arcangeli
2011-11-19 19:54 ` [PATCH 2/8] mm: compaction: Use synchronous compaction for /proc/sys/vm/compact_memory Andrea Arcangeli
2011-11-19 19:54 ` [PATCH 3/8] mm: check if we isolated a compound page during lumpy scan Andrea Arcangeli
2011-11-21 11:51   ` Mel Gorman
2011-11-19 19:54 ` [PATCH 4/8] mm: compaction: defer compaction only with sync_migration Andrea Arcangeli
2011-11-21 12:36   ` Mel Gorman
2011-11-19 19:54 ` [PATCH 5/8] mm: compaction: avoid overwork in migrate sync mode Andrea Arcangeli
2011-11-21 21:59   ` Rik van Riel
2011-11-22  9:51     ` Mel Gorman
2011-11-19 19:54 ` [PATCH 6/8] Revert "mm: compaction: make isolate_lru_page() filter-aware" Andrea Arcangeli
2011-11-21 12:57   ` Mel Gorman
2011-11-19 19:54 ` [PATCH 7/8] Revert "vmscan: abort reclaim/compaction if compaction can proceed" Andrea Arcangeli
2011-11-21 13:09   ` Mel Gorman
2011-11-21 15:37     ` Rik van Riel
2011-11-19 19:54 ` [PATCH 8/8] Revert "vmscan: limit direct reclaim for higher order allocations" Andrea Arcangeli
2011-11-21 21:57   ` Rik van Riel

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=20111118213530.GA6323@redhat.com \
    --to=aarcange@redhat.com \
    --cc=adi@hexapodia.org \
    --cc=jack@suse.cz \
    --cc=jweiner@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=minchan.kim@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 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).