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

[Rong Qianfeng <11065417@vivo.com>]

Hi Mike,

在 2024/9/4 23:38, Mike Rapoport 写道:
> On Wed, Sep 04, 2024 at 07:59:37PM +0800, Rong Qianfeng wrote:
>> Hi Mel,
>>
>> 在 2024/9/4 19:13, Mel Gorman 写道:
>>> On Mon, Sep 02, 2024 at 08:24:43PM +0800, 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.
>>>>
>>>> Because some drivers may also use PG_reserved, we just set PB_migrate_skip
>>>> for those clustered reserved bootmem during memory initialization.
>>>>
>>>> Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com>
>>> I'm not convinced the savings due to skipping a few pages during the scan
>>> would justify the additional code. There would have to be a large number
>>> of reserved pages scattered throughout the zone to make a difference and
>>> even that situation would be a big surprise. I'm not even sure this can be
>>> explicitly tested unless you artifically create reserved pages throughout the
>>> zone, which would not be convincing, or know if a driver that exhibits such
>>> behaviour in which case my first question is -- what is that driver doing?!?
>> Thanks for taking the time to reply.
>>
>> At first I thought that there was not much PageReserved pages, but when I
>> looked at the memory initialization code, I found that no-map pages were
>> also marked as PageReserved.  On mobile platforms, there is a lot of no-map
>> pages (for example, ARM64 MT6991 no-map pages has 1065MB).  These
>> pages are usually used by various hardware subsystems such as modem.  So
>> I think it makes sense to skip these pages.
>>
>>
>> //no-map and  reserved memory marked as PageReserved
>> static void __init memmap_init_reserved_pages(void)
>> {
>> ...
>>      for_each_mem_region(region) {
>> ...
>>          if (memblock_is_nomap(region))
>>              reserve_bootmem_region(start, end, nid);  //for no-map memory
> If nomap regions are a problem won't that be simpler to make all pageblocks
> of a nomap region PB_migrate_skip here and leave other reserved pages
> alone?

Sorry, maybe my explanation confused you. I didn't mean to say that the 
root of
the problem comes from the no-map region. I just gave a special example. 
There
may be a lot of reserved pages on some machines, because in DTS, you can use
the "no-map" attribute to specify a piece of memory as a no-map region, and
you can also use "reusable" and "shared-dma-pool" to specify a piece of 
memory
as a reserved region.

Sorry again, "ARM64 MT6991 no-map pages has 1065MB" I counted it wrongly.
1065MB includes the memory occupied by struct page, kernel code, kernel 
data,
etc. (these are actually reserved memory). Let's use ARM64 MT6991 16GB RAM
device as an example. The actual no-map memory is about 700MB, and the
reserved memory is about 1GB.

>
>>          memblock_set_node(start, end, &memblock.reserved, nid);
>>      }
>>
>>      for_each_reserved_mem_region(region) {
>>          if (!memblock_is_reserved_noinit(region)) {
>> ...
>>              reserve_bootmem_region(start, end, nid); //for reserved memory
>>          }
>>      }
>>
>> }
>>
>> Best Regards,
>> Qianfeng