<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 Friday, 10 February 2017 at 9:32 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 Fri, Feb 10, 2017 at 09:58:19AM +0800, Eli Qiao wrote:</div><blockquote type="cite"><div><div><br></div><div><br></div><div>-- </div><div>Eli Qiao</div><div>Sent with Sparrow (<a href="http://www.sparrowmailapp.com/?sig">http://www.sparrowmailapp.com/?sig</a>)</div><div><br></div><div><br></div><div>On Thursday, 9 February 2017 at 10:25 PM, Marcelo Tosatti wrote:</div><div><br></div><blockquote type="cite"><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> </div><div>Daniel:</div><div>* Fixed coding style, passed `make check` and `make syntax-check`</div><div> </div><div>* Variables renaming and move from header file to c file.</div><div> </div><div>* For locking/mutex support, no progress.</div><div> </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> </div><div>I'll explain the process and please help to advice what shoud we do.</div><div> </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> </div></div></blockquote><div> </div><div> </div><div>Yes, this 4 steps have to be serialized and cannot occur in parallel.</div><div> </div><blockquote type="cite"><div><div> </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> </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> </div><div>Before calculate a schemata or update default schemata, libvirt</div><div>should gain this global mutex.</div><div> </div><div>I will try to think more about how to support this gracefully in next</div><div>patch set.</div><div> </div></div></blockquote><div> </div><div> </div><div>There are two things you need to protect:</div><div> </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> </div><div>Other applications that use resctrlfs should use the same lock.</div><div> </div><div>All you have to do is to grab and release the fcntl lock as follows:</div><div> </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> </div><div> </div></div></blockquote><div>Thank Marcelo, libvirt have a util named virFileLock</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></div></blockquote><div><br></div><div>No, virFileLock uses:</div><div><br></div><div> Advisory record locking</div><div> Linux implements traditional ("process-associated") UNIX record</div><div> locks, as standardized by POSIX. For a Linux-specific</div><div>alternative</div><div> with better semantics, see the discussion of open file</div><div>description</div><div> locks below.</div><div><br></div><div> F_SETLK, F_SETLKW, and F_GETLK are used to acquire, release, and</div><div>test</div><div> for the existence of record locks (also known as byte-range,</div><div>file-</div><div> segment, or file-region locks). The third argument, lock, is a</div><div> pointer to a structure that has at least the following fields (in</div><div> unspecified order).</div><div><br></div><div> struct flock {</div><div> ...</div><div> short l_type; /* Type of lock: F_RDLCK,</div><div> F_WRLCK, F_UNLCK */</div><div> short l_whence; /* How to interpret l_start:</div><div> SEEK_SET, SEEK_CUR, SEEK_END */</div><div> off_t l_start; /* Starting offset for lock */</div><div> off_t l_len; /* Number of bytes to lock */</div><div> pid_t l_pid; /* PID of process blocking our lock</div><div> (set by F_GETLK and F_OFD_GETLK) */</div><div> ...</div><div> };</div><div><br></div><div> The l_whence, l_start, and l_len fields of this structure specify</div><div>the</div><div> range of bytes we wish to lock. Bytes past the end of the file</div><div>may</div><div> be locked, but not bytes before the start of the file.</div><div><br></div><div>Which is different than LOCK_SH/LOCK_EX. Please use LOCK_SH/LOCK_EX.</div><div><br></div></div></div></span></blockquote><div><br></div><div>okay, will use LOCK_SH/EX</div><div> </div><blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;"><span><div><div><div></div><blockquote type="cite"><div><blockquote type="cite"><div><div> </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><div> </div></div></blockquote><div><br></div><div>Sure, I will try to refine my code.</div><div><br></div><blockquote type="cite"><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> </div><div><cachetune id='0' host_id=0 size='3072' unit='KiB' vcpus='0,1'/></div><div> </div><div>vcpus is a cpumap, the vcpu pids will be added to tasks file</div><div> </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> </div><div><cachetune id='0' host_id='0' type='l3' size='3072' unit='KiB'/></div><div>rcelo@amt patchesv3]$ cd ..</div><div> </div></div></blockquote><div> </div><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> </div><div>* Would you please help to test if the functions work.</div></div></blockquote><div> </div><div>Will do.</div><div> </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> </div><div>This series patches are for supportting CAT featues, which also</div><div>called cache tune in libvirt.</div><div> </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> </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> </div><div>There are still some TODOs such as expose new public interface to get free</div><div>cache information.</div><div> </div></div></blockquote><div> </div><div> </div><div>Can you please list the TODOs?</div></div></blockquote><div><br></div><div>Sure, there’s one public API missing for query cache free on each bank. </div></div></blockquote><div><br></div><div>And that is all?</div></div></div></span></blockquote><div>Maybe another public API for live tune cache </div><blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;"><span><div><div><div> </div><div>(testing now, will let you know the results soon).</div></div></div></span>
</blockquote>
<div>
<br>
</div>