Re: [PATCH] hmat: Register attributes for memory hot add

[Keith Busch]

On Thu, Apr 11, 2019 at 04:42:45PM +0200, Rafael J. Wysocki wrote:
> On Tue, Apr 9, 2019 at 11:42 PM Keith Busch <keith.busch@xxxxxxxxx> wrote:
> > -static __init void hmat_register_targets(void)
> > +static void hmat_register_targets(void)
> >  {
> >         struct memory_target *target;
> >
> >         list_for_each_entry(target, &targets, node) {
> > +               if (!node_online(pxm_to_node(target->memory_pxm)))
> > +                       continue;
> > +
> >                 hmat_register_target_initiators(target);
> >                 hmat_register_target_perf(target);
> > +               target->registered = true;
> >         }
> >  }
> >
> > +static int hmat_callback(struct notifier_block *self,
> > +                        unsigned long action, void *arg)
> > +{
> > +       struct memory_notify *mnb = arg;
> > +       int pxm, nid = mnb->status_change_nid;
> > +       struct memory_target *target;
> > +
> > +       if (nid == NUMA_NO_NODE || action != MEM_ONLINE)
> > +               return NOTIFY_OK;
> > +
> > +       pxm = node_to_pxm(nid);
> > +       target = find_mem_target(pxm);
> > +       if (!target || target->registered)
> > +               return NOTIFY_OK;
> > +
> > +       hmat_register_target_initiators(target);
> > +       hmat_register_target_perf(target);
> > +       target->registered = true;
> > +
> > +       return NOTIFY_OK;
> > +}
> 
> This appears to assume that there will never be any races between the
> two functions above.
> 
> It this guaranteed to be the case?

The hmat_init() will call this directly before registering the memory
notifier callback, so those two paths should be 'ok'.

I may have assumed memory notification callbacks were single threaded,
but after taking a quick look, I think I do need additional locking for
this to be safe. I'll get that fixed up, thanks for the catch.