| Message ID | 20220320051334.44502-2-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few fixup patches for memory failure | expand |
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)) > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > 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 > >
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@huawei.com> >> --- >> 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 >> >> > > . >
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.
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. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/memory-failure.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)