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: <10c87a0e-c9fe-48fe-9bbd-16afd244b4ec@redhat.com>
Date: Mon, 16 Jun 2025 10:39:24 +0200
From: David Hildenbrand <david@...hat.com>
To: Oscar Salvador <osalvador@...e.de>
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 16.06.25 10:30, Oscar Salvador wrote:
> 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?

It's suboptimal yes, But to get it right, you'd have to remmeber for 
exactly which notofiers you performed the calls ...

> 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.

The burden is already on the users I think.

E.g., virio-mem maintains a "hotplug_active" variable, to detect whether 
MEM_ONLINE was actually called.

-- 
Cheers,

David / dhildenb