Messages in this thread | | | Date | Thu, 5 Oct 2023 22:04:28 +0800 | Subject | Re: [PATCH v4 2/2] mm: Init page count in reserve_bootmem_region when MEMINIT_EARLY | From | Yajun Deng <> |
| |
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.
| |