| Message ID | 20220312074613.4798-3-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few fixup patches for memory failure | expand |
On Sat, Mar 12, 2022 at 03:46:12PM +0800, Miaohe Lin wrote: > Since commit 042c4f32323b ("mm/truncate: Inline invalidate_complete_page() This commit ID does not exist in mainline (or in the latest mmotm?), so you can't use it in patch description. Could you update this part? Thanks, Naoya Horiguchi > into its one caller"), invalidate_inode_page() can invalidate the pages in > the swap cache because the check of page->mapping != mapping is removed. > But invalidate_inode_page() is not expected to deal with the pages in 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. > > 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 dabecd87ad3f..2ff7dd2078c4 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -2190,7 +2190,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/14 7:41, HORIGUCHI NAOYA(堀口 直也) wrote: > On Sat, Mar 12, 2022 at 03:46:12PM +0800, Miaohe Lin wrote: >> Since commit 042c4f32323b ("mm/truncate: Inline invalidate_complete_page() > > This commit ID does not exist in mainline (or in the latest mmotm?), > so you can't use it in patch description. Could you update this part? > This commit is in the mmotm but not in mainline yet: commit 042c4f32323beb28146c658202d3e69899e4f245 Author: Matthew Wilcox (Oracle) <willy@infradead.org> Date: Sat Feb 12 15:27:42 2022 -0500 mm/truncate: Inline invalidate_complete_page() into its one caller invalidate_inode_page() is the only caller of invalidate_complete_page() and inlining it reveals that the first check is unnecessary (because we hold the page locked, and we just retrieved the mapping from the page). Actually, it does make a difference, in that tail pages no longer fail at this check, so it's now possible to remove a tail page from a mapping. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Reviewed-by: John Hubbard <jhubbard@nvidia.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Am I "not" supposed to use this commit id as it's not "stable" now? Will update this part in next version. Many thanks. > Thanks, > Naoya Horiguchi > >> into its one caller"), invalidate_inode_page() can invalidate the pages in >> the swap cache because the check of page->mapping != mapping is removed. >> But invalidate_inode_page() is not expected to deal with the pages in 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. >> >> 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 dabecd87ad3f..2ff7dd2078c4 100644 >> --- a/mm/memory-failure.c >> +++ b/mm/memory-failure.c >> @@ -2190,7 +2190,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 Mon, Mar 14, 2022 at 09:58:49AM +0800, Miaohe Lin wrote: > On 2022/3/14 7:41, HORIGUCHI NAOYA(堀口 直也) wrote: > > On Sat, Mar 12, 2022 at 03:46:12PM +0800, Miaohe Lin wrote: > >> Since commit 042c4f32323b ("mm/truncate: Inline invalidate_complete_page() > > > > This commit ID does not exist in mainline (or in the latest mmotm?), > > so you can't use it in patch description. Could you update this part? > > > > This commit is in the mmotm but not in mainline yet: > > commit 042c4f32323beb28146c658202d3e69899e4f245 > Author: Matthew Wilcox (Oracle) <willy@infradead.org> > Date: Sat Feb 12 15:27:42 2022 -0500 > > mm/truncate: Inline invalidate_complete_page() into its one caller > > invalidate_inode_page() is the only caller of invalidate_complete_page() > and inlining it reveals that the first check is unnecessary (because we > hold the page locked, and we just retrieved the mapping from the page). > Actually, it does make a difference, in that tail pages no longer fail > at this check, so it's now possible to remove a tail page from a mapping. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > Reviewed-by: John Hubbard <jhubbard@nvidia.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > > Am I "not" supposed to use this commit id as it's not "stable" now? No, it's not stable yet. In whatever way you get the above commit (I guess you get it from https://github.com/hnaz/linux-mm), all acked mm-related patches are sent to Linus by Andrew *by email*, so the eventual commit IDs should be determined when they are applied to mainline. Thanks, Naoya Horiguchi
On 2022/3/14 10:50, HORIGUCHI NAOYA(堀口 直也) wrote: > On Mon, Mar 14, 2022 at 09:58:49AM +0800, Miaohe Lin wrote: >> On 2022/3/14 7:41, HORIGUCHI NAOYA(堀口 直也) wrote: >>> On Sat, Mar 12, 2022 at 03:46:12PM +0800, Miaohe Lin wrote: >>>> Since commit 042c4f32323b ("mm/truncate: Inline invalidate_complete_page() >>> >>> This commit ID does not exist in mainline (or in the latest mmotm?), >>> so you can't use it in patch description. Could you update this part? >>> >> >> This commit is in the mmotm but not in mainline yet: >> >> commit 042c4f32323beb28146c658202d3e69899e4f245 >> Author: Matthew Wilcox (Oracle) <willy@infradead.org> >> Date: Sat Feb 12 15:27:42 2022 -0500 >> >> mm/truncate: Inline invalidate_complete_page() into its one caller >> >> invalidate_inode_page() is the only caller of invalidate_complete_page() >> and inlining it reveals that the first check is unnecessary (because we >> hold the page locked, and we just retrieved the mapping from the page). >> Actually, it does make a difference, in that tail pages no longer fail >> at this check, so it's now possible to remove a tail page from a mapping. >> >> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> >> Reviewed-by: John Hubbard <jhubbard@nvidia.com> >> Reviewed-by: Christoph Hellwig <hch@lst.de> >> >> Am I "not" supposed to use this commit id as it's not "stable" now? > > No, it's not stable yet. In whatever way you get the above commit (I guess > you get it from https://github.com/hnaz/linux-mm), all acked mm-related > patches are sent to Linus by Andrew *by email*, so the eventual commit IDs > should be determined when they are applied to mainline. > Many thanks for your explanation. (I get this commit id from linux-next tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git) So I should remember always to get the commit id from mainline. Thanks again. :) > Thanks, > Naoya Horiguchi >
On Mon, 14 Mar 2022 10:59:40 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote: > On 2022/3/14 10:50, HORIGUCHI NAOYA(堀口 直也) wrote: > > On Mon, Mar 14, 2022 at 09:58:49AM +0800, Miaohe Lin wrote: > >> On 2022/3/14 7:41, HORIGUCHI NAOYA(堀口 直也) wrote: > >>> On Sat, Mar 12, 2022 at 03:46:12PM +0800, Miaohe Lin wrote: > >>>> Since commit 042c4f32323b ("mm/truncate: Inline invalidate_complete_page() > >>> > >>> This commit ID does not exist in mainline (or in the latest mmotm?), > >>> so you can't use it in patch description. Could you update this part? > >>> > >> > >> This commit is in the mmotm but not in mainline yet: > >> > >> commit 042c4f32323beb28146c658202d3e69899e4f245 > >> Author: Matthew Wilcox (Oracle) <willy@infradead.org> > >> Date: Sat Feb 12 15:27:42 2022 -0500 > >> > >> mm/truncate: Inline invalidate_complete_page() into its one caller > >> > >> invalidate_inode_page() is the only caller of invalidate_complete_page() > >> and inlining it reveals that the first check is unnecessary (because we > >> hold the page locked, and we just retrieved the mapping from the page). > >> Actually, it does make a difference, in that tail pages no longer fail > >> at this check, so it's now possible to remove a tail page from a mapping. > >> > >> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > >> Reviewed-by: John Hubbard <jhubbard@nvidia.com> > >> Reviewed-by: Christoph Hellwig <hch@lst.de> > >> > >> Am I "not" supposed to use this commit id as it's not "stable" now? > > > > No, it's not stable yet. In whatever way you get the above commit (I guess > > you get it from https://github.com/hnaz/linux-mm), all acked mm-related > > patches are sent to Linus by Andrew *by email*, so the eventual commit IDs > > should be determined when they are applied to mainline. > > > > Many thanks for your explanation. (I get this commit id from linux-next tree: > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git) > So I should remember always to get the commit id from mainline. It's likely that this commit ID will be the same once Matthew's patch goes into mainline. But this is why we include the patch title ("mm/truncate: Inline ...") when identifying commits. Sometimes stuff happens...
On 2022/3/15 7:45, Andrew Morton wrote: > On Mon, 14 Mar 2022 10:59:40 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote: > >> On 2022/3/14 10:50, HORIGUCHI NAOYA(堀口 直也) wrote: >>> On Mon, Mar 14, 2022 at 09:58:49AM +0800, Miaohe Lin wrote: >>>> On 2022/3/14 7:41, HORIGUCHI NAOYA(堀口 直也) wrote: >>>>> On Sat, Mar 12, 2022 at 03:46:12PM +0800, Miaohe Lin wrote: >>>>>> Since commit 042c4f32323b ("mm/truncate: Inline invalidate_complete_page() >>>>> >>>>> This commit ID does not exist in mainline (or in the latest mmotm?), >>>>> so you can't use it in patch description. Could you update this part? >>>>> >>>> >>>> This commit is in the mmotm but not in mainline yet: >>>> >>>> commit 042c4f32323beb28146c658202d3e69899e4f245 >>>> Author: Matthew Wilcox (Oracle) <willy@infradead.org> >>>> Date: Sat Feb 12 15:27:42 2022 -0500 >>>> >>>> mm/truncate: Inline invalidate_complete_page() into its one caller >>>> >>>> invalidate_inode_page() is the only caller of invalidate_complete_page() >>>> and inlining it reveals that the first check is unnecessary (because we >>>> hold the page locked, and we just retrieved the mapping from the page). >>>> Actually, it does make a difference, in that tail pages no longer fail >>>> at this check, so it's now possible to remove a tail page from a mapping. >>>> >>>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> >>>> Reviewed-by: John Hubbard <jhubbard@nvidia.com> >>>> Reviewed-by: Christoph Hellwig <hch@lst.de> >>>> >>>> Am I "not" supposed to use this commit id as it's not "stable" now? >>> >>> No, it's not stable yet. In whatever way you get the above commit (I guess >>> you get it from https://github.com/hnaz/linux-mm), all acked mm-related >>> patches are sent to Linus by Andrew *by email*, so the eventual commit IDs >>> should be determined when they are applied to mainline. >>> >> >> Many thanks for your explanation. (I get this commit id from linux-next tree: >> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git) >> So I should remember always to get the commit id from mainline. > > It's likely that this commit ID will be the same once Matthew's patch > goes into mainline. > > But this is why we include the patch title ("mm/truncate: Inline ...") > when identifying commits. Sometimes stuff happens... I remember I used the stale commit id once in my past patch. I made this mistake again. Sorry about it. :( > . >
diff --git a/mm/memory-failure.c b/mm/memory-failure.c index dabecd87ad3f..2ff7dd2078c4 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -2190,7 +2190,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.
Since commit 042c4f32323b ("mm/truncate: Inline invalidate_complete_page() into its one caller"), invalidate_inode_page() can invalidate the pages in the swap cache because the check of page->mapping != mapping is removed. But invalidate_inode_page() is not expected to deal with the pages in 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. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/memory-failure.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)