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

[David Hildenbrand]

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