Johannes Weiner has suggested:
"
So here is a crazy idea that may be worth exploring:
Right now, pgdat->lru_lock protects both PageLRU *and* the lruvec's
linked list.
Can we make PageLRU atomic and use it to stabilize the lru_lock
instead, and then use the lru_lock only serialize list operations?
...
"
Yes, this patch is doing so on __isolate_lru_page which is the core
page isolation func in compaction and shrinking path.
This patch move clear page lru action before compaction getting lru_lock,
makes it as a necessary condition for page isolation. Hence, PageLRU may
be cleared druing shrink_inactive_list path for isolation reason. If so,
we can skip that page's in reclaim.
It's a preparation for later per memcg lru_lock change.
Suggested-by: Johannes Weiner <hannes@xxxxxxxxxxx>
Signed-off-by: Alex Shi <alex.shi@xxxxxxxxxxxxxxxxx>
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
Cc: Hugh Dickins <hughd@xxxxxxxxxx>
Cc: linux-kernel@xxxxxxxxxxxxxxx
Cc: linux-mm@xxxxxxxxx
---
include/linux/swap.h | 2 +-
mm/compaction.c | 25 +++++++++++++++++--------
mm/vmscan.c | 48 ++++++++++++++++++++++++++----------------------
3 files changed, 44 insertions(+), 31 deletions(-)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index c555e8f161ad..69f0794f1da3 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -351,7 +351,7 @@ extern void lru_cache_add_active_or_unevictable(struct page *page,
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,
diff --git a/mm/compaction.c b/mm/compaction.c
index 672d3c78c6ab..1baba328d089 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -948,6 +948,23 @@ static bool too_many_isolated(pg_data_t *pgdat)
if (!(cc->gfp_mask & __GFP_FS) && page_mapping(page))
goto isolate_fail;
+ if (__isolate_lru_page_prepare(page, isolate_mode) != 0)
+ 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;
+
+ /* Try isolate the page */
+ if (!TestClearPageLRU(page)) {
+ put_page(page);
+ goto isolate_fail;
+ }
+
/* If we already hold the lock, we can skip some rechecking */
if (!locked) {
locked = compact_lock_irqsave(&pgdat->lru_lock,
@@ -960,10 +977,6 @@ static bool too_many_isolated(pg_data_t *pgdat)
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
@@ -977,10 +990,6 @@ static bool too_many_isolated(pg_data_t *pgdat)
lruvec = mem_cgroup_page_lruvec(page, pgdat);
- /* Try isolate the page */
- if (__isolate_lru_page(page, isolate_mode) != 0)
- goto isolate_fail;
-
VM_BUG_ON_PAGE(PageCompound(page), page);
/* Successfully isolated */
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 8958454d50fe..bc2ec3fe4f48 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1522,20 +1522,20 @@ unsigned long reclaim_clean_pages_from_list(struct zone *zone,
*
* 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 = -EINVAL;
- /* Only take pages on the LRU. */
- if (!PageLRU(page))
- return ret;
-
/* Compaction should not handle unevictable pages but CMA can do so */
if (PageUnevictable(page) && !(mode & ISOLATE_UNEVICTABLE))
return ret;
ret = -EBUSY;
+ /* Only take pages on the LRU. */
+ if (!PageLRU(page))
+ return ret;
+
/*
* To minimise LRU disruption, the caller can indicate that it only
* wants to isolate pages it will be able to operate on without
@@ -1576,20 +1576,9 @@ int __isolate_lru_page(struct page *page, isolate_mode_t mode)
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.
- */
- ClearPageLRU(page);
- ret = 0;
- }
-
- 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 santity check.
@@ -1653,8 +1642,6 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
page = lru_to_page(src);
prefetchw_prev_lru_page(page, src, flags);
- VM_BUG_ON_PAGE(!PageLRU(page), page);
-
nr_pages = compound_nr(page);
total_scan += nr_pages;
@@ -1675,17 +1662,34 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
* 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;
-
+busy:
case -EBUSY:
/* else it is being freed elsewhere */
list_move(&page->lru, src);
- continue;
+ break;
default:
BUG();
--
1.8.3.1
