Re: [PATCH] mm: Skip the reserved bootmem for compaction

[Rong Qianfeng]

Hi David,

Thanks very much for the detailed comments and explanations!

在 2024/9/2 21:45, David Hildenbrand 写道:
> On 02.09.24 14:24, Rong Qianfeng wrote:
> > Reserved pages are basically non-lru pages. This kind of memory can't be
> > used as migration sources and targets, skip it can bring some 
> > performance
> > benefits.
>
> Any numbers? :)

I am still thinking about how to design test cases. If you have any good
suggestions,  please tell me. Thank you very much.

> > +
> > +extern void set_pageblock_skip_range(unsigned long start_pfn,
> > +                                  unsigned long end_pfn);
>
> two tabs indentation on the second line please. Applies to all others as
> well.

Got it. Will do in the next version.

> > +     /*
> > +      * We set the PB_migrate_skip in
> > +      * reserve_bootmem_region() for cma
> > +      * memory, clear it now.
>
> You can fit this easily into less lines

Will do in the next version. Thanks.

> > +     /*
> > +      * Clear PB_migrate_skip if the memory have released
> > +      * to the buddy system.
> > +      */
>
> ... after freeing the memory to the buddy."
>
> And maybe
>
> if (pages) {
>        if (s)
>                pr_info("Freeing %s memory: %ldK\n", s, K(pages));
>        clear_pageblock_skip_range(...)
> }
>
> > + clear_pageblock_skip_range(page_to_pfn(virt_to_page(start)),
> > + page_to_pfn(virt_to_page(end)));
> > +
>
> PHYS_PFN(virt_to_phys(start)) might look a bit nicer, not need to
> get pages involved. virt_to_pfn might be even better(), but it's
> not available on all archs I think.

You are right, I tried to use virt_to_pfn, but later found out that it 
is not
supported on x86.

> What about free_reserved_page() ? There might be more, though
> (kimage_free_pages()). You have to take a look at all functions where we
> clear PageReserved.

Thanks for your reminder, I found that I missed a lot of functions.
Maybe a better choice is to clear PB_migrate_skip in free_reserved_page()
to reduce the amount of modification.

Best Regards,
Qianfeng