From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: Re: [Patch] update cpumask handling for cpu pools in libxc and python Date: Fri, 17 Sep 2010 12:04:11 +0200 Message-ID: <4C933D1B.3040308@ts.fujitsu.com> References: <4C9301DB.4050009@ts.fujitsu.com> <1284714037.16095.3083.camel@zakaz.uk.xensource.com> <4C9332EA.3030006@ts.fujitsu.com> <1284716674.16095.3180.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1284716674.16095.3180.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 09/17/10 11:44, Ian Campbell wrote: > On Fri, 2010-09-17 at 10:20 +0100, Juergen Gross wrote: >> On 09/17/10 11:00, Ian Campbell wrote: >>> local_size has already been rounded up in get_cpumap_size. Do we really >>> need to do it again? >> >> I've made it more clear that this is rounding to uint64. > > Wouldn't that be "(.. + 63) / 8" then? No, local_size is already bytes... >> >>> >>>> + size = sizeof(xc_cpupoolinfo_t) + cpumap_size * 8 + local_size; >>> >>> Why do we need both "cpumap_size * 8" and local_size additional bytes >>> here? Both contain the number of bytes necessary to contain a cpumap >>> bitmask and in fact I suspect they are both equal at this point (see >>> point about rounding above). >> >> The hypervisor returns a cpumask based on bytes, the tools use uint64-based >> cpumasks. > > Oh, I see, as well as xc_cpupool_info_t and the cpumap which it contains > being allocated together in a single buffer you are also including a > third buffer which is for local use in this function only but which is > included in the memory region returned to the caller (who doesn't know > about it). Seems a bit odd to me, why not just allocate it locally then > free it (or use alloca)? > > Actually, when I complete my hypercall buffer patch this memory will > need to be separately allocated any way since it needs to come from a > special pool. I'd prefer it if you just used explicit separate > allocation for this buffer from the start. Okay. > >> In practice this is equivalent as long as multiple of 8 bytes are >> written by the hypervisor and the system is running little endian. >> I prefer a clean interface mapping here. > > Using a single uint64 when there was a limit of 64 cpus made sense but > now that it is variable length why not just use bytes everywhere? It > would avoid a lot of confusion about what various size units are at > various points etc. You would avoid needing to translate between the > hypervisor and tools representations too, wouldn't you? This would suggest changing xc_vcpu_setaffinity() and -- Juergen Gross Principal Developer Operating Systems TSP 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