Skip to contentSkip to conversation

intelrdt: resctrl: recommend locking for resctrlfs

PATCH Superseded Marcelo Tosatti 4 participants 11 messages Nov 30, 2016 – Dec 15, 2016 Documentation

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 intelrdt: resctrl: recommend locking for resctrlfs

There is a locking problem between different applications reading/writing to resctrlfs directory at the same time (read the patch below for details).

Suggest a standard locking scheme for applications to use.

x86/intel_rdt_ui.txt renamed from x86/intel_rdt_ui.txt.orig +27 -0
@@ -212,3 +212,30 @@ Finally we move core 4-7 over to the new
 kernel and the tasks running there get 50% of the cache.
 
 # echo C0 > p0/cpus
+
+4) Locking between applications
+
+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
+
+If two applications attempt to allocate space race with each other
+(if two processes execute the steps above in a interlocked fashion),
+they can end up using the same bits of CBMMASK, which renders the
+reservations non-exclusive but shared.
+
+To coordinate creation of reservations on resctrl and avoid the problem
+above, the following locking procedure is recommended:
+
+A) open /var/lock/resctrl/fs.lock with O_CREAT|O_EXCL.
+B) if success, write pid of program accessing the directory
+   structure to this file.
+C) read/write the directory structure.
+D) remove file.
+
#1 lore raw

x86/intel_rdt_ui.txt

Quoted 11 lines · Marcelo Tosatti

There is a locking problem between different applications reading/writing to resctrlfs directory at the same time (read the patch below for details).

Suggest a standard locking scheme for applications to use.

x86/intel_rdt_ui.txt renamed from x86/intel_rdt_ui.txt.orig +0 -0

I can't remember that we changed the -p1 patch format to -p0 :(

#2 lore raw
In reply to Marcelo Tosatti
Quoted 17 lines · Marcelo Tosatti

There is a locking problem between different applications reading/writing to resctrlfs directory at the same time (read the patch below for details).

Suggest a standard locking scheme for applications to use.

x86/intel_rdt_ui.txt renamed from x86/intel_rdt_ui.txt.orig +2 -0
@@ -212,3 +212,30 @@ Finally we move core 4-7 over to the new
 kernel and the tasks running there get 50% of the cache.
 
 # echo C0 > p0/cpus
+
+4) Locking between applications

x86/intel_rdt_ui.txt Finally we move core 4-7 over to the new · line 226

Quoted 10 lines
x86/intel_rdt_ui.txt renamed from x86/intel_rdt_ui.txt.orig +10 -0
+
+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

This is one example of why locking is needed. There are other scenarios that need the locking as well. For example, two applications scan each directory to find an empty/less loaded "tasks". Both of them find that directory p1 has empty "tasks" and write their own thread ids into the "tasks" in p1. Turns out the "tasks" in p1 will have crowded threads or workloads. A locking can solve this race scenario too.

As a user interface document, maybe we need a generic explanation why locking plus the example.

Quoted 15 lines
x86/intel_rdt_ui.txt renamed from x86/intel_rdt_ui.txt.orig +15 -0
+
+If two applications attempt to allocate space race with each other
+(if two processes execute the steps above in a interlocked fashion),
+they can end up using the same bits of CBMMASK, which renders the
+reservations non-exclusive but shared.
+
+To coordinate creation of reservations on resctrl and avoid the problem
+above, the following locking procedure is recommended:
+
+A) open /var/lock/resctrl/fs.lock with O_CREAT|O_EXCL.
+B) if success, write pid of program accessing the directory
+   structure to this file.
+C) read/write the directory structure.
+D) remove file.
+
#3 lore raw
Quoted 36 lines · Fenghua Yu
Quoted 17 lines · Marcelo Tosatti

There is a locking problem between different applications reading/writing to resctrlfs directory at the same time (read the patch below for details).

Suggest a standard locking scheme for applications to use.

x86/intel_rdt_ui.txt renamed from x86/intel_rdt_ui.txt.orig +2 -0
@@ -212,3 +212,30 @@ Finally we move core 4-7 over to the new
 kernel and the tasks running there get 50% of the cache.
 
 # echo C0 > p0/cpus
+
+4) Locking between applications

