Re: [Xen-devel] [PATCH v9 6/6] tools: enable Cache QoS Monitoring feature for libxl/libxc

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@xxxxxxxxxx]
> Sent: Wednesday, February 19, 2014 5:39 PM
> To: Xu, Dongxiao
> Cc: xen-devel@xxxxxxxxxxxxx; keir@xxxxxxx; JBeulich@xxxxxxxx;
> Ian.Jackson@xxxxxxxxxxxxx; stefano.stabellini@xxxxxxxxxxxxx;
> andrew.cooper3@xxxxxxxxxx; konrad.wilk@xxxxxxxxxx; dgdegra@xxxxxxxxxxxxx
> Subject: Re: [PATCH v9 6/6] tools: enable Cache QoS Monitoring feature for
> libxl/libxc
> 
> On Wed, 2014-02-19 at 14:32 +0800, Dongxiao Xu wrote:
> > +=item B<pqos-attach> [I<qos-type>] [I<domain-id>]
> > +
> > +Attach certain platform QoS service for a domain.
> > +Current supported I<qos-type> is: "cqm".
> 
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index 12d6c31..f3d2202 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -1105,6 +1105,10 @@ int libxl_flask_getenforce(libxl_ctx *ctx);
> >  int libxl_flask_setenforce(libxl_ctx *ctx, int mode);
> >  int libxl_flask_loadpolicy(libxl_ctx *ctx, void *policy, uint32_t size);
> >
> > +int libxl_pqos_attach(libxl_ctx *ctx, uint32_t domid, const char * 
> > qos_type);
> > +int libxl_pqos_detach(libxl_ctx *ctx, uint32_t domid, const char * 
> > qos_type);
> 
> I have a feeling that qos_type should actually be an enum in the IDL.
> The xl functions can probably use the autogenerate
> libxl_BLAH_from_string() functions to help with parsing.

Okay.


> 
> What other qos types are you envisaging? Is it valid to enable or
> disable multiple such things independently?

Yes, there are different types of QoS resources, and yes, they need to handle 
separately.

> 
> > +void libxl_map_cqm_buffer(libxl_ctx *ctx, libxl_cqminfo *xlinfo);
> 
> So each qos type is going to come with its own map function?

Yes.

> 
> I don't see the LIBXL_HAVE #define which we discussed last time anywhere
> here.

Oh, I mis-understood your previous comments.
Will add it in next patch version.

> 
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 649ce50..43c0f48 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -596,3 +596,10 @@ libxl_event = Struct("event",[
> >                                   ])),
> >             ("domain_create_console_available", Struct(None, [])),
> >             ]))])
> > +
> > +libxl_cqminfo = Struct("cqminfo", [
> > +    ("buffer_virt",    uint64),
> 
> An opaque void * masquerading as an integer is not a suitable interface.
> 
> This should be a (pointer to a) struct of the appropriate type, or an
> Array of such types etc (or possibly several such arrays depending on
> what you are returning).
> 
> I haven't looked in detail into what is actually in this buffer, but
> please try and have libxl bake it into a more consumable form -- e.g. an
> array of per-domain properties or something rather than a raw list.

The buffer contains the data of L3 cache usage for a certain domain on certain 
socket, so there is no structure inside.

For defining it as an array, we don't know its size until the 
libxl_map_cqm_buffer() function got returned. :(

> 
> > +    ("size",           uint32),
> > +    ("nr_rmids",       uint32),
> > +    ("nr_sockets",     uint32),
> > +    ])
> > [...][
> > +static void print_cqm_info(const libxl_cqminfo *info)
> > +{
> > +    unsigned int i, j, k;
> > +    char *domname;
> > +    int print_header;
> > +    int cqm_domains = 0;
> > +    uint16_t *rmid_to_dom;
> > +    uint64_t *l3c_data;
> > +    uint32_t first_domain = 0;
> > +    unsigned int num_domains = 1024;
> > +
> > +    if (info->nr_rmids == 0) {
> > +        printf("System doesn't support CQM.\n");
> > +        return;
> > +    }
> > +
> > +    print_header = 1;
> > +    l3c_data = (uint64_t *)(info->buffer_virt);
> > +    rmid_to_dom = (uint16_t *)(info->buffer_virt +
> > +                  info->nr_sockets * info->nr_rmids * sizeof(uint64_t));
> > +    for (i = first_domain; i < (first_domain + num_domains); i++) {
> > +        for (j = 0; j < info->nr_rmids; j++) {
> > +            if (rmid_to_dom[j] != i)
> > +                continue;
> > +
> > +            if (print_header) {
> > +                printf("Name
> ID");
> > +                for (k = 0; k < info->nr_sockets; k++)
> > +                    printf("\tSocketID\tL3C_Usage");
> > +                print_header = 0;
> > +            }
> > +
> > +            domname = libxl_domid_to_name(ctx, i);
> > +            printf("\n%-40s %5d", domname, i);
> > +            free(domname);
> > +            cqm_domains++;
> > +
> > +            for (k = 0; k < info->nr_sockets; k++)
> > +                printf("%10u %16lu     ",
> > +                        k, l3c_data[info->nr_rmids * k + j]);
> > +        }
> 
> This should be transformed into a sensible interface within libxl so
> that it can be consumed in a straightforward manner by the users of
> libxl, rather than asking them all to reimplement this.

Okay.

>       
> Is the buffer format considered a frozen ABI?

Yes, it is a fixed style for CQM.

> 
> > diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> > index ebe0220..d4af4a9 100644
> > --- a/tools/libxl/xl_cmdtable.c
> > +++ b/tools/libxl/xl_cmdtable.c
> > @@ -494,6 +494,21 @@ struct cmd_spec cmd_table[] = {
> >        "[options]",
> >        "-F                      Run in the foreground",
> >      },
> > +    { "pqos-attach",
> > +      &main_pqosattach, 0, 1,
> > +      "Allocate and map qos resource",
> > +      "<Resource> <Domain>",
> > +    },
> > +    { "pqos-detach",
> > +      &main_pqosdetach, 0, 1,
> > +      "Reliquish qos resource",
> 
> "Relinquish"
> 
> and perhaps "resources" (in both cases)

Okay.

Thanks,
Dongxiao

> 
> > +      "<Resource> <Domain>",
> > +    },
> > +    { "pqos-list",
> > +      &main_pqoslist, 0, 0,
> > +      "List qos information for all domains",
> > +      "<Resource>",
> > +    },
> >  };
> >
> >  int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
> 

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