Re: [libvirt] [PATCH resend V10 01/12] Resctrl: Add some utils functions

On Mon, Mar 06, 2017 at 06:06:30PM +0800, Eli Qiao wrote: > This patch adds some utils struct and functions to expose resctrl > information. > > virResCtrlAvailable: if resctrl interface exist on host. > virResCtrlGet: get specific type resource control information. > virResCtrlInit: initialize resctrl struct from the host's sys fs. > resctrlall[]: an array to maintain resource control information. > > Some of host cpu related information methods was added in virhostcpu.c > > Signed-off-by: Eli Qiao <liyong.qiao(a)intel.com (mailto:liyong.qiao@intel.com)> > --- > include/libvirt/virterror.h | 1 + > po/POTFILES.in (http://POTFILES.in) | 1 + > src/Makefile.am (http://Makefile.am) | 1 + > src/libvirt_private.syms | 4 + > src/util/virerror.c | 1 + > src/util/virhostcpu.c | 186 ++++++++++++++++++++++++++++++++++++---- > src/util/virhostcpu.h | 6 ++ > src/util/virresctrl.c | 201 ++++++++++++++++++++++++++++++++++++++++++++ > src/util/virresctrl.h | 78 +++++++++++++++++ > 9 files changed, 462 insertions(+), 17 deletions(-) > create mode 100644 src/util/virresctrl.c > create mode 100644 src/util/virresctrl.h > > diff --git a/src/util/virerror.c b/src/util/virerror.c > index ef17fb5..0ba15e6 100644 > --- a/src/util/virerror.c > +++ b/src/util/virerror.c > @@ -139,6 +139,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, > > "Perf", /* 65 */ > "Libssh transport layer", > + "Resouce Control", > s/resouce/resource/ > diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c > new file mode 100644 > index 0000000..44a47cc > --- /dev/null > +++ b/src/util/virresctrl.c > @@ -0,0 +1,201 @@ > [...] > + > +static unsigned int host_id; > + > +static virResCtrl resctrlall[] = { > + { > + .name = "L3", > + .cache_level = "l3", > + }, > + { > + .name = "L3DATA", > + .cache_level = "l3", > + }, > + { > + .name = "L3CODE", > + .cache_level = "l3", > + }, > + { > + .name = "L2", > + .cache_level = "l2", > + }, > +}; > + > You are using global variables, still. But I *still* see no locking. What if yet another driver (not just QEMU) will want to use resctrl? Bunch of these accesses can happen at the same time and break everything. How much of this information do we really need to keep (and not reload)?

For example host_id can screw up a lot of things. I might be discussing in the latter patches as well.