x86/intel_rdt_ui.txt Finally we move core 4-7 over to the new · line 226

Quoted 10 lines
x86/intel_rdt_ui.txt renamed from x86/intel_rdt_ui.txt.orig +10 -0
+
+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

This is one example of why locking is needed. There are other scenarios that need the locking as well. For example, two applications scan each directory to find an empty/less loaded "tasks". Both of them find that directory p1 has empty "tasks" and write their own thread ids into the "tasks" in p1. Turns out the "tasks" in p1 will have crowded threads or workloads. A locking can solve this race scenario too.

Quoted 2 lines

As a user interface document, maybe we need a generic explanation why locking plus the example.

Well, agreed there are other races, but in this particular example taking the file lock does not solve the "tasks" race: the contents of the tasks file can change in face of fork.

So i've added your suggestion but can't use this example, if you have another one you'd like to see added, please let me know... Replying with V2.

#4 lore raw
Quoted 37 lines · Marcelo Tosatti
Quoted 36 lines · Fenghua Yu
Quoted 17 lines · Marcelo Tosatti

There is a locking problem between different applications reading/writing to resctrlfs directory at the same time (read the patch below for details).

Suggest a standard locking scheme for applications to use.

x86/intel_rdt_ui.txt renamed from x86/intel_rdt_ui.txt.orig +2 -0
@@ -212,3 +212,30 @@ Finally we move core 4-7 over to the new
 kernel and the tasks running there get 50% of the cache.
 
 # echo C0 > p0/cpus
+
+4) Locking between applications

x86/intel_rdt_ui.txt Finally we move core 4-7 over to the new · line 226

Quoted 10 lines
x86/intel_rdt_ui.txt renamed from x86/intel_rdt_ui.txt.orig +10 -0
+
+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

This is one example of why locking is needed. There are other scenarios that need the locking as well. For example, two applications scan each directory to find an empty/less loaded "tasks". Both of them find that directory p1 has empty "tasks" and write their own thread ids into the "tasks" in p1. Turns out the "tasks" in p1 will have crowded threads or workloads. A locking can solve this race scenario too.

Quoted 6 lines
Quoted 2 lines

As a user interface document, maybe we need a generic explanation why locking plus the example.

Well, agreed there are other races, but in this particular example taking the file lock does not solve the "tasks" race: the contents of the tasks file can change in face of fork.

The "tasks" example is only for resolving the race when allocating two tasks to an empty rdtgroup. Once a task is allocated to a "tasks", the task's forked children will automatically stay with the task unless they are moved. Without the locking, task A and task B are allocated to the same directory because both of them thought "tasks" in the directory is empty. Then all forked children of both A and B will populate the directory and cause crowded cache.

Sure the contents of the "tasks" can change in face of fork. But the race of the allocating two groups of tasks can cause wrong decision to allocate them at the beginning.

Quoted 4 lines

So i've added your suggestion but can't use this example, if you have another one you'd like to see added, please let me know... Replying with V2.

Thanks.

-Fenghua

#5 lore raw

PATCH v2 1/1 intelrdt: resctrl: recommend locking for resctrlfs

In reply to Fenghua Yu

There is a locking problem between different applications reading/writing to resctrlfs directory at the same time (read the patch below for details).

Suggest a standard locking scheme for applications to use.

---
v2: Improve commentary about generality of locking problem ("Yu, Fenghua")
Proper patch -p level (Thomas Gleixner)

Documentation/x86/intel_rdt_ui.txt +30 -0
index d918d26..14b3214 100644
@@ -212,3 +212,33 @@ Finally we move core 4-7 over to the new group and make sure that the
 kernel and the tasks running there get 50% of the cache.
 
 # echo C0 > p0/cpus
+
+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
+
+If two applications attempting to allocate space race with each other
+(if two processes execute the steps above in a interlocked fashion),
+they can end up using the same bits of CBMMASK, which renders the
+reservations non-exclusive but shared.
+
+To coordinate atomic operations on resctrl and avoid the problem
+above, the following locking procedure is recommended:
+
+A) open /var/lock/resctrl/fs.lock with O_CREAT|O_EXCL.
+B) if success, write pid of program accessing the directory
+   structure to this file.
+C) read/write the directory structure.
+D) remove file.
+
#6 lore raw
Quoted 6 lines · Marcelo Tosatti

