| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f5eb2eb8-4eb7-4697-afc7-bd4a32785415@redhat.com>
Date: Tue, 10 Jun 2025 09:55:27 +0200
From: David Hildenbrand <david@...hat.com>
To: Oscar Salvador <osalvador@...e.de>,
Andrew Morton <akpm@...ux-foundation.org>
Cc: Vlastimil Babka <vbabka@...e.cz>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Harry Yoo <harry.yoo@...cle.com>, Rakie Kim <rakie.kim@...com>,
Hyeonggon Yoo <42.hyeyoo@...il.com>, Joshua Hahn <joshua.hahnjy@...il.com>,
linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 10/10] mm,memory_hotplug: Drop status_change_nid
parameter from memory_notify
On 09.06.25 11:21, Oscar Salvador wrote:
> The 'status_change_nid' field was used to track changes in the memory
> state of a numa node, but that funcionality has been decoupled from
> memory_notify and moved to node_notify.
> Current consumers of memory_notify are only interested in which node the
> memory we are adding belongs to, but we can derive the nid from the pfn
> because we call move_pfn_range_to_zone(), which sets the node in the page
> via __init_single_page(), before calling in any memory notifier.
>
> Drop the 'status_change_nid' parameter from 'memory_notify' struct and update documentation
> accordingly.
>
> Suggested-by: David Hildenbrand <david@...hat.com>
> Reviewed-by: Harry Yoo <harry.yoo@...cle.com>
> Reviewed-by: Vlastimil Babka <vbabka@...e.cz>
> Signed-off-by: Oscar Salvador <osalvador@...e.de>
> ---
> Documentation/core-api/memory-hotplug.rst | 7 -------
> include/linux/memory.h | 1 -
> mm/memory_hotplug.c | 2 --
> mm/page_ext.c | 16 +++-------------
> 4 files changed, 3 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/core-api/memory-hotplug.rst b/Documentation/core-api/memory-hotplug.rst
> index b19c3be7437d..74897713c4f8 100644
> --- a/Documentation/core-api/memory-hotplug.rst
> +++ b/Documentation/core-api/memory-hotplug.rst
> @@ -59,17 +59,10 @@ The third argument (arg) passes a pointer of struct memory_notify::
> struct memory_notify {
> unsigned long start_pfn;
> unsigned long nr_pages;
> - int status_change_nid;
> }
>
> - start_pfn is start_pfn of online/offline memory.
> - nr_pages is # of pages of online/offline memory.
> -- status_change_nid is set node id when N_MEMORY of nodemask is (will be)
> - set/clear. It means a new(memoryless) node gets new memory by online and a
> - node loses all memory. If this is -1, then nodemask status is not changed.
> -
> - If status_changed_nid* >= 0, callback should create/discard structures for the
> - node if necessary.
>
> The callback routine shall return one of the values
> NOTIFY_DONE, NOTIFY_OK, NOTIFY_BAD, NOTIFY_STOP
> diff --git a/include/linux/memory.h b/include/linux/memory.h
> index a9ccd6579422..de8b898ada3f 100644
> --- a/include/linux/memory.h
> +++ b/include/linux/memory.h
> @@ -109,7 +109,6 @@ struct memory_notify {
> unsigned long altmap_nr_pages;
> unsigned long start_pfn;
> unsigned long nr_pages;
> - int status_change_nid;
> };
>
> struct notifier_block;
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 0550f3061fc4..07d7bdb65761 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1186,7 +1186,6 @@ int online_pages(unsigned long pfn, unsigned long nr_pages,
>
> mem_arg.start_pfn = pfn;
> mem_arg.nr_pages = nr_pages;
> - mem_arg.status_change_nid = node_arg.nid;
> cancel_mem_notifier_on_err = true;
> ret = memory_notify(MEM_GOING_ONLINE, &mem_arg);
> ret = notifier_to_errno(ret);
> @@ -1987,7 +1986,6 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
>
> mem_arg.start_pfn = start_pfn;
> mem_arg.nr_pages = nr_pages;
> - mem_arg.status_change_nid = node_arg.nid;
> cancel_mem_notifier_on_err = true;
> ret = memory_notify(MEM_GOING_OFFLINE, &mem_arg);
> ret = notifier_to_errno(ret);
> diff --git a/mm/page_ext.c b/mm/page_ext.c
> index c351fdfe9e9a..f08353802fa6 100644
> --- a/mm/page_ext.c
> +++ b/mm/page_ext.c
> @@ -369,25 +369,15 @@ static void __invalidate_page_ext(unsigned long pfn)
> }
>
> static int __meminit online_page_ext(unsigned long start_pfn,
> - unsigned long nr_pages,
> - int nid)
> + unsigned long nr_pages)
> {
> unsigned long start, end, pfn;
> int fail = 0;
> + int nid = pfn_to_nid(start_pfn);
Nit: Keep reverse xmas tree :)
>
> start = SECTION_ALIGN_DOWN(start_pfn);
> end = SECTION_ALIGN_UP(start_pfn + nr_pages);
>
> - if (nid == NUMA_NO_NODE) {
> - /*
> - * In this case, "nid" already exists and contains valid memory.
> - * "start_pfn" passed to us is a pfn which is an arg for
> - * online__pages(), and start_pfn should exist.
> - */
> - nid = pfn_to_nid(start_pfn);
> - VM_BUG_ON(!node_online(nid));
> - }
> -
> for (pfn = start; !fail && pfn < end; pfn += PAGES_PER_SECTION)
> fail = init_section_page_ext(pfn, nid);
> if (!fail)
> @@ -436,7 +426,7 @@ static int __meminit page_ext_callback(struct notifier_block *self,
> switch (action) {
> case MEM_GOING_ONLINE:
> ret = online_page_ext(mn->start_pfn,
> - mn->nr_pages, mn->status_change_nid);
> + mn->nr_pages);
> break;
> case MEM_OFFLINE:
> offline_page_ext(mn->start_pfn,
I would have moved the page_ext stuff into a separate patch, including
documenting why that is fine (e.g., memmap initialized before
GOING_ONLINE call).
Acked-by: David Hildenbrand <david@...hat.com>
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists