linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: aarcange@redhat.com, akpm@linux-foundation.org,
	alex.shi@linux.alibaba.com, alexander.duyck@gmail.com,
	aryabinin@virtuozzo.com, daniel.m.jordan@oracle.com,
	hannes@cmpxchg.org, hughd@google.com, iamjoonsoo.kim@lge.com,
	jannh@google.com, khlebnikov@yandex-team.ru,
	kirill.shutemov@linux.intel.com, kirill@shutemov.name,
	linux-mm@kvack.org, mgorman@techsingularity.net,
	mhocko@kernel.org, mhocko@suse.com, mika.penttila@nextfour.com,
	minchan@kernel.org, mm-commits@vger.kernel.org,
	richard.weiyang@gmail.com, rong.a.chen@intel.com,
	shakeelb@google.com, tglx@linutronix.de, tj@kernel.org,
	torvalds@linux-foundation.org, vbabka@suse.cz,
	vdavydov.dev@gmail.com, willy@infradead.org,
	yang.shi@linux.alibaba.com, ying.huang@intel.com
Subject: [patch 15/19] mm/compaction: do page isolation first in compaction
Date: Tue, 15 Dec 2020 12:34:20 -0800	[thread overview]
Message-ID: <20201215203420.HrcA96Ocz%akpm@linux-foundation.org> (raw)
In-Reply-To: <20201215123253.954eca9a5ef4c0d52fd381fa@linux-foundation.org>

From: Alex Shi <alex.shi@linux.alibaba.com>
Subject: mm/compaction: do page isolation first in compaction

Currently, compaction would get the lru_lock and then do page isolation
which works fine with pgdat->lru_lock, since any page isoltion would
compete for the lru_lock.  If we want to change to memcg lru_lock, we have
to isolate the page before getting lru_lock, thus isoltion would block
page's memcg change which relay on page isoltion too.  Then we could
safely use per memcg lru_lock later.

The new page isolation use previous introduced TestClearPageLRU() + pgdat
lru locking which will be changed to memcg lru lock later.

Hugh Dickins <hughd@google.com> fixed following bugs in this patch's early
version:

Fix lots of crashes under compaction load: isolate_migratepages_block()
must clean up appropriately when rejecting a page, setting PageLRU again
if it had been cleared; and a put_page() after get_page_unless_zero()
cannot safely be done while holding locked_lruvec - it may turn out to be
the final put_page(), which will take an lruvec lock when PageLRU.

And move __isolate_lru_page_prepare back after get_page_unless_zero to
make trylock_page() safe: trylock_page() is not safe to use at this time:
its setting PG_locked can race with the page being freed or allocated
("Bad page"), and can also erase flags being set by one of those "sole
owners" of a freshly allocated page who use non-atomic __SetPageFlag().

Link: https://lkml.kernel.org/r/1604566549-62481-16-git-send-email-alex.shi@linux.alibaba.com
Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com>
Acked-by: Hugh Dickins <hughd@google.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Alexander Duyck <alexander.duyck@gmail.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: "Chen, Rong A" <rong.a.chen@intel.com>
Cc: Daniel Jordan <daniel.m.jordan@oracle.com>
Cc: "Huang, Ying" <ying.huang@intel.com>
Cc: Jann Horn <jannh@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mika Penttilä <mika.penttila@nextfour.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Shakeel Butt <shakeelb@google.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: Yang Shi <yang.shi@linux.alibaba.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/swap.h |    2 -
 mm/compaction.c      |   42 +++++++++++++++++++++++++++++++---------
 mm/vmscan.c          |   43 ++++++++++++++++++++---------------------
 3 files changed, 56 insertions(+), 31 deletions(-)

--- a/include/linux/swap.h~mm-compaction-do-page-isolation-first-in-compaction
+++ a/include/linux/swap.h
@@ -356,7 +356,7 @@ extern void lru_cache_add_inactive_or_un
 extern unsigned long zone_reclaimable_pages(struct zone *zone);
 extern unsigned long try_to_free_pages(struct zonelist *zonelist, int order,
 					gfp_t gfp_mask, nodemask_t *mask);