There is a locking problem between different applications reading/writing to resctrlfs directory at the same time (read the patch below for details).

Suggest a standard locking scheme for applications to use.

....

Documentation/x86/intel_rdt_ui.txt Finally we move core 4-7 over to the new group and make sure that the · line 243

Quoted 8 lines
Documentation/x86/intel_rdt_ui.txt +8 -0
+To coordinate atomic operations on resctrl and avoid the problem
+above, the following locking procedure is recommended:
+
+A) open /var/lock/resctrl/fs.lock with O_CREAT|O_EXCL.
+B) if success, write pid of program accessing the directory
+   structure to this file.
+C) read/write the directory structure.
+D) remove file.

What's wrong with using flock, which works from shell scripts as well?

Thanks,

tglx

#7 lore raw
Quoted 21 lines · Thomas Gleixner
Quoted 6 lines · Marcelo Tosatti

There is a locking problem between different applications reading/writing to resctrlfs directory at the same time (read the patch below for details).

Suggest a standard locking scheme for applications to use.

....

Documentation/x86/intel_rdt_ui.txt Finally we move core 4-7 over to the new group and make sure that the · line 243

Quoted 8 lines
Documentation/x86/intel_rdt_ui.txt +8 -0
+To coordinate atomic operations on resctrl and avoid the problem
+above, the following locking procedure is recommended:
+
+A) open /var/lock/resctrl/fs.lock with O_CREAT|O_EXCL.
+B) if success, write pid of program accessing the directory
+   structure to this file.
+C) read/write the directory structure.
+D) remove file.

What's wrong with using flock, which works from shell scripts as well?

Quoted 3 lines

Thanks,

tglx

Hi Thomas,

Nothing wrong with it... I'm just copying the behaviour of other programs.

Actually, using flock(2) allows one to use LOCK_SH for readers and this allows consistent writer/reader behaviour (say, a reader won't see a partially written directory).

NAME
flock - apply or remove an advisory lock on an open file

SYNOPSIS

       #include <sys/file.h>

       int flock(int fd, int operation);

DESCRIPTION
Apply or remove an advisory lock on the open file specified by
fd. The argument operation is one of
the following:

LOCK_SH Place a shared lock. More than one process may hold
a shared lock for a given file at a
given time.

LOCK_EX Place an exclusive lock. Only one process may
hold an exclusive lock for a given file
at a given time.

LOCK_UN Remove an existing lock held by this process.

---

So the procedure would be:

/var/lock/resctrl/fs.lock created previously in the filesystem.

WRITE LOCK:

A) Take flock(EXCLUSIVE) on /var/lock/resctrl/fs.lock
B) If success, write pid of the program to the file.
C) read/write the directory structure.
D) funlock

READ LOCK:

A) Take flock(SHARED) on /var/lock/resctrl/fs.lock
B) If success read the directory structure.
C) funlock

How about that?

#8 lore raw
Quoted 3 lines · Marcelo Tosatti

Actually, using flock(2) allows one to use LOCK_SH for readers and this allows consistent writer/reader behaviour (say, a reader won't see a partially written directory).

Indeed.

Quoted 11 lines

So the procedure would be:

/var/lock/resctrl/fs.lock created previously in the filesystem.

WRITE LOCK:

A) Take flock(EXCLUSIVE) on /var/lock/resctrl/fs.lock
B) If success, write pid of the program to the file.
C) read/write the directory structure.
D) funlock

Quoted 8 lines

READ LOCK:

A) Take flock(SHARED) on /var/lock/resctrl/fs.lock
B) If success read the directory structure.
C) funlock

How about that?

Looks sane. Adding a small example in C and [ba]sh would be nice.

Thanks,

tglx

#9 lore raw

PATCH v3 1/1 intelrdt: resctrl: recommend locking for resctrlfs

In reply to Thomas Gleixner

There is a locking problem between different applications reading/writing to resctrlfs directory at the same time (read the patch below for details).

Suggest a standard locking scheme for applications to use.

