> -----Original Message-----
> From: David Hildenbrand <david@xxxxxxxxxx>
> Sent: Thursday, July 4, 2024 4:15 PM
>
> On 04.07.24 09:43, Zhijian Li (Fujitsu) wrote:
> > All,
> >
> > Some progress updates
> >
> > When issue occurs, calling __drain_all_pages() can make offline_pages() escape
> from the loop.
> >
> >>
> >> Jun 28 15:29:26 linux kernel: page: refcount:0 mapcount:0
> >> mapping:0000000000000000 index:0x0 pfn:0x7980dd Jun 28 15:29:26 linux
> >> kernel: flags: 0x9fffffc0000000(node=2|zone=3|lastcpupid=0x1fffff)
> >> Jun 28 15:29:26 linux kernel: raw: 009fffffc0000000 ffffdfbd9e603788
> >> ffffd4f0ffd97ef0 0000000000000000 Jun 28 15:29:26 linux kernel: raw:
> >> 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
> Jun 28 15:29:26 linux kernel: page dumped because: trouble page...
> >>
> >
> > With this problematic page structure contents, it seems that the
> > list_head = {ffffdfbd9e603788, ffffd4f0ffd97ef0} is valid.
> >
> > I guess it was linking to the pcp_list, so I dumped the
> > per_cpu_pages[cpu].count in every in critical timings.
>
> So, is your reproducer getting fixed when you call __drain_all_pages() in the loop?
> (not that it's the right fix, but a could datapoint :) )
Yeah, it works for my reproducer.
>
> >
> > An example is as below,
> > offline_pages()
> > {
> > // per_cpu_pages[1].count = 0
> > zone_pcp_disable() // will call __drain_all_pages()
> > // per_cpu_pages[1].count = 188
> > do {
> > do {
> > scan_movable_pages()
> > ret = do_migrate_range()
> > } while (!ret)
> >
> > ret = test_pages_isolated()
> >
> > if(is the 1st iteration)
> > // per_cpu_pages[1].count = 182
> >
> > if (issue occurs) { /* if the loop take beyond 10 seconds */
> > // per_cpu_pages[1].count = 61
> > __drain_all_pages()
> > // per_cpu_pages[1].count = 0
> > /* will escape from the outer loop in later iterations */
> > }
> > } while (ret)
> > }
> >
> > Some interesting points:
> > - After the 1st __drain_all_pages(), per_cpu_pages[1].count increased to 188
> from 0,
> > does it mean it's racing with something...?
> > - per_cpu_pages[1].count will decrease but not decrease to 0 during
> iterations
> > - when issue occurs, calling __drain_all_pages() will decrease
> per_cpu_pages[1].count to 0.
>
> That's indeed weird. Maybe there is a race, or zone_pcp_disable() is not fully
> effective for a zone?
I often see there still are pages in PCP after the zone_pcp_disable().
>
> >
> > So I wonder if it's fine to call __drain_all_pages() in the loop?
> >
> > Looking forward to your insights.
>
> So, in free_unref_page(), we make sure to never place MIGRATE_ISOLATE onto
> the PCP. All pageblocks we are going to offline should be isolated at this point, so
> no page that is getting freed and part of the to-be-offlined range should end up on
> the PCP. So far the theory.
>
>
> In offlining code we do
>
> 1) Set MIGRATE_ISOLATE
> 2) zone_pcp_disable() -> set high-and-batch to 0 and drain
>
> Could there be a race in free_unref_page(), such that although
> zone_pcp_disable() succeeds, we would still end up with a page in the pcp?
> (especially, one that has MIGRATE_ISOLATE set for its pageblock?)
Thanks for your idea, I will further investigate in this direction.
Thanks
Zhijian
>
>
> --
> Cheers,
>
> David / dhildenb
