From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Jones Subject: Re: [PATCH] [pv-ops domU] support MAXSMP Date: Fri, 18 Dec 2009 14:23:00 +0100 Message-ID: <4B2B8234.50407@redhat.com> References: <4B2B4BF3.1010507@redhat.com> <4B2B625D02000078000269B3@vpn.id2.novell.com> <4B2B585D.5000305@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4B2B585D.5000305@redhat.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: Jan Beulich Cc: jeremy@goop.org, xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On 12/18/2009 11:24 AM, Andrew Jones wrote: > On 12/18/2009 11:07 AM, Jan Beulich wrote: >>>>> Andrew Jones 18.12.09 10:31>>> >>> The MAXSMP config option requires CPUMASK_OFFSTACK, which in turn >>> requires we init the memory for the maps while we bringing up the cpus. >>> MAXSMP also increases NR_CPUS to 4096. This increase in size exposed an >>> issue in the argument construction for mulitcalls from >>> xen_flush_tlb_others. The args should only need space for the actual >>> number of cpus, which with xen is currently only up to 32. >> >> I don't think new code should be making assumptions like this anymore, >> since Xen already supports higher numbers (it's merely the tools which >> so far don't). You're basically trading a compile time detectable large >> value on stack for one that can grow large dynamically (and hence >> require quite a bit more effort to debug, should it ever overrun the >> stack). > > I say 32 cpus in my description to point out the large difference > between NR_CPUS and the actual number used. However, the code shouldn't > exceed the limits in multicall until something around 2000 cpus. > > Keeping it compile-time is good for the stack analysis, but overly > wasteful when using values like 4096, since the expected case is > thousands less. If we want to keep it static then we need to change > MC_ARGS to also be dependent in some way on NR_CPUS. > Another note here is that the amount of stack allocation is the same regardless of the value of num_processors. We're just creating a pointer to this structure on the stack. sizeof is smart enough to pass the appropriate dynamic size on to the mc call for validation though. So I think all should be good with this patch. Andrew