Re: [PATCH] mm: increase totalram_pages on freeing to buddy system

[Wei Yang <richard.weiyang@gmail.com>]

On Mon, Jun 03, 2024 at 10:55:10AM +0200, David Hildenbrand wrote:
>On 02.06.24 02:58, Wei Yang wrote:
>> On Sat, Jun 01, 2024 at 06:15:33PM +0200, David Hildenbrand wrote:
>> > On 01.06.24 17:32, David Hildenbrand wrote:
>> > > On 01.06.24 15:34, Wei Yang wrote:
>> > > > Total memory represents pages managed by buddy system.
>> > > 
>> > > No, that's managed pages.
>> > > 
>> > > > After the
>> > > > introduction of DEFERRED_STRUCT_PAGE_INIT, it may count the pages before
>> > > > being managed.
>> > > > 
>> > > 
>> > > I recall one reason that is done, so other subsystem know the total
>> > > memory size even before deferred init is done.
>> > > 
>> > > > free_low_memory_core_early() returns number of pages for all free pages,
>> > > > even at this moment only early initialized pages are freed to buddy
>> > > > system. This means the total memory at this moment is not correct.
>> > > > 
>> > > > Let's increase it when pages are freed to buddy system.
>> > > 
>> > > I'm missing the "why", and the very first sentence of this patch is wrong.
>> > 
>> > Correction: your statement was correct :) That's why
>> > adjust_managed_page_count() adjusts that as well.
>> > 
>> > __free_pages_core() only adjusts managed page count, because it assumes
>> > totalram has already been adjusted early during boot.
>> > 
>> > The reason we have this split for now, I think, is because of subsystems that
>> > call totalram_pages() during init.
>> > 
>> > So the "why" question remains, because this change has the potential to break
>> > other stuff.
>> > 
>> 
>> Thanks, I didn't notice this.
>
>I think having your cleanup would be very nice, as I have patches in the
>works that would benefit from being able to move the totalram update from
>memory hotplug code to __free_pages_core().
>

I got the same feeling.

>We'd have to make sure that no code relies on totalram being sane/fixed
>during boot for the initial memory. I think right now we might have such
>code.
>

One concern is totalram would change when hotplug is enabled. That sounds
those codes should do some re-calculation after totalram changes?

>Further, we currently require only a single atomic RMW instruction to adjust
>totalram during boot, moving it to __free_pages_core() would imply more
>atomics: but usually only one per MAX_ORDER page, so I doubt this would make
>a big difference.
>

I took a rough calculation on this.One MAX_ORDER page accounts for 2MB, and
with defer_init only low zone's memory is initialized during boot. Per my
understanding, low zone's memory is 4GB for x86. So the extra calculation is
4GB / 2MB = 2K.
>-- 
>Cheers,
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me