-extern int __isolate_lru_page(struct page *page, isolate_mode_t mode);
+extern int __isolate_lru_page_prepare(struct page *page, isolate_mode_t mode);
 extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg,
 						  unsigned long nr_pages,
 						  gfp_t gfp_mask,
--- a/mm/compaction.c~mm-compaction-do-page-isolation-first-in-compaction
+++ a/mm/compaction.c
@@ -890,6 +890,7 @@ isolate_migratepages_block(struct compac
 		if (!valid_page && IS_ALIGNED(low_pfn, pageblock_nr_pages)) {
 			if (!cc->ignore_skip_hint && get_pageblock_skip(page)) {
 				low_pfn = end_pfn;
+				page = NULL;
 				goto isolate_abort;
 			}
 			valid_page = page;
@@ -971,6 +972,21 @@ isolate_migratepages_block(struct compac
 		if (!(cc->gfp_mask & __GFP_FS) && page_mapping(page))
 			goto isolate_fail;
 
+		/*
+		 * Be careful not to clear PageLRU until after we're
+		 * sure the page is not being freed elsewhere -- the
+		 * page release code relies on it.
+		 */
+		if (unlikely(!get_page_unless_zero(page)))
+			goto isolate_fail;
+
+		if (__isolate_lru_page_prepare(page, isolate_mode) != 0)
+			goto isolate_fail_put;
+
+		/* Try isolate the page */
+		if (!TestClearPageLRU(page))
+			goto isolate_fail_put;
+
 		/* If we already hold the lock, we can skip some rechecking */
 		if (!locked) {
 			locked = compact_lock_irqsave(&pgdat->lru_lock,
@@ -983,10 +999,6 @@ isolate_migratepages_block(struct compac
 					goto isolate_abort;
 			}
 
-			/* Recheck PageLRU and PageCompound under lock */
-			if (!PageLRU(page))
-				goto isolate_fail;
-
 			/*
 			 * Page become compound since the non-locked check,
 			 * and it's on LRU. It can only be a THP so the order
@@ -994,16 +1006,13 @@ isolate_migratepages_block(struct compac
 			 */
 			if (unlikely(PageCompound(page) && !cc->alloc_contig)) {
 				low_pfn += compound_nr(page) - 1;
-				goto isolate_fail;
+				SetPageLRU(page);
+				goto isolate_fail_put;
 			}
 		}
 
 		lruvec = mem_cgroup_page_lruvec(page, pgdat);
 
-		/* Try isolate the page */
-		if (__isolate_lru_page(page, isolate_mode) != 0)
-			goto isolate_fail;
-
 		/* The whole page is taken off the LRU; skip the tail pages. */
 		if (PageCompound(page))
 			low_pfn += compound_nr(page) - 1;
@@ -1032,6 +1041,15 @@ isolate_success:
 		}
 
 		continue;
+
+isolate_fail_put:
+		/* Avoid potential deadlock in freeing page under lru_lock */
+		if (locked) {
+			spin_unlock_irqrestore(&pgdat->lru_lock, flags);
+			locked = false;
+		}
+		put_page(page);
+
 isolate_fail:
 		if (!skip_on_failure)
 			continue;
@@ -1068,9 +1086,15 @@ isolate_fail:
 	if (unlikely(low_pfn > end_pfn))
 		low_pfn = end_pfn;
 
+	page = NULL;
+
 isolate_abort:
 	if (locked)
 		spin_unlock_irqrestore(&pgdat->lru_lock, flags);
+	if (page) {
+		SetPageLRU(page);
+		put_page(page);
+	}
 
 	/*
 	 * Updated the cached scanner pfn once the pageblock has been scanned
--- a/mm/vmscan.c~mm-compaction-do-page-isolation-first-in-compaction
+++ a/mm/vmscan.c
@@ -1539,7 +1539,7 @@ unsigned int reclaim_clean_pages_from_li
  *
  * returns 0 on success, -ve errno on failure.
  */
-int __isolate_lru_page(struct page *page, isolate_mode_t mode)
+int __isolate_lru_page_prepare(struct page *page, isolate_mode_t mode)
 {
 	int ret = -EBUSY;
 
@@ -1591,22 +1591,9 @@ int __isolate_lru_page(struct page *page
 	if ((mode & ISOLATE_UNMAPPED) && page_mapped(page))
 		return ret;
 
-	if (likely(get_page_unless_zero(page))) {
-		/*
-		 * Be careful not to clear PageLRU until after we're
-		 * sure the page is not being freed elsewhere -- the
-		 * page release code relies on it.
-		 */
-		if (TestClearPageLRU(page))
-			ret = 0;
-		else
-			put_page(page);
-	}
-
-	return ret;
+	return 0;
 }
 
-
 /*
  * Update LRU sizes after isolating pages. The LRU size updates must
  * be complete before mem_cgroup_update_lru_size due to a sanity check.
@@ -1686,20 +1673,34 @@ static unsigned long isolate_lru_pages(u
 		 * only when the page is being freed somewhere else.
 		 */
 		scan += nr_pages;
-		switch (__isolate_lru_page(page, mode)) {
+		switch (__isolate_lru_page_prepare(page, mode)) {
 		case 0:
+			/*
+			 * Be careful not to clear PageLRU until after we're
+			 * sure the page is not being freed elsewhere -- the
+			 * page release code relies on it.
+			 */
+			if (unlikely(!get_page_unless_zero(page)))
+				goto busy;
+
+			if (!TestClearPageLRU(page)) {
+				/*
+				 * This page may in other isolation path,
+				 * but we still hold lru_lock.
+				 */
+				put_page(page);
+				goto busy;
+			}
+
 			nr_taken += nr_pages;
 			nr_zone_taken[page_zonenum(page)] += nr_pages;
 			list_move(&page->lru, dst);
 			break;
 
-		case -EBUSY:
+		default:
+busy:
 			/* else it is being freed elsewhere */
 			list_move(&page->lru, src);
-			continue;
-
-		default:
-			BUG();
 		}
 	}
 
_


  parent reply	other threads:[~2020-12-15 20:34 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-15 20:32 incoming Andrew Morton
2020-12-15 20:33 ` [patch 01/19] mm/thp: move lru_add_page_tail() to huge_memory.c Andrew Morton
2020-12-15 20:33 ` [patch 02/19] mm/thp: use head for head page in lru_add_page_tail() Andrew Morton
2020-12-15 20:33 ` [patch 03/19] mm/thp: simplify lru_add_page_tail() Andrew Morton
2020-12-15 20:33 ` [patch 04/19] mm/thp: narrow lru locking Andrew Morton
2020-12-15 20:33 ` [patch 05/19] mm/vmscan: remove unnecessary lruvec adding Andrew Morton
2020-12-15 20:33 ` [patch 06/19] mm/rmap: stop store reordering issue on page->mapping Andrew Morton
2020-12-15 20:33 ` [patch 07/19] mm: page_idle_get_page() does not need lru_lock Andrew Morton
2020-12-15 20:33 ` [patch 08/19] mm/memcg: add debug checking in lock_page_memcg Andrew Morton
2020-12-15 20:33 ` [patch 09/19] mm/swap.c: fold vm event PGROTATED into pagevec_move_tail_fn Andrew Morton
2020-12-15 20:34 ` [patch 11/19] mm/vmscan: remove lruvec reget in move_pages_to_lru Andrew Morton
2020-12-15 20:34 ` [patch 12/19] mm/mlock: remove lru_lock on TestClearPageMlocked Andrew Morton
2020-12-15 20:34 ` [patch 13/19] mm/mlock: remove __munlock_isolate_lru_page() Andrew Morton
2020-12-15 20:34 ` [patch 14/19] mm/lru: introduce TestClearPageLRU() Andrew Morton
2020-12-15 20:34 ` Andrew Morton [this message]
2020-12-15 20:34 ` [patch 16/19] mm/swap.c: serialize memcg changes in pagevec_lru_move_fn Andrew Morton
2020-12-15 20:34 ` [patch 17/19] mm/lru: replace pgdat lru_lock with lruvec lock Andrew Morton
2020-12-15 20:34 ` [patch 18/19] mm/lru: introduce relock_page_lruvec() Andrew Morton
2020-12-15 20:34 ` [patch 19/19] mm/lru: revise the comments of lru_lock Andrew Morton
2020-12-15 21:00 ` incoming Linus Torvalds
2020-12-15 22:20 ` [patch 01/19] mm/thp: move lru_add_page_tail() to huge_memory.c Andrew Morton
2020-12-15 22:20 ` [patch 02/19] mm/thp: use head for head page in lru_add_page_tail() Andrew Morton
2020-12-15 22:20 ` [patch 03/19] mm/thp: simplify lru_add_page_tail() Andrew Morton
2020-12-15 22:20 ` [patch 04/19] mm/thp: narrow lru locking Andrew Morton
2020-12-15 22:20 ` [patch 05/19] mm/vmscan: remove unnecessary lruvec adding Andrew Morton
2020-12-15 22:20 ` [patch 06/19] mm/rmap: stop store reordering issue on page->mapping Andrew Morton
2020-12-15 22:20 ` [patch 07/19] mm: page_idle_get_page() does not need lru_lock Andrew Morton
2020-12-15 22:20 ` [patch 08/19] mm/memcg: add debug checking in lock_page_memcg Andrew Morton
2020-12-15 22:20 ` [patch 09/19] mm/swap.c: fold vm event PGROTATED into pagevec_move_tail_fn Andrew Morton
2020-12-15 22:20 ` [patch 10/19] mm/lru: move lock into lru_note_cost Andrew Morton
2020-12-15 22:20 ` [patch 11/19] mm/vmscan: remove lruvec reget in move_pages_to_lru Andrew Morton
2020-12-15 22:20 ` [patch 12/19] mm/mlock: remove lru_lock on TestClearPageMlocked Andrew Morton
2020-12-15 22:21 ` [patch 13/19] mm/mlock: remove __munlock_isolate_lru_page() Andrew Morton
2020-12-15 22:21 ` [patch 14/19] mm/lru: introduce TestClearPageLRU() Andrew Morton
2020-12-15 22:21 ` [patch 15/19] mm/compaction: do page isolation first in compaction Andrew Morton
2020-12-15 22:21 ` [patch 16/19] mm/swap.c: serialize memcg changes in pagevec_lru_move_fn Andrew Morton
2020-12-15 22:21 ` [patch 17/19] mm/lru: replace pgdat lru_lock with lruvec lock Andrew Morton
2020-12-15 22:21 ` [patch 18/19] mm/lru: introduce relock_page_lruvec() Andrew Morton
2020-12-15 22:21 ` [patch 19/19] mm/lru: revise the comments of lru_lock Andrew Morton
2020-12-15 22:48 ` incoming Linus Torvalds
2020-12-15 22:49   ` incoming Linus Torvalds
2020-12-15 22:55     ` incoming Andrew Morton

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=20201215203420.HrcA96Ocz%akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=aarcange@redhat.com \
    --cc=alex.shi@linux.alibaba.com \
    --cc=alexander.duyck@gmail.com \
    --cc=aryabinin@virtuozzo.com \
    --cc=daniel.m.jordan@oracle.com \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=jannh@google.com \
    --cc=khlebnikov@yandex-team.ru \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kirill@shutemov.name \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@kernel.org \
    --cc=mhocko@suse.com \
    --cc=mika.penttila@nextfour.com \
    --cc=minchan@kernel.org \
    --cc=mm-commits@vger.kernel.org \
    --cc=richard.weiyang@gmail.com \
    --cc=rong.a.chen@intel.com \
    --cc=shakeelb@google.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=vbabka@suse.cz \
    --cc=vdavydov.dev@gmail.com \
    --cc=willy@infradead.org \
    --cc=yang.shi@linux.alibaba.com \
    --cc=ying.huang@intel.com \
    --subject='Re: [patch 15/19] mm/compaction: do page isolation first in compaction' \
    /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

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox