Re: [PATCH v1 2/5] libxc/trace: Add xc_tbuf_set_cpu_mask_array a variant of xc_tbuf_set_cpu_mask (v3)

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Jun 05, 2014 at 01:49:35PM +0100, Ian Campbell wrote:
> On Wed, 2014-06-04 at 09:44 -0400, Konrad Rzeszutek Wilk wrote:
> > which uses an xc_cpumap_t instead of a uint32_t. This means
> > we can use an arbitrary bitmap without being limited to the
> > 32-bits the xc_tbuf_set_cpu_mask_array can only do.
> 
> We do not guarantee API stability for libxc, so it is OK to either fix
> the existing one or replace it. No need to keep the old one around
> (unless perhaps calling applications fall into two sets each of whom
> finds a different interface best).
> 
> > We also add an macro which can be used by both libxc and
> > xentrace.
> > 
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > [v2: Use DIV_ROUND_UP macro as suggested by Daniel]
> > [v3: Use 'int' for bits instead of 'unsigned int' as spotted by Boris]
> > ---
> >  tools/libxc/xc_bitops.h |  2 ++
> >  tools/libxc/xc_tbuf.c   | 40 ++++++++++++++++++++++++++++++++++++++++
> >  tools/libxc/xenctrl.h   |  1 +
> >  3 files changed, 43 insertions(+)
> > 
> > diff --git a/tools/libxc/xc_bitops.h b/tools/libxc/xc_bitops.h
> > index d8e0c16..b8cf2bd 100644
> > --- a/tools/libxc/xc_bitops.h
> > +++ b/tools/libxc/xc_bitops.h
> > @@ -12,6 +12,8 @@
> >  #define BITMAP_ENTRY(_nr,_bmap) ((_bmap))[(_nr)/BITS_PER_LONG]
> >  #define BITMAP_SHIFT(_nr) ((_nr) % BITS_PER_LONG)
> >  
> > +#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> 
> This isn't really a bitops.h thing, xc_private.h seems like the usual
> dumping ground for stuff which doesn't fit elsewhere.
> 
> > +int xc_tbuf_set_cpu_mask_array(xc_interface *xch, xc_cpumap_t mask, int bits)
> > +{
> > +    DECLARE_SYSCTL;
> > +    DECLARE_HYPERCALL_BUFFER(uint8_t, bytemap);
> > +    int ret = -1;
> > +    int local_bits;
> > +
> > +    if ( bits <= 0 )
> > +        goto out;
> > +
> > +    local_bits = xc_get_max_cpus(xch);
> > +    if ( bits > local_bits )
> > +    {
> > +        PERROR("Wrong amount of bits supplied: %u, using %u\n", bits, local_bits);
> 
> Should we not just return an error?
> 
> > +        bits = local_bits;
> > +    }
> > +    bytemap = xc_hypercall_buffer_alloc(xch, bytemap, DIV_ROUND_UP(bits, 8));
> > +    if ( bytemap == NULL )
> > +    {
> > +        PERROR("Could not allocate memory for xc_tbuf_set_cpu_mask_array hypercall");
> > +        goto out;
> > +    }
> > +
> > +    memcpy(bytemap, mask, DIV_ROUND_UP(bits, 8));
> 
> Take a look at Dario's "libxc: get and set soft and hard affinity"[0]
> for how to do this using the hypercall bounce buffer interface.
> 
> [0] <1401237770-7003-6-git-send-email-dario.faggioli@citrix.com>

Since George asked me to merge two patches (libxc + xentrace) and also
throw out the old xc_tbuf_set_cpu_mask, would you be OK if this was
a seperate commit? Too many things going in the patch already.

> 
> Ian.
> 
> > +
> > +    sysctl.cmd = XEN_SYSCTL_tbuf_op;
> > +    sysctl.interface_version = XEN_SYSCTL_INTERFACE_VERSION;
> > +    sysctl.u.tbuf_op.cmd  = XEN_SYSCTL_TBUFOP_set_cpu_mask;
> > +
> > +    set_xen_guest_handle(sysctl.u.tbuf_op.cpu_mask.bitmap, bytemap);
> > +    sysctl.u.tbuf_op.cpu_mask.nr_bits = bits;
> > +
> > +    ret = do_sysctl(xch, &sysctl);
> > +
> > +    xc_hypercall_buffer_free(xch, bytemap);
> > +
> > + out:
> > +    return ret;
> > +}
> >  
> >  int xc_tbuf_set_evt_mask(xc_interface *xch, uint32_t mask)
> >  {
> 
>