From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH RESEND 01/12] xen: numa-sched: leave node-affinity alone if not in "auto" mode Date: Tue, 5 Nov 2013 14:43:13 +0000 Message-ID: <52790401.3050007@eu.citrix.com> References: <20131105142844.30446.78671.stgit@Solace> <20131105143411.30446.78865.stgit@Solace> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20131105143411.30446.78865.stgit@Solace> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli Cc: Marcus Granado , Keir Fraser , Ian Campbell , Li Yechen , Andrew Cooper , Juergen Gross , Ian Jackson , xen-devel@lists.xen.org, Jan Beulich , Justin Weaver , Daniel De Graaf , Matt Wilson , Elena Ufimtseva List-Id: xen-devel@lists.xenproject.org On 11/05/2013 02:34 PM, Dario Faggioli wrote: > If the domain's NUMA node-affinity is being specified by the > user/toolstack (instead of being automatically computed by Xen), > we really should stick to that. This means domain_update_node_affinity() > is wrong when it filters out some stuff from there even in "!auto" > mode. > > This commit fixes that. Of course, this does not mean node-affinity > is always honoured (e.g., a vcpu won't run on a pcpu of a different > cpupool) but the necessary logic for taking into account all the > possible situations lives in the scheduler code, where it belongs. > > What could happen without this change is that, under certain > circumstances, the node-affinity of a domain may change when the > user modifies the vcpu-affinity of the domain's vcpus. This, even > if probably not a real bug, is at least something the user does > not expect, so let's avoid it. > > Signed-off-by: Dario Faggioli > Reviewed-by: George Dunlap > --- > This has been submitted already as a single patch on its own. > Since this series needs the change done here, just include it > in here, instead of pinging the original submission and deferring > posting this series. Just reiterating what I said on the last send... this one is independent and can be checked in whenever you're ready. -George > --- > xen/common/domain.c | 28 +++++++++------------------- > 1 file changed, 9 insertions(+), 19 deletions(-) > > diff --git a/xen/common/domain.c b/xen/common/domain.c > index 5999779..af31ab4 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -352,7 +352,6 @@ void domain_update_node_affinity(struct domain *d) > cpumask_var_t cpumask; > cpumask_var_t online_affinity; > const cpumask_t *online; > - nodemask_t nodemask = NODE_MASK_NONE; > struct vcpu *v; > unsigned int node; > > @@ -374,28 +373,19 @@ void domain_update_node_affinity(struct domain *d) > cpumask_or(cpumask, cpumask, online_affinity); > } > > + /* > + * If d->auto_node_affinity is true, the domain's node-affinity mask > + * (d->node_affinity) is automaically computed from all the domain's > + * vcpus' vcpu-affinity masks (the union of which we have just built > + * above in cpumask). OTOH, if d->auto_node_affinity is false, we > + * must leave the node-affinity of the domain alone. > + */ > if ( d->auto_node_affinity ) > { > - /* Node-affinity is automaically computed from all vcpu-affinities */ > + nodes_clear(d->node_affinity); > for_each_online_node ( node ) > if ( cpumask_intersects(&node_to_cpumask(node), cpumask) ) > - node_set(node, nodemask); > - > - d->node_affinity = nodemask; > - } > - else > - { > - /* Node-affinity is provided by someone else, just filter out cpus > - * that are either offline or not in the affinity of any vcpus. */ > - nodemask = d->node_affinity; > - for_each_node_mask ( node, d->node_affinity ) > - if ( !cpumask_intersects(&node_to_cpumask(node), cpumask) ) > - node_clear(node, nodemask);//d->node_affinity); > - > - /* Avoid loosing track of node-affinity because of a bad > - * vcpu-affinity has been specified. */ > - if ( !nodes_empty(nodemask) ) > - d->node_affinity = nodemask; > + node_set(node, d->node_affinity); > } > > sched_set_node_affinity(d, &d->node_affinity); >