Re: Re: [Patch] update cpumask handling for cpu pools in libxc and python

[Juergen Gross <juergen.gross@ts.fujitsu.com>]

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