Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Yang Shi <shy828301@gmail.com>
To:     naoya.horiguchi@nec.com, hughd@google.com,
        kirill.shutemov@linux.intel.com, willy@infradead.org,
        osalvador@suse.de, akpm@linux-foundation.org
Cc:     shy828301@gmail.com, linux-mm@kvack.org,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 2/4] mm: khugepaged: check if file page is on LRU after locking page
Date:   Tue, 14 Sep 2021 11:37:16 -0700
Message-Id: <20210914183718.4236-3-shy828301@gmail.com>
In-Reply-To: <20210914183718.4236-1-shy828301@gmail.com>
References: <20210914183718.4236-1-shy828301@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

The khugepaged does check if the page is on LRU or not but it doesn't
hold page lock.  And it doesn't check this again after holding page
lock.  So it may race with some others, e.g. reclaimer, migration, etc.
All of them isolates page from LRU then lock the page then do something.

But it could pass the refcount check done by khugepaged to proceed
collapse.  Typically such race is not fatal.  But if the page has been
isolated from LRU before khugepaged it likely means the page may be not
suitable for collapse for now.

The other more fatal case is the following patch will keep the poisoned
page in page cache for shmem, so khugepaged may collapse a poisoned page
since the refcount check could pass.  3 refcounts come from:
  - hwpoison
  - page cache
  - khugepaged

Since it is not on LRU so no refcount is incremented from LRU isolation.

This is definitely not expected.  Checking if it is on LRU or not after
holding page lock could help serialize against hwpoison handler.

But there is still a small race window between setting hwpoison flag and
bump refcount in hwpoison handler.  It could be closed by checking
hwpoison flag in khugepaged, however this race seems unlikely to happen
in real life workload.  So just check LRU flag for now to avoid
over-engineering.

Signed-off-by: Yang Shi <shy828301@gmail.com>
---
 mm/khugepaged.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 045cc579f724..bdc161dc27dc 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1808,6 +1808,12 @@ static void collapse_file(struct mm_struct *mm,
 			goto out_unlock;
 		}
 
+		/* The hwpoisoned page is off LRU but in page cache */
+		if (!PageLRU(page)) {
+			result = SCAN_PAGE_LRU;
+			goto out_unlock;
+		}
+
 		if (isolate_lru_page(page)) {
 			result = SCAN_DEL_PAGE_LRU;
 			goto out_unlock;
-- 
2.26.2