<div>
<br>
</div>
<div><div><br></div><div>-- </div><div>Eli Qiao</div><div>Sent with <a href="http://www.sparrowmailapp.com/?sig">Sparrow</a></div><div><br></div></div>
<p style="color: #A0A0A8;">On Thursday, 9 February 2017 at 10:25 PM, Marcelo Tosatti wrote:</p>
<blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;">
<span><div><div><div>On Thu, Feb 09, 2017 at 03:43:01PM +0800, Eli Qiao wrote:</div><blockquote type="cite"><div><div>Addressed comment from v2 -> v3:</div><div><br></div><div>Daniel:</div><div> * Fixed coding style, passed `make check` and `make syntax-check`</div><div><br></div><div> * Variables renaming and move from header file to c file.</div><div><br></div><div> * For locking/mutex support, no progress.</div><div><br></div><div> There are some discussion from mailing list, but I can not find a better</div><div> way to add locking support without performance impact.</div><div><br></div><div> I'll explain the process and please help to advice what shoud we do.</div><div><br></div><div> VM create:</div><div> 1) Get the cache left value on each bank of the host. This should be</div><div> shared amount all VMs.</div><div> 2) Calculate the schemata on the bank based on all created resctrl</div><div> domain's schemata</div><div> 3) Calculate the default schemata by scaning all domain's schemata.</div><div> 4) Flush default schemata to /sys/fs/resctrl/schemata</div></div></blockquote><div><br></div><div>Yes, this 4 steps have to be serialized and cannot occur in parallel.</div><div><br></div><blockquote type="cite"><div><div><br></div><div> VM destroy:</div><div> 1) Remove the resctrl domain of that VM</div><div> 2) Recalculate default schemata</div><div> 3) Flush default schemata to /sys/fs/resctrl/schemata</div><div><br></div><div> The key point is that all VMs shares /sys/fs/resctrl/schemata, and</div><div> when a VM create a resctrl domain, the schemata of that VM depends on</div><div> the default schemata and all other exsited schematas. So a global</div><div> mutex is reqired.</div><div><br></div><div> Before calculate a schemata or update default schemata, libvirt</div><div> should gain this global mutex.</div><div><br></div><div> I will try to think more about how to support this gracefully in next</div><div> patch set.</div></div></blockquote><div><br></div><div>There are two things you need to protect:</div><div><br></div><div>1) Access to the filesystem (and schematas). For this you should use</div><div>the fcntl lock as mentioned previously:</div><div> <a href="https://lkml.org/lkml/2016/12/14/377">https://lkml.org/lkml/2016/12/14/377</a></div><div><br></div><div>Other applications that use resctrlfs should use the same lock.</div><div><br></div><div>All you have to do is to grab and release the fcntl lock as follows:</div><div><br></div><div>+#include <sys/file.h></div><div>+#include <stdlib.h></div><div>+</div><div>+void resctrl_take_shared_lock(int fd)</div><div>+{</div><div>+ int ret;</div><div>+</div><div>+ /* take shared lock on resctrl filesystem */</div><div>+ ret = flock(fd, LOCK_SH);</div><div>+ if (ret) {</div><div>+ perror("flock");</div><div>+ exit(-1);</div><div>+ }</div><div>+}</div><div>+</div><div>+void resctrl_take_exclusive_lock(int fd)</div><div>+{</div><div>+ int ret;</div><div>+</div><div>+ /* release lock on resctrl filesystem */</div><div>+ ret = flock(fd, LOCK_EX);</div><div>+ if (ret) {</div><div>+ perror("flock");</div><div>+ exit(-1);</div><div>+ }</div><div>+}</div><div>+</div><div>+void resctrl_release_lock(int fd)</div><div>+{</div><div>+ int ret;</div><div>+</div><div>+ /* take shared lock on resctrl filesystem */</div><div>+ ret = flock(fd, LOCK_UN);</div><div>+ if (ret) {</div><div>+ perror("flock");</div><div>+ exit(-1);</div><div>+ }</div><div>+}</div><div>+</div><div>+void main(void)</div><div>+{</div><div>+ int fd, ret;</div><div>+</div><div>+ fd = open("/sys/fs/resctrl", O_DIRECTORY);</div><div>+ if (fd == -1) {</div><div>+ perror("open");</div><div>+ exit(-1);</div><div>+ }</div><div>+ resctrl_take_shared_lock(fd);</div><div>+ /* code to read directory contents */</div><div>+ resctrl_release_lock(fd);</div><div>+</div><div>+ resctrl_take_exclusive_lock(fd);</div><div>+ /* code to read and write directory contents */</div><div>+ resctrl_release_lock(fd);</div><div>+}</div><div><br></div><div><br></div></div></div></span></blockquote><div>Thank Marcelo, libvirt have a util named <span style="color: rgb(121, 93, 163); font-family: Consolas, 'Liberation Mono', Menlo, Courier, monospace; font-size: 12px; orphans: 2; white-space: pre; widows: 2; background-color: rgb(248, 238, 199);">virFileLock</span></div><div> <a href="https://github.com/libvirt/libvirt/blob/master/src/util/virfile.c#L388">https://github.com/libvirt/libvirt/blob/master/src/util/virfile.c#L388</a> </div><div><br></div><div>would that be same with your flock?</div><blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;"><span><div><div><div><br></div><div>2) The internal data structures in libvirt. I suppose you can</div><div>simply add a mutex to protect all of the "struct _virResCtrl"</div><div>since there should be no contention on that lock.</div></div></div></span></blockquote><div>Sure, I will try to refine my code.</div><div><br></div><blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;"><span><div><div><blockquote type="cite"><div><div>Marcelo:</div><div> * Added vcpu support for cachetune, this will allow user to define which</div><div> vcpu using which cache allocation bank.</div><div><br></div><div> <cachetune id='0' host_id=0 size='3072' unit='KiB' vcpus='0,1'/></div><div><br></div><div> vcpus is a cpumap, the vcpu pids will be added to tasks file</div><div><br></div><div> * Added cdp compatible, user can specify l3 cache even host enable cdp.</div><div> See patch 8.</div><div> On a cdp enabled host, specify l3code/l3data by</div><div><br></div><div> <cachetune id='0' host_id='0' type='l3' size='3072' unit='KiB'/></div><div>rcelo@amt patchesv3]$ cd ..</div></div></blockquote><div> </div><blockquote type="cite"><div><div> This will create a schemata like:</div><div> L3data:0=0xff00;...</div><div> L3code:0=0xff00;...</div><div><br></div><div> * Would you please help to test if the functions work.</div></div></blockquote><div><br></div><div>Will do.</div><div><br></div><blockquote type="cite"><div><div>Martin:</div><div> * Xml test case, I have no time to work on this yet, would you please</div><div> show me an example, would like to amend it later.</div><div><br></div><div>This series patches are for supportting CAT featues, which also</div><div>called cache tune in libvirt.</div><div><br></div><div>First to expose cache information which could be tuned in capabilites XML.</div><div>Then add new domain xml element support to add cacahe bank which will apply</div><div>on this libvirt domain.</div><div><br></div><div>This series patches add a util file `resctrl.c/h`, an interface to talk with</div><div>linux kernel's sys fs.</div><div><br></div><div>There are still some TODOs such as expose new public interface to get free</div><div>cache information.</div></div></blockquote><div><br></div><div>Can you please list the TODOs?</div></div></div></span></blockquote><div><br></div><div>Sure, there’s one public API missing for query cache free on each bank. </div><blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;"><span><div><div><div> </div><div><br></div><blockquote type="cite"><div><div><br></div><div>Some discussion about this feature support can be found from:</div><div><a href="https://www.redhat.com/archives/libvir-list/2017-January/msg00644.html">https://www.redhat.com/archives/libvir-list/2017-January/msg00644.html</a></div><div><br></div><div>Eli Qiao (8):</div><div> Resctrl: Add some utils functions</div><div> Resctrl: expose cache information to capabilities</div><div> Resctrl: Add new xml element to support cache tune</div><div> Resctrl: Add private interface to set cachebanks</div><div> Qemu: Set cache banks</div><div> Resctrl: enable l3code/l3data</div><div> Resctrl: Make sure l3data/l3code are pairs</div><div> Resctrl: Compatible mode for cdp enabled</div><div><br></div><div> docs/schemas/domaincommon.rng | 46 ++</div><div> include/libvirt/virterror.h | 1 +</div><div> po/<a href="http://POTFILES.in">POTFILES.in</a> | 1 +</div><div> src/<a href="http://Makefile.am">Makefile.am</a> | 1 +</div><div> src/conf/capabilities.c | 56 +++</div><div> src/conf/capabilities.h | 23 +</div><div> src/conf/domain_conf.c | 182 +++++++</div><div> src/conf/domain_conf.h | 19 +</div><div> src/libvirt_private.syms | 11 +</div><div> src/nodeinfo.c | 64 +++</div><div> src/nodeinfo.h | 1 +</div><div> src/qemu/qemu_capabilities.c | 8 +</div><div> src/qemu/qemu_driver.c | 6 +</div><div> src/qemu/qemu_process.c | 53 ++</div><div> src/util/virerror.c | 1 +</div><div> src/util/virresctrl.c | 1091 +++++++++++++++++++++++++++++++++++++++++</div><div> src/util/virresctrl.h | 85 ++++</div><div> 17 files changed, 1649 insertions(+)</div><div> create mode 100644 src/util/virresctrl.c</div><div> create mode 100644 src/util/virresctrl.h</div><div><br></div><div>--</div><div>1.9.1</div></div></blockquote></div></div></span>
</blockquote>
<div>
<br>
</div>