Re: [PATCH 01 of 11 v3] xen, libxc: rename xenctl_cpumap to xenctl_bitmap

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3599 bytes --]

On mar, 2013-03-12 at 17:26 +0000, Ian Campbell wrote:
> On Tue, 2013-03-12 at 17:06 +0000, Dario Faggioli wrote:
> > Fact is xenctl_bitmap is used within DOMCTLs, while xc_{cpu,node}map_t
> > is used by xc_*_{set,get}_affinity(), which is what, for instance, libxl
> > calls.
> 
> so xenctl_bitmap is completely internal to the library/hypervisor and
> the user of libxc doesn't see it?
> 
It's always tricky to track (at least for me), due to some "Chinese
Boxing" going on here, and that's why I had to recheck before replying.
Having done that, yes, AFAIUI, xenctl_bitmap is how Xen and libxc
exchanges the information about the cpu/node-map. What is actually used
is then cpumask_t (nodemask_t), in Xen, and xc_cpumap_t (xc_nodemap_t)
in libxc (and his callers).

> > > >  #ifndef __ASSEMBLY__
> > > > -struct xenctl_cpumap {
> > > > +struct xenctl_bitmap {
> > > >      XEN_GUEST_HANDLE_64(uint8) bitmap;
> > > > -    uint32_t nr_cpus;
> > > > +    uint32_t nr_elems;
> > > 
> > > Is this really "nr_bits" or can an entry in the bitmap be > 1 bit?
> > > 
> > I'm not sure I understand what you meant to say here, I'm afraid. The
> > field encodes the number of elements the bitmap has to accommodate and
> > deal with. In the (original) xenctl_cpumap case, that was the number of
> > CPUs... All I'm doing is generalizing that from CPUs to some unspecified
> > "element". It never was the size of the bitmap in bits, and is not
> > becoming anything like that after the change.
> 
> Ah, so it is the number of bytes allocated in the bitmap field?
> 
> nr_elems is confusing because the elements of this particular array are
> (logically at least) bits, yet nr_elems contains bytes. Why not call it
> nr_bytes if that is what it contains?
> 
Mmm... I think now I see what you mean (or so I hope)! Sorry if I did
not git your first comment right. So, you are (and you were, in your
previous email) right when saying that what I call nr_elems is the
number indeed, at least logically, nr_bits.

In fact, it is not at all the real size of the array in bytes, as can be
seen from here:

int xc_vcpu_getaffinity(xc_interface *xch, uint32_t domid, int vcpu, xc_cpumap_t cpumap)
{
    DECLARE_HYPERCALL_BUFFER(uint8_t, local);
    int cpusize;

    cpusize = xc_get_cpumap_size(xch);
    ...
    local = xc_hypercall_buffer_alloc(xch, local, cpusize);
    ...
    domctl.cmd = XEN_DOMCTL_getvcpuaffinity;
    domctl.domain = (domid_t)domid;
    domctl.u.vcpuaffinity.vcpu = vcpu;

    set_xen_guest_handle(domctl.u.vcpuaffinity.cpumap.bitmap, local);
    domctl.u.vcpuaffinity.cpumap.nr_elems = cpusize * 8;
    ...
}

The whole point is I'm turning a collection of CPUs into something more
general. That is why I renamed the field that encodes, currently, the
"number of CPUs", into a more general "number of elements".

Now, were you saying that, since this collection of elements is,
actually, a collection of bits && that, given the fact we represent it
as an array, the elements of which are bytes, using "number of bits"
would be more appropriate and clear?

If yes, sorry again for not getting it in the first place, and, yes, I
do agree with that, and I can sed/nr_elems/nr_bits/.
If no... Well... Let's hope it's yes. :-D

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel