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

[David Hildenbrand <david@redhat.com>]

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().

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.

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.

-- 
Cheers,

David / dhildenb