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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c4ec57e5-49e1-413f-a1e2-e258c628c55f@vivo.com>
Date: Tue, 3 Sep 2024 15:14:11 +0800
From: Rong Qianfeng <11065417@...o.com>
To: David Hildenbrand <david@...hat.com>
Cc: opensource.kernel@...o.com, Rong Qianfeng <rongqianfeng@...o.com>,
 vbabka@...e.cz, mgorman@...hsingularity.net,
 Andrew Morton <akpm@...ux-foundation.org>, Mike Rapoport <rppt@...nel.org>,
 "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
 Zi Yan <ziy@...dia.com>, Baolin Wang <baolin.wang@...ux.alibaba.com>,
 linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: Skip the reserved bootmem for compaction

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