Re: [Xen-devel] [PATCH v2 08/16] xen: derive NUMA node affinity from hard and soft CPU affinity

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On gio, 2013-11-14 at 15:21 +0000, George Dunlap wrote:
> > Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > @@ -373,8 +379,12 @@ void domain_update_node_affinity(struct domain *d)
> >   
> >       for_each_vcpu ( d, v )
> >       {
> > +        /* Build up the mask of online pcpus we have hard affinity with */
> >           cpumask_and(online_affinity, v->cpu_hard_affinity, online);
> >           cpumask_or(cpumask, cpumask, online_affinity);
> > +
> > +        /* As well as the mask of all pcpus we have soft affinity with */
> > +        cpumask_or(cpumask_soft, cpumask_soft, v->cpu_soft_affinity);
> 
> Is this really the most efficient way to do this?  I would have thought 
> or'ing cpumask and cpumask_soft, and then if it's not empty, then use 
> it; maybe use a pointer so you don't have to copy one into the other one?
> 
I'm not sure I fully get what you mean... I cannot afford neglecting
online_affinity, independently from how cpumask and cpumask_soft look
like, because that's what's needed to account for cpupools. Anyway, I'll
think more about it and see if I can make it better.

> Also, all of the above computation happens unconditionally, but is only 
> used if d->auto_node_affinity is true.  It seems like it would be better 
> to move this calculation inside the conditional.
> 
That is true, and I thought it too. I didn't do this here because it
seems an unrelated  change (the calculation of
cpumask|=hard_affinity&online was outside of the if already).

Anyway, again, I agree with this, so I'll either do it here or in a
pre-patch.

> Leaving the allocation outside might make sense to reduce the code 
> covered by the lock -- not sure about that; I don't think this is a 
> heavily-contended lock, is it?
> 
Not really, nor this is an hot-path, but I still think it's worth doing
what you suggest.

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel