From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH 2 of 3] switch to dynamically allocated cpumask in domain_update_node_affinity() Date: Tue, 24 Jan 2012 10:56:35 +0100 Message-ID: <4F1E8053.5090104@ts.fujitsu.com> References: <08232960ff4bed750d26.1327384451@nehalem1> <1327397591.24561.166.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1327397591.24561.166.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Campbell Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 01/24/2012 10:33 AM, Ian Campbell wrote: > On Tue, 2012-01-24 at 05:54 +0000, Juergen Gross wrote: >> # HG changeset patch >> # User Juergen Gross >> # Date 1327384410 -3600 >> # Node ID 08232960ff4bed750d26e5f1ff53972fee9e0130 >> # Parent 99f98e501f226825fbf16f6210b4b7834dff5df1 >> switch to dynamically allocated cpumask in >> domain_update_node_affinity() >> >> cpumasks should rather be allocated dynamically. >> >> Signed-off-by: juergen.gross@ts.fujitsu.com >> >> diff -r 99f98e501f22 -r 08232960ff4b xen/common/domain.c >> --- a/xen/common/domain.c Tue Jan 24 06:53:06 2012 +0100 >> +++ b/xen/common/domain.c Tue Jan 24 06:53:30 2012 +0100 >> @@ -333,23 +333,27 @@ struct domain *domain_create( >> >> void domain_update_node_affinity(struct domain *d) >> { >> - cpumask_t cpumask; >> + cpumask_var_t cpumask; >> nodemask_t nodemask = NODE_MASK_NONE; >> struct vcpu *v; >> unsigned int node; >> >> - cpumask_clear(&cpumask); >> + if ( !zalloc_cpumask_var(&cpumask) ) >> + return; > If this ends up always failing we will never set node_affinity to > anything at all. Granted that is already a pretty nasty situation to be > in but perhaps setting the affinity to NODE_MASK_ALL on failure is > slightly more robust? Hmm, I really don't know. node_affinity is only used in alloc_heap_pages(), which will fall back to other nodes if no memory is found on those nodes. OTOH this implementation might change in the future. The question is whether node_affinity should rather contain a subset or a superset of the nodes the domain is running on. What should be done if allocating a cpumask fails later? Should node_affinity be set to NODE_MASK_ALL/NONE or should it be left untouched assuming a real change is a rare thing to happen? Juergen >> + >> spin_lock(&d->node_affinity_lock); >> >> for_each_vcpu ( d, v ) >> - cpumask_or(&cpumask,&cpumask, v->cpu_affinity); >> + cpumask_or(cpumask, cpumask, v->cpu_affinity); >> >> for_each_online_node ( node ) >> - if ( cpumask_intersects(&node_to_cpumask(node),&cpumask) ) >> + if ( cpumask_intersects(&node_to_cpumask(node), cpumask) ) >> node_set(node, nodemask); >> >> d->node_affinity = nodemask; >> spin_unlock(&d->node_affinity_lock); >> + >> + free_cpumask_var(cpumask); >> } >> >> > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > > -- Juergen Gross Principal Developer Operating Systems PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html