linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Li, Tianyou" <tianyou.li@intel.com>
To: Mike Rapoport <rppt@kernel.org>,
	Oscar Salvador <osalvador@suse.de>,
	"David Hildenbrand" <david@redhat.com>,
	Wei Yang <richard.weiyang@gmail.com>
Cc: <linux-mm@kvack.org>, Yong Hu <yong.hu@intel.com>,
	Nanhai Zou <nanhai.zou@intel.com>, Yuan Liu <yuan1.liu@intel.com>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	Qiuxu Zhuo <qiuxu.zhuo@intel.com>,
	Yu C Chen <yu.c.chen@intel.com>, Pan Deng <pan.deng@intel.com>,
	Chen Zhang <zhangchen.kidd@jd.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v6 2/2] mm/memory hotplug: fix zone->contiguous always false when hotplug
Date: Mon, 22 Dec 2025 22:06:48 +0800	[thread overview]
Message-ID: <0b166dd0-c368-4586-9c4b-16f5e104108a@intel.com> (raw)
In-Reply-To: <ee63f7c4-0adc-46ec-8e14-277609af6243@intel.com>

Thank you all for your great comments and suggestions. Patch v7 has been 
tested by Yuan Liu and sent for your kind review. Looking forward to 
your comments. Thanks.


Regards,

Tianyou


