Re: [PATCH v4 2/2] mm: Init page count in reserve_bootmem_region when MEMINIT_EARLY

From: Yajun Deng
Date: Thu Oct 05 2023 - 11:58:34 EST



On 2023/10/5 13:06, Mike Rapoport wrote:
On Tue, Oct 03, 2023 at 10:38:09PM +0800, Yajun Deng wrote:
On 2023/10/2 19:25, David Hildenbrand wrote:
On 02.10.23 13:10, Mike Rapoport wrote:
That 'if' breaks the invariant that __free_pages_core is
always called for
pages with initialized page count. Adding it may lead to
subtle bugs and
random memory corruption so we don't want to add it at the
first place.
As long as we have to special-case memory hotplug, we know that we are
always coming via generic_online_page() in that case. We could
either move
some logic over there, or let __free_pages_core() know what it
should do.
Looks like the patch rather special cases MEMINIT_EARLY, although I
didn't
check throughfully other code paths.
Anyway, relying on page_count() to be correct in different ways for
different callers of __free_pages_core() does not sound right to me.
Absolutely agreed.

I already sent v5  a few days ago. Comments, please...
Does it address all the feedback from this thread?


Except hotplug. As far as I konw, we only clear page count in MEMINIT_EARLY and all tail pages in compound page.

So adding 'if (page_count(page))' will have no actual effect for other case. According to previous data, it didn't

become slower in hotplug.