On 2022/3/20 0:20, Matthew Wilcox wrote: > On Sun, Mar 20, 2022 at 01:13:33PM +0800, Miaohe Lin wrote: >> invalidate_inode_page() can invalidate the pages in the swap cache because >> the check of page->mapping != mapping is removed via Matthew's patch titled >> "mm/truncate: Inline invalidate_complete_page() into its one caller". But >> invalidate_inode_page() is not expected to deal with the pages in the swap >> cache. Also non-lru movable page can reach here too. They're not page cache >> pages. Skip these pages by checking PageSwapCache and PageLRU to fix this >> unexpected issue. > > I disagree with this changelog. > > invalidate_inode_page() should not be called for pages which are not > in the page cache. > > And then the patch shouldn't test PageLRU (which is actually wrong) or > PageSwapCache(). It should simply be: > > + if (!PageHuge(page) && !PageAnon(page)) If we reach here, the page can be one of the PageHuge, __PageMovable and PageLRU. If above code is used, __PageMovable can pass the check and enter the invalidate_inode_page unexpectedly. So I think PageLRU check is necessary here. But it seems PageSwapCache is only reliable when PageAnon, so might we should do: + if (!PageHuge(page) && PageLRU(page) && !PageAnon(page)) Am I miss something? Many thanks for comment. > >> Signed-off-by: Miaohe Lin <linmiaohe@xxxxxxxxxx> >> --- >> mm/memory-failure.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c >> index 5444a8ef4867..ecf45961f3b6 100644 >> --- a/mm/memory-failure.c >> +++ b/mm/memory-failure.c >> @@ -2178,7 +2178,7 @@ static int __soft_offline_page(struct page *page) >> return 0; >> } >> >> - if (!PageHuge(page)) >> + if (!PageHuge(page) && PageLRU(page) && !PageSwapCache(page)) >> /* >> * Try to invalidate first. This should work for >> * non dirty unmapped page cache pages. >> -- >> 2.23.0 >> >> > > . >
