linux-kernel - Re: [PATCH 1/2] mm,memory_hotplug: Implement numa node notifier

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z-1shl4h4fpfKyxb@localhost.localdomain>
Date: Wed, 2 Apr 2025 18:57:42 +0200
From: Oscar Salvador <osalvador@...e.de>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	David Hildenbrand <david@...hat.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, Hyeonggon Yoo <42.hyeyoo@...il.com>,
	mkoutny@...e.com, Dan Williams <dan.j.williams@...el.com>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH 1/2] mm,memory_hotplug: Implement numa node notifier

On Wed, Apr 02, 2025 at 06:03:04PM +0200, Vlastimil Babka wrote:
> On 4/1/25 11:27, Oscar Salvador wrote:
> > -	arg.start_pfn = pfn;
> > -	arg.nr_pages = nr_pages;
> > -	node_states_check_changes_online(nr_pages, zone, &arg);
> > +	mem_arg.start_pfn = pfn;
> > +	mem_arg.nr_pages = nr_pages;
> > +	node_states_check_changes_online(nr_pages, zone, &node_arg);
> >  
> > -	ret = memory_notify(MEM_GOING_ONLINE, &arg);
> > +	if (node_arg.status_change_nid >= 0) {
> 
> Hmm, don't we need to add "|| node_arg.status_change_nid_normal >= 0"? Or we
> fail to notify addition of normal memory to a node that already has !normal
> memory?

Ah, I think you are right, nice catch.
Yes, if we add normal (<= ZONE_NORMAL) memory to a node that only has ZONE_MOVABLE,
status_change_nid_normal will change, but status_change_nid will not.

So yes, we need a "|| node_arg.status_change_nid_normal >= 0".

> > -	memory_notify(MEM_CANCEL_ONLINE, &arg);
> > +	if (cancel_node_notifier_on_err)
> > +		node_notify(NODE_CANCEL_MEM_AWARE, &node_arg);
> > +	if (cancel_mem_notifier_on_err)
> > +		memory_notify(MEM_CANCEL_ONLINE, &mem_arg);
> 
> Switch the order of those just for symmetry? :)

Will do.

> > -	arg.start_pfn = start_pfn;
> > -	arg.nr_pages = nr_pages;
> > -	node_states_check_changes_offline(nr_pages, zone, &arg);
> > +	mem_arg.start_pfn = start_pfn;
> > +	mem_arg.nr_pages = nr_pages;
> > +	node_states_check_changes_offline(nr_pages, zone, &node_arg);
> > +
> > +	if (node_arg.status_change_nid >= 0) {
> 
> Ditto.

Yap.

> >  failed_removal_isolated:
> >  	/* pushback to free area */
> >  	undo_isolate_page_range(start_pfn, end_pfn, MIGRATE_MOVABLE);
> > -	memory_notify(MEM_CANCEL_OFFLINE, &arg);
> > +	if (cancel_node_notifier_on_err)
> > +		node_notify(NODE_CANCEL_MEMORYLESS, &node_arg);
> > +	if (cancel_mem_notifier_on_err)
> > +		memory_notify(MEM_CANCEL_OFFLINE, &mem_arg);
> 
> Ditto.

Copy that.

Thanks Vlastimil!

-- 
Oscar Salvador
SUSE Labs