From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH RESEND 01/12] xen: numa-sched: leave node-affinity alone if not in "auto" mode Date: Tue, 12 Nov 2013 08:35:17 +0000 Message-ID: References: <5281F0B50200007800102364@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Vg9RT-0007rZ-FP for xen-devel@lists.xenproject.org; Tue, 12 Nov 2013 08:35:23 +0000 Received: by mail-we0-f179.google.com with SMTP id x55so1639726wes.38 for ; Tue, 12 Nov 2013 00:35:21 -0800 (PST) In-Reply-To: <5281F0B50200007800102364@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel List-Id: xen-devel@lists.xenproject.org On 12/11/2013 08:11, "Jan Beulich" wrote: > Hi Keir, > > below the one remaining patch mentioned yesterday. > > Jan > >>>> On 05.11.13 at 15:34, 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 Acked-by: Keir Fraser >> --- >> 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. >> --- >> 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); > > >