linux-kernel - Re: [PATCH v6 03/10] 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: <aE_WG6bnjtLBzCp8@localhost.localdomain>
Date: Mon, 16 Jun 2025 10:30:19 +0200
From: Oscar Salvador <osalvador@...e.de>
To: David Hildenbrand <david@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	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 03/10] mm,memory_hotplug: Implement numa node notifier

On Tue, Jun 10, 2025 at 10:10:21AM +0200, David Hildenbrand wrote:
> On 09.06.25 11:21, Oscar Salvador wrote:
> > +The first argument of the callback function (self) is a pointer to the block
> > +of the notifier chain that points to the callback function itself.
> > +The second argument (action) is one of the event types described above.
> > +The third argument (arg) passes a pointer of struct node_notify::
> > +
> > +        struct node_notify {
> > +                int nid;
> > +        }
> > +
> > +- nid is the node we are adding or removing memory to.
> > +
> > +  If nid >= 0, callback should create/discard structures for the
> > +  node if necessary.
> 
> Likely that should be removed?

Yes, indeed.

> 
> It' probably worth mentioning that one might get notified about
> NODE_CANCEL_ADDING_FIRST_MEMORY even though never notified for
> NODE_ADDING_FIRST_MEMORY. (same for removing)
> 
> I recall this can happen if one of the NODE_ADDING_FIRST_MEMORY notifiers
> fails.
> 
> (same applies to MEM_CANCEL_*)
> 
> Consequently, we might simplify the cancel_mem_notifier_on_err etc stuff,
> simply unconditionally calling the cancel counterparts.

So, I managed to do another respin with all feedback included, but I
left this one for the end, and here I'm.

It's true, currently users can get notified about e.g: MEM_CANCE_ONLINE without
going through MEM_GOING_ONLINE if another user fails for the latter, but I'm
trying to workaround the fact why that's not a problem.

Because assume you have a user of MEM_CANCEL_ONLINE, who thinks it got called
for MEM_GOING_ONLINE, while in fact it didn't because some other user fail on
it, and it tries to free some memory it thinks it initialized during MEM_GOING_ONLINE.

Isn't this a bit shaky? I mean, yes, I guess we can put the burden on the users of
the notifiers to not assume anything, but then yes, I think we should document this
as it can lead to potential misbeliefs.

-- 
Oscar Salvador
SUSE Labs