[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [RFC v3 0/8] Support cache tune in libvirt



On Thu, Feb 09, 2017 at 03:43:01PM +0800, Eli Qiao wrote:
> Addressed comment from v2 -> v3:
> 
> Daniel:
>       * Fixed coding style, passed `make check` and `make syntax-check`
> 
>       * Variables renaming and move from header file to c file.
> 
>       * For locking/mutex support, no progress.
> 
>       There are some discussion from mailing list, but I can not find a better
>       way to add locking support without performance impact.
> 
>       I'll explain the process and please help to advice what shoud we do.
> 
>       VM create:
>       1) Get the cache left value on each bank of the host. This should be
>          shared amount all VMs.
>       2) Calculate the schemata on the bank based on all created resctrl
>          domain's schemata
>       3) Calculate the default schemata by scaning all domain's schemata.
>       4) Flush default schemata to /sys/fs/resctrl/schemata

Yes, this 4 steps have to be serialized and cannot occur in parallel.

> 
>       VM destroy:
>       1) Remove the resctrl domain of that VM
>       2) Recalculate default schemata
>       3) Flush default schemata to /sys/fs/resctrl/schemata
> 
>       The key point is that all VMs shares /sys/fs/resctrl/schemata, and
>       when a VM create a resctrl domain, the schemata of that VM depends on
>       the default schemata and all other exsited schematas. So a global
>       mutex is reqired.
> 
>       Before calculate a schemata or update default schemata, libvirt
>       should gain this global mutex.
> 
>       I will try to think more about how to support this gracefully in next
>       patch set.

There are two things you need to protect:

1) Access to the filesystem (and schematas). For this you should use
the fcntl lock as mentioned previously:
 https://lkml.org/lkml/2016/12/14/377

Other applications that use resctrlfs should use the same lock.

All you have to do is to grab and release the fcntl lock as follows:

+#include <sys/file.h>
+#include <stdlib.h>
+
+void resctrl_take_shared_lock(int fd)
+{
+	int ret;
+
+	/* take shared lock on resctrl filesystem */
+	ret = flock(fd, LOCK_SH);
+	if (ret) {
+		perror("flock");
+		exit(-1);
+	}
+}
+
+void resctrl_take_exclusive_lock(int fd)
+{
+	int ret;
+
+	/* release lock on resctrl filesystem */
+	ret = flock(fd, LOCK_EX);
+	if (ret) {
+		perror("flock");
+		exit(-1);
+	}
+}
+
+void resctrl_release_lock(int fd)
+{
+	int ret;
+
+	/* take shared lock on resctrl filesystem */
+	ret = flock(fd, LOCK_UN);
+	if (ret) {
+		perror("flock");
+		exit(-1);
+	}
+}
+
+void main(void)
+{
+	int fd, ret;
+
+	fd = open("/sys/fs/resctrl", O_DIRECTORY);
+	if (fd == -1) {
+		perror("open");
+		exit(-1);
+	}
+	resctrl_take_shared_lock(fd);
+	/* code to read directory contents */
+	resctrl_release_lock(fd);
+
+	resctrl_take_exclusive_lock(fd);
+	/* code to read and write directory contents */
+	resctrl_release_lock(fd);
+}


2) The internal data structures in libvirt. I suppose you can
simply add a mutex to protect all of the "struct _virResCtrl"
since there should be no contention on that lock.

> Marcelo:
>       * Added vcpu support for cachetune, this will allow user to define which
>         vcpu using which cache allocation bank.
> 
>         <cachetune id='0' host_id=0 size='3072' unit='KiB' vcpus='0,1'/>
> 
>         vcpus is a cpumap, the vcpu pids will be added to tasks file
> 
>       * Added cdp compatible, user can specify l3 cache even host enable cdp.
>         See patch 8.
>         On a cdp enabled host, specify l3code/l3data by
> 
>         <cachetune id='0' host_id='0' type='l3' size='3072' unit='KiB'/>
>rcelo amt patchesv3]$ cd ..
 
>         This will create a schemata like:
>             L3data:0=0xff00;...
>             L3code:0=0xff00;...
> 
>       * Would you please help to test if the functions work.

Will do.

> Martin:
>       * Xml test case, I have no time to work on this yet, would you please
>         show me an example, would like to amend it later.
> 
> This series patches are for supportting CAT featues, which also
> called cache tune in libvirt.
> 
> First to expose cache information which could be tuned in capabilites XML.
> Then add new domain xml element support to add cacahe bank which will apply
> on this libvirt domain.
> 
> This series patches add a util file `resctrl.c/h`, an interface to talk with
> linux kernel's sys fs.
> 
> There are still some TODOs such as expose new public interface to get free
> cache information.

Can you please list the TODOs? 

> 
> Some discussion about this feature support can be found from:
> https://www.redhat.com/archives/libvir-list/2017-January/msg00644.html
> 
> Eli Qiao (8):
>   Resctrl: Add some utils functions
>   Resctrl: expose cache information to capabilities
>   Resctrl: Add new xml element to support cache tune
>   Resctrl: Add private interface to set cachebanks
>   Qemu: Set cache banks
>   Resctrl: enable l3code/l3data
>   Resctrl: Make sure l3data/l3code are pairs
>   Resctrl: Compatible mode for cdp enabled
> 
>  docs/schemas/domaincommon.rng |   46 ++
>  include/libvirt/virterror.h   |    1 +
>  po/POTFILES.in                |    1 +
>  src/Makefile.am               |    1 +
>  src/conf/capabilities.c       |   56 +++
>  src/conf/capabilities.h       |   23 +
>  src/conf/domain_conf.c        |  182 +++++++
>  src/conf/domain_conf.h        |   19 +
>  src/libvirt_private.syms      |   11 +
>  src/nodeinfo.c                |   64 +++
>  src/nodeinfo.h                |    1 +
>  src/qemu/qemu_capabilities.c  |    8 +
>  src/qemu/qemu_driver.c        |    6 +
>  src/qemu/qemu_process.c       |   53 ++
>  src/util/virerror.c           |    1 +
>  src/util/virresctrl.c         | 1091 +++++++++++++++++++++++++++++++++++++++++
>  src/util/virresctrl.h         |   85 ++++
>  17 files changed, 1649 insertions(+)
>  create mode 100644 src/util/virresctrl.c
>  create mode 100644 src/util/virresctrl.h
> 
> --
> 1.9.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]