Documentation/x86/intel_rdt_ui.txt +115 -0
index d918d26..ec3a58b 100644
@@ -212,3 +212,118 @@ Finally we move core 4-7 over to the new group and make sure that the
 kernel and the tasks running there get 50% of the cache.
 
 # echo C0 > p0/cpus
+
+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
+
+If two applications attempting to allocate space race with each other
+(if two processes execute the steps above in a interlocked fashion),
+they can end up using the same bits of CBMMASK, which renders the
+reservations non-exclusive but shared.
+
+To coordinate atomic operations on resctrl and avoid the problem
+above, the following locking procedure is recommended:
+
+WRITE LOCK:
+
+ A) Take flock(LOCK_EX) on /sys/fs/resctrl
+ B) read/write the directory structure.
+ C) funlock
+
+READ LOCK:
+
+ A) Take flock(LOCK_SH) on /sys/fs/resctrl
+ B) If success read the directory structure.
+ C) funlock
+
+Example with bash:
+
+# Atomically read directory structure
+$ flock -s /sys/fs/resctrl/ find /sys/fs/resctrl
+
+# Read directory contents and create new subdirectory
+
+$ cat create-dir.sh
+find /sys/fs/resctrl/ > output.txt
+mask = function-of(output.txt)
+mkdir /sys/fs/resctrl/newres/
+echo mask > /sys/fs/resctrl/newres/schemata
+
+$ flock /sys/fs/resctrl/ ./create-dir.sh
+
+Example with C:
+
+/*
+ * Example code do take advisory locks
+ * before accessing resctrl filesystem
+ */
+#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);
+}
+
+
#10 lore raw

[tip:x86/cache] Documentation, x86, resctrl: Recommend locking for resctrlfs

Documentation, x86, resctrl: Recommend locking for resctrlfs
Commit-ID:  3c2a769de7955ff81818b49d388dd771bf6ae29d
Gitweb:     http://git.kernel.org/tip/3c2a769de7955ff81818b49d388dd771bf6ae29d
Author:     Marcelo Tosatti <[email protected]>
AuthorDate: Wed, 14 Dec 2016 15:08:37 -0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Thu, 15 Dec 2016 14:44:27 +0100

Documentation, x86, resctrl: Recommend locking for resctrlfs

Concurrent write or read/write access from applications to the resctrlfs directory can result in incorrect readouts or setups.

Recommend a standard locking scheme for applications to use.

---
 Documentation/x86/intel_rdt_ui.txt | 114 +++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)
Documentation/x86/intel_rdt_ui.txt +114 -0
index d918d26..51cf6fa 100644
@@ -212,3 +212,117 @@ Finally we move core 4-7 over to the new group and make sure that the
 kernel and the tasks running there get 50% of the cache.
 
 # echo C0 > p0/cpus
+
+4) Locking between applications
+
+Certain operations on the resctrl filesystem, composed of read/writes
+to/from multiple files, must be atomic.
+
+As an example, the allocation of an exclusive reservation of L3 cache
+involves:
+
+  1. Read the cbmmasks from 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
+
+If two applications attempt to allocate space concurrently then they can
+end up allocating the same bits so the reservations are shared instead of
+exclusive.
+
+To coordinate atomic operations on the resctrlfs and to avoid the problem
+above, the following locking procedure is recommended:
+
+Locking is based on flock, which is available in libc and also as a shell
+script command
+
+Write lock:
+
+ A) Take flock(LOCK_EX) on /sys/fs/resctrl
+ B) Read/write the directory structure.
+ C) funlock
+
+Read lock:
+
+ A) Take flock(LOCK_SH) on /sys/fs/resctrl
+ B) If success read the directory structure.
+ C) funlock
+
+Example with bash:
+
+# Atomically read directory structure
+$ flock -s /sys/fs/resctrl/ find /sys/fs/resctrl
+
+# Read directory contents and create new subdirectory
+
+$ cat create-dir.sh
+find /sys/fs/resctrl/ > output.txt
+mask = function-of(output.txt)
+mkdir /sys/fs/resctrl/newres/
+echo mask > /sys/fs/resctrl/newres/schemata
+
+$ flock /sys/fs/resctrl/ ./create-dir.sh
+
+Example with C:
+
+/*
+ * Example code do take advisory locks
+ * before accessing resctrl filesystem
+ */
+#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);
+}