Re: [PATCH v9 3/3] tools, docs: add total/local memory bandwith monitoring

[Ian Campbell <ian.campbell@citrix.com>]

On Fri, 2015-02-13 at 13:09 +0000, Wei Liu wrote:
> On Mon, Feb 02, 2015 at 04:06:09PM +0800, Chao Peng wrote: 
> > +#ifdef LIBXL_HAVE_PSR_MBM
> > +int libxl_psr_cmt_type_supported(libxl_ctx *ctx, libxl_psr_cmt_type type);
> > +int libxl_psr_cmt_get_mem_bandwidth_sample(libxl_ctx *ctx,
> > +                                           uint32_t domid,
> > +                                           uint32_t socketid,
> > +                                           bool     total,
> > +                                           uint64_t *sample_r,
> > +                                           uint64_t *tsc_r);
> 
> Should the interface be only limited to socket level? Do you (Intel) has
> plan to provide bandwidth sampling on finer grain level (say, core
> level)?

If we assume not then obviously some h/w designer will implement it ;-)

The existing libxl_psr_cmt_get_l3_cache_size has uint32_t socketid
though, so we may as well do the same here and fixup everything at once
if it ever happens.

> I also assume that you will never have third type of memory bandwidth?
> I.e. do we need to change boolean total to a enum?

There is already an enum, but some of the values it has don't really fit
this interface (the "l3_cache_size" isn't really a stat which needs
sampling), cf my wonder in response to v8 "if wedging all of these
logically unrelated statistics into an umbrella psr interface at the
libxl level makes sense."

Perhaps just call this interface psr_cmt_get_sample, pass it the enum
and always return a tsc? Then other types of stats sampling would fit in
the future, hopefully.

No reason to exclude l3_cache_size from it even if it is a bit pointless
to use this interface for it instead of the dedicated one.

Ian.