On 12/19/2025 10:33 PM, Li, Tianyou wrote:
>
> On 12/19/2025 5:37 PM, Mike Rapoport wrote:
>> Hi,
>>
>> On Mon, Dec 15, 2025 at 09:04:37PM +0800, Tianyou Li wrote:
>>> From: Yuan Liu <yuan1.liu@intel.com>
>>>
>>> Function set_zone_contiguous used __pageblock_pfn_to_page to
>>> check the whole pageblock is in the same zone. One assumption is
>>> the memory section must online, otherwise the __pageblock_pfn_to_page
>>> will return NULL, then the set_zone_contiguous will be false.
>>> When move_pfn_range_to_zone invoked set_zone_contiguous, since the
>>> memory section did not online, the return value will always be false.
>>>
>>> To fix this issue, we removed the set_zone_contiguous from the
>>> move_pfn_range_to_zone, and place it after memory section onlined.
>>>
>>> Function remove_pfn_range_from_zone did not have this issue because
>>> memory section remains online at the time set_zone_contiguous invoked.
>> Since the fix is relevant even without the optimization patch, can we
>> please reorder the patches so that the fix will be the first in the 
>> series?
>> Than it can be applied to stable trees as well.
>
>
> Thanks for the comments. That's totally make sense. I will work on the 
> patch v7, will rebase with 6.19-rc1. I will be out of office next 
> week, probably slow response but will keep working on the patch with 
> Yuan Liu. Thanks.
>
>
> Regards,
>
> Tianyou
>
>
>>> Reviewed-by: Tianyou Li <tianyou.li@intel.com>
>>> Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>
>>> Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
>>> ---
>>>   mm/memory_hotplug.c | 18 ++++++++++++++----
>>>   1 file changed, 14 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>>> index 12839032ad42..0220021f6a68 100644
>>> --- a/mm/memory_hotplug.c
>>> +++ b/mm/memory_hotplug.c
>>> @@ -810,8 +810,7 @@ void move_pfn_range_to_zone(struct zone *zone, 
>>> unsigned long start_pfn,
>>>   {
>>>       struct pglist_data *pgdat = zone->zone_pgdat;
>>>       int nid = pgdat->node_id;
>>> -    const enum zone_contig_state new_contiguous_state =
>>> -        zone_contig_state_after_growing(zone, start_pfn, nr_pages);
>>> +
>>>       clear_zone_contiguous(zone);
>>>         if (zone_is_empty(zone))
>>> @@ -841,8 +840,6 @@ void move_pfn_range_to_zone(struct zone *zone, 
>>> unsigned long start_pfn,
>>>       memmap_init_range(nr_pages, nid, zone_idx(zone), start_pfn, 0,
>>>                MEMINIT_HOTPLUG, altmap, migratetype,
>>>                isolate_pageblock);
>>> -
>>> -    set_zone_contiguous(zone, new_contiguous_state);
>>>   }
>>>     struct auto_movable_stats {
>>> @@ -1151,6 +1148,7 @@ int mhp_init_memmap_on_memory(unsigned long 
>>> pfn, unsigned long nr_pages,
>>>   {
>>>       unsigned long end_pfn = pfn + nr_pages;
>>>       int ret, i;
>>> +    enum zone_contig_state new_contiguous_state = ZONE_CONTIG_NO;
>>>         ret = kasan_add_zero_shadow(__va(PFN_PHYS(pfn)), 
>>> PFN_PHYS(nr_pages));
>>>       if (ret)
>>> @@ -1165,6 +1163,14 @@ int mhp_init_memmap_on_memory(unsigned long 
>>> pfn, unsigned long nr_pages,
>>>       if (mhp_off_inaccessible)
>>>           page_init_poison(pfn_to_page(pfn), sizeof(struct page) * 
>>> nr_pages);
>>>   +    /*
>>> +     * If the allocated memmap pages are not in a full section, 
>>> keep the
>>> +     * contiguous state as ZONE_CONTIG_NO.
>>> +     */
>>> +    if (IS_ALIGNED(end_pfn, PAGES_PER_SECTION))
>>> +        new_contiguous_state = zone_contig_state_after_growing(zone,
>>> +                                pfn, nr_pages);
>>> +
>>>       move_pfn_range_to_zone(zone, pfn, nr_pages, NULL, 
>>> MIGRATE_UNMOVABLE,
>>>                      false);
>>>   @@ -1183,6 +1189,7 @@ int mhp_init_memmap_on_memory(unsigned long 
>>> pfn, unsigned long nr_pages,
>>>       if (nr_pages >= PAGES_PER_SECTION)
>>>               online_mem_sections(pfn, ALIGN_DOWN(end_pfn, 
>>> PAGES_PER_SECTION));
>>>   +    set_zone_contiguous(zone, new_contiguous_state);
>>>       return ret;
>>>   }
>>>   @@ -1221,6 +1228,7 @@ int online_pages(unsigned long pfn, unsigned 
>>> long nr_pages,
>>>       };
>>>       const int nid = zone_to_nid(zone);
>>>       int need_zonelists_rebuild = 0;
>>> +    enum zone_contig_state new_contiguous_state = ZONE_CONTIG_NO;
>>>       unsigned long flags;
>>>       int ret;
>>>   @@ -1235,6 +1243,7 @@ int online_pages(unsigned long pfn, unsigned 
>>> long nr_pages,
>>>                !IS_ALIGNED(pfn + nr_pages, PAGES_PER_SECTION)))
>>>           return -EINVAL;
>>>   +    new_contiguous_state = zone_contig_state_after_growing(zone, 
>>> pfn, nr_pages);
>>>         /* associate pfn range with the zone */
>>>       move_pfn_range_to_zone(zone, pfn, nr_pages, NULL, 
>>> MIGRATE_MOVABLE,
>>> @@ -1273,6 +1282,7 @@ int online_pages(unsigned long pfn, unsigned 
>>> long nr_pages,
>>>       }
>>>         online_pages_range(pfn, nr_pages);
>>> +    set_zone_contiguous(zone, new_contiguous_state);
>>>       adjust_present_page_count(pfn_to_page(pfn), group, nr_pages);
>>>         if (node_arg.nid >= 0)
>>> -- 
>>> 2.47.1
>>>


      reply	other threads:[~2025-12-22 14:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-15 13:04 [PATCH v6 0/2] Optimize zone->contiguous update and issue fix Tianyou Li
2025-12-15 13:04 ` [PATCH v6 1/2] mm/memory hotplug/unplug: Optimize zone->contiguous update when changes pfn range Tianyou Li
2025-12-15 13:04 ` [PATCH v6 2/2] mm/memory hotplug: fix zone->contiguous always false when hotplug Tianyou Li
2025-12-19  9:37   ` Mike Rapoport
2025-12-19 14:33     ` Li, Tianyou
2025-12-22 14:06       ` Li, Tianyou [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0b166dd0-c368-4586-9c4b-16f5e104108a@intel.com \
    --to=tianyou.li@intel.com \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nanhai.zou@intel.com \
    --cc=osalvador@suse.de \
    --cc=pan.deng@intel.com \
    --cc=qiuxu.zhuo@intel.com \
    --cc=richard.weiyang@gmail.com \
    --cc=rppt@kernel.org \
    --cc=tim.c.chen@linux.intel.com \
    --cc=yong.hu@intel.com \
    --cc=yu.c.chen@intel.com \
    --cc=yuan1.liu@intel.com \
    --cc=zhangchen.kidd@jd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox