Re: [PATCH v6 03/10] mm,memory_hotplug: Implement numa node notifier

[David Hildenbrand <david@redhat.com>]

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