| Message ID | 20210830141051.64090-6-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Cleanups and fixup for page_alloc | expand |
On Mon, Aug 30, 2021 at 10:10:50PM +0800, Miaohe Lin wrote: > If it's not prepared to free unref page, the pcp page migratetype is > unset. Thus We will get rubbish from get_pcppage_migratetype() and > might list_del &page->lru again after it's already deleted from the > list leading to grumble about data corruption. > > Fixes: 3dcbe270d8ec ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> Acked-by: Mel Gorman <mgorman@techsingularity.net> This fix is fairly important. Take this patch out and send it on its own so it gets picked up relatively quickly. It does not belong in a series that is mostly cosmetic cleanups.
On 31.08.21 15:43, Mel Gorman wrote: > On Mon, Aug 30, 2021 at 10:10:50PM +0800, Miaohe Lin wrote: >> If it's not prepared to free unref page, the pcp page migratetype is >> unset. Thus We will get rubbish from get_pcppage_migratetype() and >> might list_del &page->lru again after it's already deleted from the >> list leading to grumble about data corruption. >> >> Fixes: 3dcbe270d8ec ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > > Acked-by: Mel Gorman <mgorman@techsingularity.net> > > This fix is fairly important. Take this patch out and send it on its own > so it gets picked up relatively quickly. It does not belong in a series > that is mostly cosmetic cleanups. I think the commit id is wrong. Shouldn't that be df1acc856923 ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") ?
On 8/31/21 15:43, Mel Gorman wrote: > On Mon, Aug 30, 2021 at 10:10:50PM +0800, Miaohe Lin wrote: >> If it's not prepared to free unref page, the pcp page migratetype is >> unset. Thus We will get rubbish from get_pcppage_migratetype() and >> might list_del &page->lru again after it's already deleted from the >> list leading to grumble about data corruption. >> >> Fixes: 3dcbe270d8ec ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > > Acked-by: Mel Gorman <mgorman@techsingularity.net> > > This fix is fairly important. Take this patch out and send it on its own > so it gets picked up relatively quickly. It does not belong in a series > that is mostly cosmetic cleanups. Also Cc: stable, please.
On 2021/8/31 22:23, David Hildenbrand wrote: > On 31.08.21 15:43, Mel Gorman wrote: >> On Mon, Aug 30, 2021 at 10:10:50PM +0800, Miaohe Lin wrote: >>> If it's not prepared to free unref page, the pcp page migratetype is >>> unset. Thus We will get rubbish from get_pcppage_migratetype() and >>> might list_del &page->lru again after it's already deleted from the >>> list leading to grumble about data corruption. >>> >>> Fixes: 3dcbe270d8ec ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") >>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> >> Acked-by: Mel Gorman <mgorman@techsingularity.net> >> >> This fix is fairly important. Take this patch out and send it on its own >> so it gets picked up relatively quickly. It does not belong in a series >> that is mostly cosmetic cleanups. > > I think the commit id is wrong. Shouldn't that be > > df1acc856923 ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") > > ? > Many thanks for pointing this out. I used to save the git log in a file to make life easier. But it seems this leads to the old commit id above. commit 3dcbe270d8ec57e534f5c605279cdc3ceb1f044a Author: Mel Gorman <mgorman@techsingularity.net> Date: Fri Jun 4 14:20:03 2021 +1000 mm/page_alloc: avoid conflating IRQs disabled with zone->lock git name-rev 3dcbe270d8ec 3dcbe270d8ec tags/next-20210604~2^2~196 vs commit df1acc856923c0a65c28b588585449106c316b71 Author: Mel Gorman <mgorman@techsingularity.net> Date: Mon Jun 28 19:42:00 2021 -0700 mm/page_alloc: avoid conflating IRQs disabled with zone->lock git name-rev df1acc856923 df1acc856923 tags/next-20210630~2^2~278 Their contents are same but with different commit id. The previous one could have been git-rebased.
On 2021/9/1 0:34, Vlastimil Babka wrote: > On 8/31/21 15:43, Mel Gorman wrote: >> On Mon, Aug 30, 2021 at 10:10:50PM +0800, Miaohe Lin wrote: >>> If it's not prepared to free unref page, the pcp page migratetype is >>> unset. Thus We will get rubbish from get_pcppage_migratetype() and >>> might list_del &page->lru again after it's already deleted from the >>> list leading to grumble about data corruption. >>> >>> Fixes: 3dcbe270d8ec ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") >>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> >> Acked-by: Mel Gorman <mgorman@techsingularity.net> >> >> This fix is fairly important. Take this patch out and send it on its own >> so it gets picked up relatively quickly. It does not belong in a series >> that is mostly cosmetic cleanups. > > Also Cc: stable, please. > > Will do. Many thanks for both of your suggestions! > > . >
On 01.09.21 10:02, Miaohe Lin wrote: > On 2021/8/31 22:23, David Hildenbrand wrote: >> On 31.08.21 15:43, Mel Gorman wrote: >>> On Mon, Aug 30, 2021 at 10:10:50PM +0800, Miaohe Lin wrote: >>>> If it's not prepared to free unref page, the pcp page migratetype is >>>> unset. Thus We will get rubbish from get_pcppage_migratetype() and >>>> might list_del &page->lru again after it's already deleted from the >>>> list leading to grumble about data corruption. >>>> >>>> Fixes: 3dcbe270d8ec ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") >>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >>> >>> Acked-by: Mel Gorman <mgorman@techsingularity.net> >>> >>> This fix is fairly important. Take this patch out and send it on its own >>> so it gets picked up relatively quickly. It does not belong in a series >>> that is mostly cosmetic cleanups. >> >> I think the commit id is wrong. Shouldn't that be >> >> df1acc856923 ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") >> >> ? >> > > Many thanks for pointing this out. > > I used to save the git log in a file to make life easier. But it seems this leads > to the old commit id above. > > commit 3dcbe270d8ec57e534f5c605279cdc3ceb1f044a > Author: Mel Gorman <mgorman@techsingularity.net> > Date: Fri Jun 4 14:20:03 2021 +1000 > > mm/page_alloc: avoid conflating IRQs disabled with zone->lock > > git name-rev 3dcbe270d8ec > 3dcbe270d8ec tags/next-20210604~2^2~196 > > vs > > commit df1acc856923c0a65c28b588585449106c316b71 > Author: Mel Gorman <mgorman@techsingularity.net> > Date: Mon Jun 28 19:42:00 2021 -0700 > > mm/page_alloc: avoid conflating IRQs disabled with zone->lock > > git name-rev df1acc856923 > df1acc856923 tags/next-20210630~2^2~278 > > Their contents are same but with different commit id. The previous one > could have been git-rebased. > -mm tree commit ids keep changing until patches are upstream. Therefore, you can observe changing commit ids in -next. Always use the ones from Linus' tree, they are stable.
On 2021/9/1 16:10, David Hildenbrand wrote: > On 01.09.21 10:02, Miaohe Lin wrote: >> On 2021/8/31 22:23, David Hildenbrand wrote: >>> On 31.08.21 15:43, Mel Gorman wrote: >>>> On Mon, Aug 30, 2021 at 10:10:50PM +0800, Miaohe Lin wrote: >>>>> If it's not prepared to free unref page, the pcp page migratetype is >>>>> unset. Thus We will get rubbish from get_pcppage_migratetype() and >>>>> might list_del &page->lru again after it's already deleted from the >>>>> list leading to grumble about data corruption. >>>>> >>>>> Fixes: 3dcbe270d8ec ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") >>>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >>>> >>>> Acked-by: Mel Gorman <mgorman@techsingularity.net> >>>> >>>> This fix is fairly important. Take this patch out and send it on its own >>>> so it gets picked up relatively quickly. It does not belong in a series >>>> that is mostly cosmetic cleanups. >>> >>> I think the commit id is wrong. Shouldn't that be >>> >>> df1acc856923 ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") >>> >>> ? >>> >> >> Many thanks for pointing this out. >> >> I used to save the git log in a file to make life easier. But it seems this leads >> to the old commit id above. >> >> commit 3dcbe270d8ec57e534f5c605279cdc3ceb1f044a >> Author: Mel Gorman <mgorman@techsingularity.net> >> Date: Fri Jun 4 14:20:03 2021 +1000 >> >> mm/page_alloc: avoid conflating IRQs disabled with zone->lock >> >> git name-rev 3dcbe270d8ec >> 3dcbe270d8ec tags/next-20210604~2^2~196 >> >> vs >> >> commit df1acc856923c0a65c28b588585449106c316b71 >> Author: Mel Gorman <mgorman@techsingularity.net> >> Date: Mon Jun 28 19:42:00 2021 -0700 >> >> mm/page_alloc: avoid conflating IRQs disabled with zone->lock >> >> git name-rev df1acc856923 >> df1acc856923 tags/next-20210630~2^2~278 >> >> Their contents are same but with different commit id. The previous one >> could have been git-rebased. >> > > -mm tree commit ids keep changing until patches are upstream. Therefore, you can observe changing commit ids in -next. Always use the ones from Linus' tree, they are stable. > Many thanks for your advice, David. :)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 7bb79e959ab4..d87b7e6e9e6b 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -3415,8 +3415,10 @@ void free_unref_page_list(struct list_head *list) /* Prepare pages for freeing */ list_for_each_entry_safe(page, next, list, lru) { pfn = page_to_pfn(page); - if (!free_unref_page_prepare(page, pfn, 0)) + if (!free_unref_page_prepare(page, pfn, 0)) { list_del(&page->lru); + continue; + } /* * Free isolated pages directly to the allocator, see
If it's not prepared to free unref page, the pcp page migratetype is unset. Thus We will get rubbish from get_pcppage_migratetype() and might list_del &page->lru again after it's already deleted from the list leading to grumble about data corruption. Fixes: 3dcbe270d8ec ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/page_alloc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)