Re: [libvirt] [v6 09/10] Resctrl: concurrence support
On Wed, Feb 15, 2017 at 01:41:52PM +0800, Eli Qiao wrote:
The internal struct list domainall is a list which are resctral
domain
status shared by all VMs, especiall the default domain, each VM should
access it concomitantly. Ues a mutex to control it.
Each bank's cache_left field is also a global shared resource we need
to be care, add a mutex for each bank.
We need also to add lock to access /sys/fs/resctrl, use flock.
Signed-off-by: Eli Qiao <liyong.qiao(a)intel.com>
Hi Eli Qiao,
I don't think this is correct: the lock must be held across entire
operations, for example:
+4) Locking between applications
+
+Certain operations on the resctrl filesystem, composed of
+read / writes to multiple files, must be atomic.
+
+As an example, the allocation of an exclusive reservation
+of L3 cache involves:
+
+ 1. read list of cbmmasks for each directory
+ 2. find a contiguous set of bits in the global CBM bitmask
+ that is clear in any of the directory cbmmasks
+ 3. create a new directory
+ 4. set the bits found in step 2 to the new directory "schemata"
+ file
The lock must be held across steps 1 through 4, and not taken
and released in between these steps (which is what this patchset
has now).
Two libvirt examples:
VM INITIALIZATION:
1) Lock filesystem lock (LOCK_EX).
2) Scan directories.
3) Find free space.
4) Create new directory.
5) Write schemata files in new directory.
6) Write tasks file with vcpus pid.
7) Write default schemata to exclude space used by new
directory.
8) Unlock filesystem lock.
GET FREE SPACE API CALL:
1) Lock filesystem lock (LOCK_SH).
2) Scan directories.
3) Unlock filesystem lock.
4) return data to libvirt user.
Can you please fix this?
Other than this, and testing of v6 relative to usage of other apps
(which i plan to do tomorrow), patchset looks good to me.