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.
