Intel(R) Resource Director Technology Cache Pseudo-Locking enabling

The cache

Documentation/x86/intel_rdt_ui.txt +6 -0

+pseudo-locked memory is made accessible to user space where an
+application can map it into its virtual address space and thus have
+a region of memory with reduced average read latency.
+
+Cache pseudo-locking increases the probability that data will remain
+in the cache via carefully configuring the CAT feature and controlling

Documentation/x86/intel_rdt_ui.txt L3CODE:0=fffff;1=fffff;2=fffff;3=fffff

Quoted 8 lines · Reinette Chatre

Documentation/x86/intel_rdt_ui.txt +8 -0

+Cache Pseudo-Locking
+--------------------
+CAT enables a user to specify the amount of cache space into which an
+application can fill. Cache pseudo-locking builds on the fact that a
+CPU can still read and write data pre-allocated outside its current
+allocated area on a cache hit. With cache pseudo-locking, data can be
+preloaded into a reserved portion of cache that no application can
+fill, and from that point on will only serve cache hits.

This lacks explanation how that preloading works.

Quoted 13 lines

The cache

Documentation/x86/intel_rdt_ui.txt +11 -0

+pseudo-locked memory is made accessible to user space where an
+application can map it into its virtual address space and thus have
+a region of memory with reduced average read latency.
+
+Cache pseudo-locking increases the probability that data will remain
+in the cache via carefully configuring the CAT feature and controlling
+application behavior. There is no guarantee that data is placed in
+cache. Instructions like INVD, WBINVD, CLFLUSH, etc. can still evict
+“locked” data from cache. Power management C-states may shrink or
+power off cache. It is thus recommended to limit the processor maximum
+C-state, for example, by setting the processor.max_cstate kernel parameter.

Documentation/x86/intel_rdt_ui.txt L3CODE:0=fffff;1=fffff;2=fffff;3=fffff · line 358

Quoted 5 lines

Documentation/x86/intel_rdt_ui.txt +5 -0

+
+It is required that an application using a pseudo-locked region runs
+with affinity to the cores (or a subset of the cores) associated
+with the cache on which the pseudo-locked region resides. This is
+enforced by the implementation.

Well, you only enforce in pseudo_lock_dev_mmap() that the caller is affine to the right CPUs. But that's not a guarantee that the task stays there.

Hi Thomas,

Quoted 11 lines · Thomas Gleixner

Documentation/x86/intel_rdt_ui.txt L3CODE:0=fffff;1=fffff;2=fffff;3=fffff

Quoted 8 lines · Reinette Chatre

Documentation/x86/intel_rdt_ui.txt +8 -0

+Cache Pseudo-Locking
+--------------------
+CAT enables a user to specify the amount of cache space into which an
+application can fill. Cache pseudo-locking builds on the fact that a
+CPU can still read and write data pre-allocated outside its current
+allocated area on a cache hit. With cache pseudo-locking, data can be
+preloaded into a reserved portion of cache that no application can
+fill, and from that point on will only serve cache hits.

This lacks explanation how that preloading works.

Following this text you quote there is a brief explanation starting with "Pseudo-locking is accomplished in two stages:" - I'll add more details to that area.

Quoted 13 lines

Quoted 13 lines

The cache

Documentation/x86/intel_rdt_ui.txt +11 -0

+pseudo-locked memory is made accessible to user space where an
+application can map it into its virtual address space and thus have
+a region of memory with reduced average read latency.
+
+Cache pseudo-locking increases the probability that data will remain
+in the cache via carefully configuring the CAT feature and controlling
+application behavior. There is no guarantee that data is placed in
+cache. Instructions like INVD, WBINVD, CLFLUSH, etc. can still evict
+“locked” data from cache. Power management C-states may shrink or
+power off cache. It is thus recommended to limit the processor maximum
+C-state, for example, by setting the processor.max_cstate kernel parameter.

Quoted 8 lines

Documentation/x86/intel_rdt_ui.txt L3CODE:0=fffff;1=fffff;2=fffff;3=fffff · line 358

Quoted 5 lines

Documentation/x86/intel_rdt_ui.txt +5 -0

+
+It is required that an application using a pseudo-locked region runs
+with affinity to the cores (or a subset of the cores) associated
+with the cache on which the pseudo-locked region resides. This is
+enforced by the implementation.

Well, you only enforce in pseudo_lock_dev_mmap() that the caller is affine to the right CPUs. But that's not a guarantee that the task stays there.

It is required that the user space application self sets affinity to cores associated with the cache. This is also highlighted in the example application code (later in this patch) within the comments as well as the example usage of sched_setaffinity(). The enforcement done in the kernel code is done as a check that the user space application did so, no the actual affinity management.

Quoted 6 lines

It is required that the user space application self sets affinity to cores associated with the cache. This is also highlighted in the example application code (later in this patch) within the comments as well as the example usage of sched_setaffinity(). The enforcement done in the kernel code is done as a check that the user space application did so, no the actual affinity management.

Right, but your documentation claims it's enforced. There is no enforcement aside of the initial sanity check.

Add description of Cache Pseudo-Locking feature, its interface, as well as an example of its usage.

Signed-off-by Reinette Chatre <[email protected]>

---
 Documentation/x86/intel_rdt_ui.txt | 229 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 228 insertions(+), 1 deletion(-)

Documentation/x86/intel_rdt_ui.txt +0 -0

index 756fd76b78a6..bb3d6fe0a3e4 100644

Documentation/x86/intel_rdt_ui.txt +27 -0

+application can fill. Cache pseudo-locking builds on the fact that a
+CPU can still read and write data pre-allocated outside its current
+allocated area on a cache hit. With cache pseudo-locking, data can be
+preloaded into a reserved portion of cache that no application can
+fill, and from that point on will only serve cache hits. The cache
+pseudo-locked memory is made accessible to user space where an
+application can map it into its virtual address space and thus have
+a region of memory with reduced average read latency.
+
+Cache pseudo-locking increases the probability that data will remain
+in the cache via carefully configuring the CAT feature and controlling
+application behavior. There is no guarantee that data is placed in
+cache. Instructions like INVD, WBINVD, CLFLUSH, etc. can still evict
+“locked” data from cache. Power management C-states may shrink or
+power off cache. It is thus recommended to limit the processor maximum
+C-state, for example, by setting the processor.max_cstate kernel parameter.
+
+It is required that an application using a pseudo-locked region runs
+with affinity to the cores (or a subset of the cores) associated
+with the cache on which the pseudo-locked region resides. This is
+enforced by the implementation.
+
+Pseudo-locking is accomplished in two stages:
+1) During the first stage the system administrator allocates a portion
+   of cache that should be dedicated to pseudo-locking. At this time an
+   equivalent portion of memory is allocated, loaded into allocated
+   cache portion, and exposed as a character device.

Documentation/x86/intel_rdt_ui.txt +5 -0

+	single cache instance. Writing of schemata to this file will
+	return success on successful pseudo-locked region setup.
+"size":
+	After successful pseudo-locked region setup this read-only file
+	will contain the size in bytes of pseudo-locked region.

Quoted 17 lines · Reinette Chatre

Add description of Cache Pseudo-Locking feature, its interface, as well as an example of its usage.

Signed-off-by Reinette Chatre <[email protected]>

---
 Documentation/x86/intel_rdt_ui.txt | 229 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 228 insertions(+), 1 deletion(-)

Documentation/x86/intel_rdt_ui.txt +0 -0

index 756fd76b78a6..bb3d6fe0a3e4 100644
@@ -329,6 +332,149 @@ L3CODE:0=fffff;1=fffff;2=fffff;3=fffff
 L3DATA:0=fffff;1=fffff;2=3c0;3=fffff
 L3CODE:0=fffff;1=fffff;2=fffff;3=fffff
 

Documentation/x86/intel_rdt_ui.txt L3CODE:0=fffff;1=fffff;2=fffff;3=fffff · line 337

Quoted 3 lines

Documentation/x86/intel_rdt_ui.txt +3 -0

+Cache Pseudo-Locking
+--------------------
+CAT enables a user to specify the amount of cache space into which an

space that an

Quoted 32 lines

Documentation/x86/intel_rdt_ui.txt +31 -0

+application can fill. Cache pseudo-locking builds on the fact that a
+CPU can still read and write data pre-allocated outside its current
+allocated area on a cache hit. With cache pseudo-locking, data can be
+preloaded into a reserved portion of cache that no application can
+fill, and from that point on will only serve cache hits. The cache
+pseudo-locked memory is made accessible to user space where an
+application can map it into its virtual address space and thus have
+a region of memory with reduced average read latency.
+
+Cache pseudo-locking increases the probability that data will remain
+in the cache via carefully configuring the CAT feature and controlling
+application behavior. There is no guarantee that data is placed in
+cache. Instructions like INVD, WBINVD, CLFLUSH, etc. can still evict
+“locked” data from cache. Power management C-states may shrink or
+power off cache. It is thus recommended to limit the processor maximum
+C-state, for example, by setting the processor.max_cstate kernel parameter.
+
+It is required that an application using a pseudo-locked region runs
+with affinity to the cores (or a subset of the cores) associated
+with the cache on which the pseudo-locked region resides. This is
+enforced by the implementation.
+
+Pseudo-locking is accomplished in two stages:
+1) During the first stage the system administrator allocates a portion
+   of cache that should be dedicated to pseudo-locking. At this time an
+   equivalent portion of memory is allocated, loaded into allocated
+   cache portion, and exposed as a character device.
+2) During the second stage a user-space application maps (mmap()) the
+   pseudo-locked memory into its address space.
+
+Cache Pseudo-Locking Interface

Documentation/x86/intel_rdt_ui.txt L3CODE:0=fffff;1=fffff;2=fffff;3=fffff · line 374

Quoted 6 lines

Documentation/x86/intel_rdt_ui.txt +6 -0

+------------------------------
+Platforms supporting cache pseudo-locking will expose a new
+"/sys/fs/restrl/pseudo_lock" directory after successful mount of the
+resctrl filesystem. Initially this directory will contain a single file,
+"avail" that contains the schemata, one line per resource, of cache region
+available for pseudo-locking.

uh, sysfs is supposed to be one value per file.

Documentation/x86/intel_rdt_ui.txt L3CODE:0=fffff;1=fffff;2=fffff;3=fffff · line 382

Quoted 7 lines

Documentation/x86/intel_rdt_ui.txt +7 -0

+A pseudo-locked region is created by creating a new directory within
+/sys/fs/resctrl/pseudo_lock. On success two new files will appear in
+the directory:
+
+"schemata":
+	Shows the schemata representing the pseudo-locked cache region.
+	User writes schemata of requested locked area to file.

use complete sentences, please. E.g.:

The user writes the schemata of the requested locked area to the file.

Documentation/x86/intel_rdt_ui.txt L3CODE:0=fffff;1=fffff;2=fffff;3=fffff · line 383

Quoted 1 line

Documentation/x86/intel_rdt_ui.txt +1 -0

+	Only one id of single resource accepted - can only lock from

of a single resource is accepted -

Documentation/x86/intel_rdt_ui.txt L3CODE:0=fffff;1=fffff;2=fffff;3=fffff · line 388

Quoted 5 lines

Documentation/x86/intel_rdt_ui.txt +5 -0

+	single cache instance. Writing of schemata to this file will
+	return success on successful pseudo-locked region setup.
+"size":
+	After successful pseudo-locked region setup this read-only file
+	will contain the size in bytes of pseudo-locked region.

System administrator creates/removes pseudo-locked regions by creating/removing directories in the pseudo-lock subdirectory of the resctrl filesystem. Here we add directory creation and removal support.

A "pseudo-lock region" is introduced, which represents an instance of a pseudo-locked cache region. During mkdir a new region is created but since we do not know which cache it belongs to at that time we maintain a global pointer to it from where it will be moved to the cache (rdt_domain) it belongs to after initialization. This implies that we only support one uninitialized pseudo-locked region at a time.

Signed-off-by Reinette Chatre <[email protected]>

---
 arch/x86/kernel/cpu/intel_rdt.h             |   3 +
 arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c | 220 +++++++++++++++++++++++++++-
 2 files changed, 222 insertions(+), 1 deletion(-)

arch/x86/kernel/cpu/intel_rdt.h +3 -0

index 8f5ded384e19..55f085985072 100644
@@ -352,6 +352,7 @@ extern struct mutex rdtgroup_mutex;
 extern struct rdt_resource rdt_resources_all[];
 extern struct rdtgroup rdtgroup_default;
 DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
+extern struct kernfs_node *pseudo_lock_kn;
 
 int __init rdtgroup_init(void);
 

@@ -457,5 +458,7 @@ bool has_busy_rmid(struct rdt_resource *r, struct rdt_domain *d);
 void __check_limbo(struct rdt_domain *d, bool force_free);
 int rdt_pseudo_lock_fs_init(struct kernfs_node *root);
 void rdt_pseudo_lock_fs_remove(void);
+int rdt_pseudo_lock_mkdir(const char *name, umode_t mode);
+int rdt_pseudo_lock_rmdir(struct kernfs_node *kn);
 
 #endif /* _ASM_X86_INTEL_RDT_H */

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +34 -1

index a787a103c432..7a22e367b82f 100644
@@ -20,11 +20,142 @@
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
 #include <linux/kernfs.h>
+#include <linux/kref.h>
 #include <linux/seq_file.h>
 #include <linux/stat.h>
+#include <linux/slab.h>
 #include "intel_rdt.h"
 
-static struct kernfs_node *pseudo_lock_kn;
+struct kernfs_node *pseudo_lock_kn;
+
+/*
+ * Protect the pseudo_lock_region access. Since we will link to
+ * pseudo_lock_region from rdt domains rdtgroup_mutex should be obtained
+ * first if needed.
+ */
+static DEFINE_MUTEX(rdt_pseudo_lock_mutex);
+
+/**
+ * struct pseudo_lock_region - pseudo-lock region information
+ * @kn:			kernfs node representing this region in the resctrl
+ *			filesystem
+ * @cbm:		bitmask of the pseudo-locked region
+ * @cpu:		core associated with the cache on which the setup code
+ *			will be run
+ * @minor:		minor number of character device associated with this
+ *			region
+ * @locked:		state indicating if this region has been locked or not
+ * @refcount:		how many are waiting to access this pseudo-lock
+ *			region via kernfs
+ * @deleted:		user requested removal of region via rmdir on kernfs
+ */
+struct pseudo_lock_region {
+	struct kernfs_node	*kn;
+	u32			cbm;
+	int			cpu;
+	unsigned int		minor;
+	bool			locked;
+	struct kref		refcount;
+	bool			deleted;
+};

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +19 -0

+	mutex_lock(&rdt_pseudo_lock_mutex);
+	__pseudo_lock_region_release(plr);
+	mutex_unlock(&rdt_pseudo_lock_mutex);
+}
+
+/**
+ * pseudo_lock_region_kn_lock - Obtain lock to pseudo-lock region kernfs node
+ *
+ * This is called from the kernfs related functions which are called with
+ * an active reference to the kernfs_node that contains a valid pointer to
+ * the pseudo-lock region it represents. We can thus safely take an active
+ * reference to the pseudo-lock region before dropping the reference to the
+ * kernfs_node.
+ *
+ * We need to handle the scenarios where the kernfs directory representing
+ * this pseudo-lock region can be removed while an application still has an
+ * open handle to one of the directory's files and operations on this
+ * handle are attempted.
+ * To support this we allow a file operation to drop its reference to the

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +7 -0

+int rdt_pseudo_lock_mkdir(const char *name, umode_t mode)
+{
+	struct pseudo_lock_region *plr;
+	struct kernfs_node *kn;
+	int ret = 0;
+
+	mutex_lock(&rdtgroup_mutex);

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +27 -0

+		goto out;
+	}
+
+	kn = kernfs_create_dir(pseudo_lock_kn, name, mode, plr);
+	if (IS_ERR(kn)) {
+		ret = PTR_ERR(kn);
+		goto out_free;
+	}
+
+	plr->kn = kn;
+	ret = rdtgroup_kn_set_ugid(kn);
+	if (ret)
+		goto out_remove;
+
+	kref_init(&plr->refcount);
+	kernfs_activate(kn);
+	new_plr = plr;
+	ret = 0;
+	goto out;
+
+out_remove:
+	kernfs_remove(kn);
+out_free:
+	kfree(plr);
+out:
+	mutex_unlock(&rdt_pseudo_lock_mutex);
+	mutex_unlock(&rdtgroup_mutex);

Quoted 10 lines · Reinette Chatre

System administrator creates/removes pseudo-locked regions by creating/removing directories in the pseudo-lock subdirectory of the resctrl filesystem. Here we add directory creation and removal support.

A "pseudo-lock region" is introduced, which represents an instance of a pseudo-locked cache region. During mkdir a new region is created but since we do not know which cache it belongs to at that time we maintain a global pointer to it from where it will be moved to the cache (rdt_domain) it belongs to after initialization. This implies that we only support one uninitialized pseudo-locked region at a time.

Whats the reason for this restriction? If there are uninitialized directories, so what?

Quoted 77 lines

Signed-off-by Reinette Chatre <[email protected]>

---
 arch/x86/kernel/cpu/intel_rdt.h             |   3 +
 arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c | 220 +++++++++++++++++++++++++++-
 2 files changed, 222 insertions(+), 1 deletion(-)

arch/x86/kernel/cpu/intel_rdt.h +3 -0

index 8f5ded384e19..55f085985072 100644
@@ -352,6 +352,7 @@ extern struct mutex rdtgroup_mutex;
 extern struct rdt_resource rdt_resources_all[];
 extern struct rdtgroup rdtgroup_default;
 DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
+extern struct kernfs_node *pseudo_lock_kn;
 
 int __init rdtgroup_init(void);
 

@@ -457,5 +458,7 @@ bool has_busy_rmid(struct rdt_resource *r, struct rdt_domain *d);
 void __check_limbo(struct rdt_domain *d, bool force_free);
 int rdt_pseudo_lock_fs_init(struct kernfs_node *root);
 void rdt_pseudo_lock_fs_remove(void);
+int rdt_pseudo_lock_mkdir(const char *name, umode_t mode);
+int rdt_pseudo_lock_rmdir(struct kernfs_node *kn);
 
 #endif /* _ASM_X86_INTEL_RDT_H */

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +38 -1

index a787a103c432..7a22e367b82f 100644
@@ -20,11 +20,142 @@
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
 #include <linux/kernfs.h>
+#include <linux/kref.h>
 #include <linux/seq_file.h>
 #include <linux/stat.h>
+#include <linux/slab.h>
 #include "intel_rdt.h"
 
-static struct kernfs_node *pseudo_lock_kn;
+struct kernfs_node *pseudo_lock_kn;
+
+/*
+ * Protect the pseudo_lock_region access. Since we will link to
+ * pseudo_lock_region from rdt domains rdtgroup_mutex should be obtained
+ * first if needed.
+ */
+static DEFINE_MUTEX(rdt_pseudo_lock_mutex);
+
+/**
+ * struct pseudo_lock_region - pseudo-lock region information
+ * @kn:			kernfs node representing this region in the resctrl
+ *			filesystem
+ * @cbm:		bitmask of the pseudo-locked region
+ * @cpu:		core associated with the cache on which the setup code
+ *			will be run
+ * @minor:		minor number of character device associated with this
+ *			region
+ * @locked:		state indicating if this region has been locked or not
+ * @refcount:		how many are waiting to access this pseudo-lock
+ *			region via kernfs
+ * @deleted:		user requested removal of region via rmdir on kernfs
+ */
+struct pseudo_lock_region {
+	struct kernfs_node	*kn;
+	u32			cbm;
+	int			cpu;
+	unsigned int		minor;
+	bool			locked;
+	struct kref		refcount;
+	bool			deleted;
+};
+
+/*
+ * Only one uninitialized pseudo-locked region can exist at a time. An
+ * uninitialized pseudo-locked region is created when the user creates a

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c line 70

Quoted 6 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +6 -0

+ * new directory within the pseudo_lock subdirectory of the resctrl
+ * filsystem. The user will initialize the pseudo-locked region by writing
+ * to its schemata file at which point this structure will be moved to the
+ * cache domain it belongs to.
+ */
+static struct pseudo_lock_region *new_plr;

Why isn't the struct pointer not stored in the corresponding kernfs's node->priv?

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c line 78

Quoted 7 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +7 -0

+static void __pseudo_lock_region_release(struct pseudo_lock_region *plr)
+{
+	bool is_new_plr = (plr == new_plr);
+
+	WARN_ON(!plr->deleted);
+	if (!plr->deleted)
+		return;

if (WARN_ON(...))

		return;

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c line 89

Quoted 11 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +11 -0

+
+	kfree(plr);
+	if (is_new_plr)
+		new_plr = NULL;
+}
+
+static void pseudo_lock_region_release(struct kref *ref)
+{
+	struct pseudo_lock_region *plr = container_of(ref,
+						      struct pseudo_lock_region,
+						      refcount);

You simply can avoid those line breaks by:

	struct pseudo_lock_region *plr;

	plr = container_of(ref, struct pseudo_lock_region, refcount);

Hmm?

Quoted 23 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +23 -0

+	mutex_lock(&rdt_pseudo_lock_mutex);
+	__pseudo_lock_region_release(plr);
+	mutex_unlock(&rdt_pseudo_lock_mutex);
+}
+
+/**
+ * pseudo_lock_region_kn_lock - Obtain lock to pseudo-lock region kernfs node
+ *
+ * This is called from the kernfs related functions which are called with
+ * an active reference to the kernfs_node that contains a valid pointer to
+ * the pseudo-lock region it represents. We can thus safely take an active
+ * reference to the pseudo-lock region before dropping the reference to the
+ * kernfs_node.
+ *
+ * We need to handle the scenarios where the kernfs directory representing
+ * this pseudo-lock region can be removed while an application still has an
+ * open handle to one of the directory's files and operations on this
+ * handle are attempted.
+ * To support this we allow a file operation to drop its reference to the
+ * kernfs_node so that the removal can proceed, while using the mutex to
+ * ensure these operations on the pseudo-lock region are serialized. At the
+ * time an operation does obtain access to the region it may thus have been
+ * deleted.

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c line 119

Quoted 6 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +6 -0

+ */
+static struct pseudo_lock_region *pseudo_lock_region_kn_lock(
+						struct kernfs_node *kn)
+{
+	struct pseudo_lock_region *plr = (kernfs_type(kn) == KERNFS_DIR) ?
+						kn->priv : kn->parent->priv;

See above.

Quoted 12 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +11 -0

+int rdt_pseudo_lock_mkdir(const char *name, umode_t mode)
+{
+	struct pseudo_lock_region *plr;
+	struct kernfs_node *kn;
+	int ret = 0;
+
+	mutex_lock(&rdtgroup_mutex);
+	mutex_lock(&rdt_pseudo_lock_mutex);
+
+	if (new_plr) {
+		ret = -ENOSPC;

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c static struct kernfs_ops pseudo_lock_avail_ops = { · line 246

Quoted 6 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +6 -0

+		goto out;
+	}
+
+	plr = kzalloc(sizeof(*plr), GFP_KERNEL);
+	if (!plr) {
+		ret = -ENOSPC;

ENOMEM is the proper error code here.

Quoted 32 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +31 -0

+		goto out;
+	}
+
+	kn = kernfs_create_dir(pseudo_lock_kn, name, mode, plr);
+	if (IS_ERR(kn)) {
+		ret = PTR_ERR(kn);
+		goto out_free;
+	}
+
+	plr->kn = kn;
+	ret = rdtgroup_kn_set_ugid(kn);
+	if (ret)
+		goto out_remove;
+
+	kref_init(&plr->refcount);
+	kernfs_activate(kn);
+	new_plr = plr;
+	ret = 0;
+	goto out;
+
+out_remove:
+	kernfs_remove(kn);
+out_free:
+	kfree(plr);
+out:
+	mutex_unlock(&rdt_pseudo_lock_mutex);
+	mutex_unlock(&rdtgroup_mutex);
+	return ret;
+}
+
+/*

Quoted 6 lines

+ * rdt_pseudo_lock_rmdir - Remove pseudo-lock region
+ *
+ * LOCKING:
+ * Since the pseudo-locked region can be associated with a RDT domain at
+ * removal we take both rdtgroup_mutex and rdt_pseudo_lock_mutex to protect
+ * the rdt_domain access as well as the pseudo_lock_region access.

Is there a real reason / benefit for having this second mutex?

Quoted 13 lines · Thomas Gleixner

Quoted 10 lines · Reinette Chatre

System administrator creates/removes pseudo-locked regions by creating/removing directories in the pseudo-lock subdirectory of the resctrl filesystem. Here we add directory creation and removal support.

A "pseudo-lock region" is introduced, which represents an instance of a pseudo-locked cache region. During mkdir a new region is created but since we do not know which cache it belongs to at that time we maintain a global pointer to it from where it will be moved to the cache (rdt_domain) it belongs to after initialization. This implies that we only support one uninitialized pseudo-locked region at a time.

Quoted 2 lines

Whats the reason for this restriction? If there are uninitialized directories, so what?

I was thinking about a problematic scenario where an application attempts to create infinite directories. All of these uninitialized directories need to be kept track of before they are initialized as pseudo-locked regions. It seemed simpler to require that one pseudo-locked region is set up at a time.

Quoted 8 lines

Quoted 7 lines

+/*
+ * rdt_pseudo_lock_rmdir - Remove pseudo-lock region
+ *
+ * LOCKING:
+ * Since the pseudo-locked region can be associated with a RDT domain at
+ * removal we take both rdtgroup_mutex and rdt_pseudo_lock_mutex to protect
+ * the rdt_domain access as well as the pseudo_lock_region access.

Quoted 1 line

Is there a real reason / benefit for having this second mutex?

Some interactions with the pseudo-locked region are currently done without the need for the rdtgroup_mutex. For example, interaction with the character device associated with the pseudo-locked region (the mmap() call) as well as the debugfs operations.

Quoted 14 lines · Reinette Chatre

Quoted 13 lines · Thomas Gleixner

Quoted 10 lines · Reinette Chatre

System administrator creates/removes pseudo-locked regions by creating/removing directories in the pseudo-lock subdirectory of the resctrl filesystem. Here we add directory creation and removal support.

A "pseudo-lock region" is introduced, which represents an instance of a pseudo-locked cache region. During mkdir a new region is created but since we do not know which cache it belongs to at that time we maintain a global pointer to it from where it will be moved to the cache (rdt_domain) it belongs to after initialization. This implies that we only support one uninitialized pseudo-locked region at a time.

Quoted 8 lines

Quoted 2 lines

Whats the reason for this restriction? If there are uninitialized directories, so what?

I was thinking about a problematic scenario where an application attempts to create infinite directories. All of these uninitialized directories need to be kept track of before they are initialized as pseudo-locked regions. It seemed simpler to require that one pseudo-locked region is set up at a time.

If the application is allowed to create directories then it can also create a dozen unused resource control groups. This is not a Joe User operation so there is no problem.

Quoted 10 lines

Quoted 9 lines

Quoted 7 lines

+/*
+ * rdt_pseudo_lock_rmdir - Remove pseudo-lock region
+ *
+ * LOCKING:
+ * Since the pseudo-locked region can be associated with a RDT domain at
+ * removal we take both rdtgroup_mutex and rdt_pseudo_lock_mutex to protect
+ * the rdt_domain access as well as the pseudo_lock_region access.

Is there a real reason / benefit for having this second mutex?

Quoted 4 lines

Some interactions with the pseudo-locked region are currently done without the need for the rdtgroup_mutex. For example, interaction with the character device associated with the pseudo-locked region (the mmap() call) as well as the debugfs operations.

Well, yes. But none of those operations are hot path so having the double locking in lots of the other function is just extra complexity for no real value.

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +9 -0

index 7a22e367b82f..94bd1b4fbfee 100644
@@ -40,6 +40,7 @@ static DEFINE_MUTEX(rdt_pseudo_lock_mutex);
  * @kn:			kernfs node representing this region in the resctrl
  *			filesystem
  * @cbm:		bitmask of the pseudo-locked region
+ * @size:		size of pseudo-locked region in bytes
  * @cpu:		core associated with the cache on which the setup code
  *			will be run
  * @minor:		minor number of character device associated with this

@@ -52,6 +53,7 @@ static DEFINE_MUTEX(rdt_pseudo_lock_mutex);
 struct pseudo_lock_region {
 	struct kernfs_node	*kn;
 	u32			cbm;
+	unsigned int		size;
 	int			cpu;
 	unsigned int		minor;
 	bool			locked;

@@ -227,6 +229,49 @@ static struct kernfs_ops pseudo_lock_avail_ops = {
 	.seq_show		= pseudo_lock_avail_show,
 };
 
+int pseudo_lock_schemata_show(struct kernfs_open_file *of,
+			      struct seq_file *seq, void *v)
+{
+	struct pseudo_lock_region *plr;
+	struct rdt_resource *r;
+	int ret = 0;
+

After a pseudo-locked region is locked it needs to be associated with the RDT domain representing the pseudo-locked cache so that its life cycle can be managed correctly.

Only a single pseudo-locked region can exist on any cache instance so we maintain a single pointer to a pseudo-locked region from each RDT domain.

Signed-off-by Reinette Chatre <[email protected]>

---
 arch/x86/kernel/cpu/intel_rdt.h | 4 ++++
 1 file changed, 4 insertions(+)

arch/x86/kernel/cpu/intel_rdt.h +3 -0

index 060a0976ac00..f0e020686e99 100644
@@ -187,6 +187,8 @@ struct mbm_state {
 	u64	prev_msr;
 };
 
+struct pseudo_lock_region;
+
 /**
  * struct rdt_domain - group of cpus sharing an RDT resource
  * @list:	all instances of this resource

@@ -205,6 +207,7 @@ struct mbm_state {
  * @ctrl_val:	array of cache or mem ctrl values (indexed by CLOSID)
  * @new_ctrl:	new ctrl value to be loaded
  * @have_new_ctrl: did user provide new_ctrl for this domain
+ * @plr:	pseudo-locked region associated with this domain
  */
 struct rdt_domain {
 	struct list_head	list;

Quoted 7 lines · Reinette Chatre

After a pseudo-locked region is locked it needs to be associated with the RDT domain representing the pseudo-locked cache so that its life cycle can be managed correctly.

Only a single pseudo-locked region can exist on any cache instance so we maintain a single pointer to a pseudo-locked region from each RDT domain.

Why is only a single pseudo locked region possible?

Quoted 26 lines

Signed-off-by Reinette Chatre <[email protected]>

---
 arch/x86/kernel/cpu/intel_rdt.h | 4 ++++
 1 file changed, 4 insertions(+)

arch/x86/kernel/cpu/intel_rdt.h +3 -0

index 060a0976ac00..f0e020686e99 100644
@@ -187,6 +187,8 @@ struct mbm_state {
 	u64	prev_msr;
 };
 
+struct pseudo_lock_region;
+
 /**
  * struct rdt_domain - group of cpus sharing an RDT resource
  * @list:	all instances of this resource

@@ -205,6 +207,7 @@ struct mbm_state {
  * @ctrl_val:	array of cache or mem ctrl values (indexed by CLOSID)
  * @new_ctrl:	new ctrl value to be loaded
  * @have_new_ctrl: did user provide new_ctrl for this domain
+ * @plr:	pseudo-locked region associated with this domain
  */
 struct rdt_domain {

arch/x86/kernel/cpu/intel_rdt.h struct rdt_domain { · line 226

Quoted 6 lines

arch/x86/kernel/cpu/intel_rdt.h +1 -0

 	struct list_head	list;

@@ -220,6 +223,7 @@ struct rdt_domain {
 	u32			*ctrl_val;
 	u32			new_ctrl;
 	bool			have_new_ctrl;
+	struct pseudo_lock_region	*plr;

Please keep the tabular fashion of the struct declaration intact.

Hi Thomas,

Quoted 10 lines · Thomas Gleixner

Quoted 7 lines · Reinette Chatre

After a pseudo-locked region is locked it needs to be associated with the RDT domain representing the pseudo-locked cache so that its life cycle can be managed correctly.

Only a single pseudo-locked region can exist on any cache instance so we maintain a single pointer to a pseudo-locked region from each RDT domain.

Quoted 1 line

Why is only a single pseudo locked region possible?

The setup of a pseudo-locked region requires the usage of wbinvd. If a second pseudo-locked region is thus attempted it will evict the pseudo-locked data of the first.

Quoted 11 lines · Reinette Chatre

Quoted 10 lines · Thomas Gleixner

Quoted 7 lines · Reinette Chatre

After a pseudo-locked region is locked it needs to be associated with the RDT domain representing the pseudo-locked cache so that its life cycle can be managed correctly.

Only a single pseudo-locked region can exist on any cache instance so we maintain a single pointer to a pseudo-locked region from each RDT domain.

Quoted 5 lines

Quoted 1 line

Why is only a single pseudo locked region possible?

The setup of a pseudo-locked region requires the usage of wbinvd. If a second pseudo-locked region is thus attempted it will evict the pseudo-locked data of the first.

Why does it neeed wbinvd? wbinvd is a big hammer. What's wrong with clflush?

Quoted 12 lines · Thomas Gleixner

Quoted 11 lines · Reinette Chatre

Quoted 10 lines · Thomas Gleixner

Quoted 7 lines · Reinette Chatre

After a pseudo-locked region is locked it needs to be associated with the RDT domain representing the pseudo-locked cache so that its life cycle can be managed correctly.

Only a single pseudo-locked region can exist on any cache instance so we maintain a single pointer to a pseudo-locked region from each RDT domain.

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

Why is only a single pseudo locked region possible?

The setup of a pseudo-locked region requires the usage of wbinvd. If a second pseudo-locked region is thus attempted it will evict the pseudo-locked data of the first.

Why does it neeed wbinvd? wbinvd is a big hammer. What's wrong with clflush?

wbinvd is required by this hardware supported feature but limited to the creation of the pseudo-locked region. An administrator could dedicate a portion of cache to pseudo-locking and applications using this region can come and go. The pseudo-locked region lifetime need not be tied to application lifetime. The pseudo-locked region could be set up once on boot and remain for lifetime of system.

Even so, understanding that it is a big hammer I did explore the alternatives. Trying clflush, clflushopt, as well as clwb. Finding them all to perform poorly(*) I went further to explore if it is possible to use these other instructions with some additional work in support to make them perform as well as wbinvd. The additional work included, looping over the data more times than done for wbinvd, reducing the size of memory locked in relationship to cache size, unused spacing between pseudo-locked region and other regions, unmapped memory at end of pseudo-locked region.

In addition to the above research from my side I also followed up with the CPU architects directly to question the usage of these instructions instead of wbinvd.

Quoted 38 lines · Reinette Chatre

Quoted 12 lines · Thomas Gleixner

Quoted 11 lines · Reinette Chatre

Quoted 10 lines · Thomas Gleixner

Quoted 7 lines · Reinette Chatre

After a pseudo-locked region is locked it needs to be associated with the RDT domain representing the pseudo-locked cache so that its life cycle can be managed correctly.

Only a single pseudo-locked region can exist on any cache instance so we maintain a single pointer to a pseudo-locked region from each RDT domain.

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

Why is only a single pseudo locked region possible?

The setup of a pseudo-locked region requires the usage of wbinvd. If a second pseudo-locked region is thus attempted it will evict the pseudo-locked data of the first.

Why does it neeed wbinvd? wbinvd is a big hammer. What's wrong with clflush?

wbinvd is required by this hardware supported feature but limited to the creation of the pseudo-locked region. An administrator could dedicate a portion of cache to pseudo-locking and applications using this region can come and go. The pseudo-locked region lifetime need not be tied to application lifetime. The pseudo-locked region could be set up once on boot and remain for lifetime of system.

Even so, understanding that it is a big hammer I did explore the alternatives. Trying clflush, clflushopt, as well as clwb. Finding them all to perform poorly(*) I went further to explore if it is possible to use these other instructions with some additional work in support to make them perform as well as wbinvd. The additional work included, looping over the data more times than done for wbinvd, reducing the size of memory locked in relationship to cache size, unused spacing between pseudo-locked region and other regions, unmapped memory at end of pseudo-locked region.

Quoted 3 lines

In addition to the above research from my side I also followed up with the CPU architects directly to question the usage of these instructions instead of wbinvd.

What was their answer? This really wants a proper explanation and not just experimentation results as it makes absolutely no sense at all.

Quoted 5 lines · Thomas Gleixner

Quoted 3 lines · Reinette Chatre

In addition to the above research from my side I also followed up with the CPU architects directly to question the usage of these instructions instead of wbinvd.

Quoted 2 lines

What was their answer? This really wants a proper explanation and not just experimentation results as it makes absolutely no sense at all.

I always prefer to provide detailed answers but here I find myself at the threshold where I may end up sharing information not publicly known. This cannot be the first time you find yourself in this situation. How do you prefer to proceed?

static void __pseudo_lock_region_release(struct pseudo_lock_region *plr) {

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +10 -0

 	bool is_new_plr = (plr == new_plr);

@@ -93,6 +175,23 @@ static void __pseudo_lock_region_release(struct pseudo_lock_region *plr)
 	if (!plr->deleted)
 		return;
 
+	if (plr->locked) {
+		plr->d->plr = NULL;
+		/*
+		 * Resource groups come and go. Simply returning this
+		 * pseudo-locked region's bits to the default CLOS may
+		 * result in default CLOS to become fragmented, causing
+		 * the setting of its bitmask to fail. Ensure it is valid
+		 * first. If this check does fail we cannot return the bits
+		 * to the default CLOS and userspace intervention would be
+		 * required to ensure portions of the cache do not go

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +21 -0

+/**
+ * pseudo_lock_fn - Load kernel memory into cache
+ *
+ * This is the core pseudo-locking function.
+ *
+ * First we ensure that the kernel memory cannot be found in the cache.
+ * Then, while taking care that there will be as little interference as
+ * possible, each cache line of the memory to be loaded is touched while
+ * core is running with class of service set to the bitmask of the
+ * pseudo-locked region. After this is complete no future CAT allocations
+ * will be allowed to overlap with this bitmask.
+ *
+ * Local register variables are utilized to ensure that the memory region
+ * to be locked is the only memory access made during the critical locking
+ * loop.
+ */
+static int pseudo_lock_fn(void *_plr)
+{
+	struct pseudo_lock_region *plr = _plr;
+	u32 rmid_p, closid_p;
+	unsigned long flags;

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +10 -0

+	unsigned int line_size;
+	unsigned int size;
+	void *mem_r;
+#else
+	register unsigned int line_size asm("esi");
+	register unsigned int size asm("edi");
+#ifdef CONFIG_X86_64
+	register void *mem_r asm("rbx");
+#else
+	register void *mem_r asm("ebx");

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +19 -0

+	}
+
+	ret = pseudo_lock_region_init(plr, r, d);
+	if (ret < 0)
+		return ret;
+
+	closid = closid_alloc();
+	if (closid < 0) {
+		ret = closid;
+		rdt_last_cmd_puts("unable to obtain free closid\n");
+		goto out_region;
+	}
+
+	/*
+	 * Ensure we end with a valid default CLOS. If a pseudo-locked
+	 * region in middle of possible bitmasks is selected it will split
+	 * up default CLOS which would be a fault and for which handling
+	 * is unclear so we fail back to userspace. Validation will also
+	 * ensure that default CLOS is not zero, keeping some cache

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +17 -0

+	if (ret < 0) {
+		rdt_last_cmd_printf("unable to set clos 0 bitmask to 0x%x\n",
+				    d->ctrl_val[0] & ~plr->cbm);
+		goto out_closid;
+	}
+
+	ret = pseudo_lock_clos_set(plr, closid, plr->cbm);
+	if (ret < 0) {
+		rdt_last_cmd_printf("unable to set closid %d bitmask to 0x%x\n",
+				    closid, plr->cbm);
+		goto out_clos_def;
+	}
+
+	plr->closid = closid;
+
+	thread_done = 0;
+

Quoted 23 lines · Reinette Chatre

static void __pseudo_lock_region_release(struct pseudo_lock_region *plr) {

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +16 -0

 	bool is_new_plr = (plr == new_plr);

@@ -93,6 +175,23 @@ static void __pseudo_lock_region_release(struct pseudo_lock_region *plr)
 	if (!plr->deleted)
 		return;
 
+	if (plr->locked) {
+		plr->d->plr = NULL;
+		/*
+		 * Resource groups come and go. Simply returning this
+		 * pseudo-locked region's bits to the default CLOS may
+		 * result in default CLOS to become fragmented, causing
+		 * the setting of its bitmask to fail. Ensure it is valid
+		 * first. If this check does fail we cannot return the bits
+		 * to the default CLOS and userspace intervention would be
+		 * required to ensure portions of the cache do not go
+		 * unused.
+		 */
+		if (cbm_validate_val(plr->d->ctrl_val[0] | plr->cbm, plr->r))
+			pseudo_lock_clos_set(plr, 0,
+					     plr->d->ctrl_val[0] | plr->cbm);
+		pseudo_lock_region_clear(plr);

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c static void __pseudo_lock_region_release(struct pseudo_lock_region *plr) · line 197

Quoted 4 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +1 -0

+	}
 	kfree(plr);
 	if (is_new_plr)
 		new_plr = NULL;

Are you really sure that the life time rules of plr are correct vs. an application which still has the locked memory mapped? i.e. the following operation:

1# create_pseudo_lock_region()

					2# start_app()
					   fd = open(/dev/.../lock);
					   ptr = mmap(fd, .....); <- takes a ref on fd
					   close(fd);
					   do_stuff(ptr);

1# rmdir .../lock

unmap(ptr); <- releases fd

I can't see how that is protected. You already have a kref in the PLR, but it's in no way connected to the file descriptor lifetime. So the refcount logic here must be:

create_lock_region()

	plr = alloc_plr();
	take_ref(plr);
	if (!init_plr(plr)) {
		drop_ref(plr);
		...
	}

lockdev_open(filp)

	take_ref(plr);
	filp->private = plr;

rmdir_lock_region()

	...
	drop_ref(plr);

lockdev_relese(filp)

	filp->private = NULL;
	drop_ref(plr);

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c static u64 prefetch_disable_bits; · line 63

Quoted 13 lines

/*
+ * Only one pseudo-locked region can be set up at a time and that is
+ * enforced by taking the rdt_pseudo_lock_mutex when the user writes the
+ * requested schemata to the resctrl file and releasing the mutex on
+ * completion. The thread locking the kernel memory into the cache starts
+ * and completes during this time so we can be sure that only one thread
+ * can run at any time.
+ * The functions starting the pseudo-locking thread needs to wait for its
+ * completion and since there can only be one we have a global workqueue
+ * and variable to support this.
+ */

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +2 -0

+static DECLARE_WAIT_QUEUE_HEAD(wq);
+static int thread_done;

Eew. For one, you really couldn't come up with more generic and less relatable variable names, right?

That aside, its just wrong to build code based on current hardware limitations. The waitqueue and the result code belong into PLR.

Quoted 22 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +21 -0

+/**
+ * pseudo_lock_fn - Load kernel memory into cache
+ *
+ * This is the core pseudo-locking function.
+ *
+ * First we ensure that the kernel memory cannot be found in the cache.
+ * Then, while taking care that there will be as little interference as
+ * possible, each cache line of the memory to be loaded is touched while
+ * core is running with class of service set to the bitmask of the
+ * pseudo-locked region. After this is complete no future CAT allocations
+ * will be allowed to overlap with this bitmask.
+ *
+ * Local register variables are utilized to ensure that the memory region
+ * to be locked is the only memory access made during the critical locking
+ * loop.
+ */
+static int pseudo_lock_fn(void *_plr)
+{
+	struct pseudo_lock_region *plr = _plr;
+	u32 rmid_p, closid_p;
+	unsigned long flags;

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c int pseudo_lock_schemata_show(struct kernfs_open_file *of, · line 525

Quoted 10 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +10 -0

+	u64 i;
+#ifdef CONFIG_KASAN
+	/*
+	 * The registers used for local register variables are also used
+	 * when KASAN is active. When KASAN is active we use a regular
+	 * variable to ensure we always use a valid pointer, but the cost
+	 * is that this variable will enter the cache through evicting the
+	 * memory we are trying to lock into the cache. Thus expect lower
+	 * pseudo-locking success rate when KASAN is active.
+	 */

I'm not a real fan of this mess. But well,

Quoted 10 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +10 -0

+	unsigned int line_size;
+	unsigned int size;
+	void *mem_r;
+#else
+	register unsigned int line_size asm("esi");
+	register unsigned int size asm("edi");
+#ifdef CONFIG_X86_64
+	register void *mem_r asm("rbx");
+#else
+	register void *mem_r asm("ebx");

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c int pseudo_lock_schemata_show(struct kernfs_open_file *of, · line 545

Quoted 10 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +10 -0

+#endif /* CONFIG_X86_64 */
+#endif /* CONFIG_KASAN */
+
+	/*
+	 * Make sure none of the allocated memory is cached. If it is we
+	 * will get a cache hit in below loop from outside of pseudo-locked
+	 * region.
+	 * wbinvd (as opposed to clflush/clflushopt) is required to
+	 * increase likelihood that allocated cache portion will be filled
+	 * with associated memory

Sigh.

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c int pseudo_lock_schemata_show(struct kernfs_open_file *of, · line 550

Quoted 5 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +5 -0

+	 */
+	wbinvd();
+
+	preempt_disable();
+	local_irq_save(flags);

preempt_disable() is pointless when you disable interrupts. And this really should be local_irq_disable(). This code is always called with interrupts enabled....

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c int pseudo_lock_schemata_show(struct kernfs_open_file *of, · line 554

Quoted 4 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +4 -0

+	/*
+	 * Call wrmsr and rdmsr as directly as possible to avoid tracing
+	 * clobbering local register variables or affecting cache accesses.
+	 */

You probably want to make sure that the code below is in L1 cache already before the CLOSID is set to the allocation. To do this you want to put the preload mechanics into a separate ASM function.

Then you run it with size = 1 on some other temporary memory buffer with the default CLOSID, which has the CBM bits of the lock region excluded, Then switch to the real CLOSID and run the loop with the real buffer and the real size.

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c int pseudo_lock_schemata_show(struct kernfs_open_file *of, · line 555

Quoted 1 line

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +1 -0

+	__wrmsr(MSR_MISC_FEATURE_CONTROL, prefetch_disable_bits, 0x0);

This wants an explanation why the prefetcher needs to be disabled.

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c int pseudo_lock_schemata_show(struct kernfs_open_file *of, · line 603

Quoted 11 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +11 -0

+static int pseudo_lock_doit(struct pseudo_lock_region *plr,
+			    struct rdt_resource *r,
+			    struct rdt_domain *d)
+{
+	struct task_struct *thread;
+	int closid;
+	int ret, i;
+
+	/*
+	 * With the usage of wbinvd we can only support one pseudo-locked
+	 * region per domain at this time.

This really sucks.

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c int pseudo_lock_schemata_show(struct kernfs_open_file *of, · line 607

Quoted 4 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +4 -0

+	 */
+	if (d->plr) {
+		rdt_last_cmd_puts("pseudo-locked region exists on cache\n");
+		return -ENOSPC;

This check is not sufficient for a CPU which has both L2 and L3 allocation capability. If there is already a L3 locked region and the current call sets up a L2 locked region then this will not catch it and the following wbinvd will wipe the L3 locked region ....

Quoted 25 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +25 -0

+	}
+
+	ret = pseudo_lock_region_init(plr, r, d);
+	if (ret < 0)
+		return ret;
+
+	closid = closid_alloc();
+	if (closid < 0) {
+		ret = closid;
+		rdt_last_cmd_puts("unable to obtain free closid\n");
+		goto out_region;
+	}
+
+	/*
+	 * Ensure we end with a valid default CLOS. If a pseudo-locked
+	 * region in middle of possible bitmasks is selected it will split
+	 * up default CLOS which would be a fault and for which handling
+	 * is unclear so we fail back to userspace. Validation will also
+	 * ensure that default CLOS is not zero, keeping some cache
+	 * available to rest of system.
+	 */
+	if (!cbm_validate_val(d->ctrl_val[0] & ~plr->cbm, r)) {
+		ret = -EINVAL;
+		rdt_last_cmd_printf("bm 0x%x causes invalid clos 0 bm 0x%x\n",
+				    plr->cbm, d->ctrl_val[0] & ~plr->cbm);

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c int pseudo_lock_schemata_show(struct kernfs_open_file *of, · line 636

Quoted 4 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +4 -0

+		goto out_closid;
+	}
+
+	ret = pseudo_lock_clos_set(plr, 0, d->ctrl_val[0] & ~plr->cbm);

Fiddling with the default CBM is wrong. The lock operation should only succeed when the bits in that domain are not used by _ANY_ control group including the default one. This is a reasonable constraint.

Quoted 17 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +17 -0

+	if (ret < 0) {
+		rdt_last_cmd_printf("unable to set clos 0 bitmask to 0x%x\n",
+				    d->ctrl_val[0] & ~plr->cbm);
+		goto out_closid;
+	}
+
+	ret = pseudo_lock_clos_set(plr, closid, plr->cbm);
+	if (ret < 0) {
+		rdt_last_cmd_printf("unable to set closid %d bitmask to 0x%x\n",
+				    closid, plr->cbm);
+		goto out_clos_def;
+	}
+
+	plr->closid = closid;
+
+	thread_done = 0;
+

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c int pseudo_lock_schemata_show(struct kernfs_open_file *of, · line 663

Quoted 10 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +10 -0

+	thread = kthread_create_on_node(pseudo_lock_fn, plr,
+					cpu_to_node(plr->cpu),
+					"pseudo_lock/%u", plr->cpu);
+	if (IS_ERR(thread)) {
+		ret = PTR_ERR(thread);
+		rdt_last_cmd_printf("locking thread returned error %d\n", ret);
+		/*
+		 * We do not return CBM to newly allocated CLOS here on
+		 * error path since that will result in a CBM of all
+		 * zeroes which is an illegal MSR write.

I'm not sure what you are trying to explain here.

If you remove a ctrl group then the CBM bits are not added to anything either. It's up to the operator to handle that. Why would this be any different for the pseudo-locking stuff?

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c int pseudo_lock_schemata_show(struct kernfs_open_file *of, · line 674

Quoted 11 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +11 -0

+		 */
+		goto out_clos_def;
+	}
+
+	kthread_bind(thread, plr->cpu);
+	wake_up_process(thread);
+
+	ret = wait_event_interruptible(wq, thread_done == 1);
+	if (ret < 0) {
+		rdt_last_cmd_puts("locking thread interrupted\n");
+		goto out_clos_def;

This is broken. If the thread does not get on the CPU for whatever reason and the process which sets up the region is interrupted then this will leave the thread in runnable state and once it gets on the CPU it will happily derefence the freed plr struct and fiddle with the freed memory.

You need to make sure that the thread holds a reference on the plr struct, which prevents freeing. That includes the CLOSID .....

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c int pseudo_lock_schemata_show(struct kernfs_open_file *of, · line 686

Quoted 13 lines

arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c +12 -0

+	}
+
+	/*
+	 * closid will be released soon but its CBM as well as CBM of not
+	 * yet allocated CLOS as stored in the array will remain. Ensure
+	 * that CBM will be what is currently the default CLOS, which
+	 * excludes pseudo-locked region.
+	 */
+	for (i = 1; i < r->num_closid; i++) {
+		if (i == closid || !closid_allocated(i))
+			pseudo_lock_clos_set(plr, i, d->ctrl_val[0]);
+	}

This is all magical duct tape. The overall design of this is sideways and not really well integrated into the existing infrastructure which creates these kinds of magic warts and lots of duplicated code.

The deeper I read into the patch series the less I like that interface and the implementation.

Let's look at the existing crtl/mon groups which are each represented by a directory already.

- Adding a 'size' file to the ctrl groups would be a natural extension which makes sense for regular cache allocations as well.

- Adding a 'exclusive' flag would be an interesting feature even for the normal use case. Marking a group as exclusive prevents other groups to request CBM bits which are held by a exclusive allocation.

I'd suggest to have a file 'mode' for controlling this. The valid values would be something like 'shareable' and 'exclusive'.

When trying to set a group to exclusive mode then the schemata has to be checked for overlaps with the other schematas and in case of conflict the write fails. Once enabled subsequent writes to the schemata file need to be checked for conflicts as well.

If the exclusive setting is enabled then the CBM bits of that group are excluded from being used in other control groups.

Aside of that a file in the info directory which shows the (un)used CBM bits of all groups is really helpful for controlling all of that (even w/o pseudo locking). You have this in the 'avail' file, but there is no reason why this should only be available for pseudo locking enabled systems.

Now for the pseudo locking part.

What you need on top of the above is a new 'mode': 'locked'. That mode utilizes the 'exclusive' mode rules vs. conflict checking and the protection against allocating the associated CBM bits in other control groups.

The setup would be like this:

mkdir group
echo '$CONFIG' >group/schemata
echo 'locked' >group/mode

Setting mode to locked locks down the schemata file along with the task/cpus/cpus_list files. The task/cpu files need to be empty when entering locked mode, otherwise the operation fails. I'd even would not bother handing back the CLOSID. For simplicity the CLOSID should just stay associated with the control group until it is destroyed as any other control group.

Now the remaining thing is the memory allocation and the mmap itself. I really dislike the preallocation of memory right at setup time. Ideally that should be an allocation of the application itself, but the horrid wbinvd stuff kinda prevents that. With that restriction we are more or less bound to immediate allocation and population.

Quoted 5 lines · Thomas Gleixner

On Tue, 13 Feb 2018, Reinette Chatre wrote:

Are you really sure that the life time rules of plr are correct vs. an application which still has the locked memory mapped? i.e. the following operation:

You are correct. I am not preventing an administrator from removing the pseudo-locked region if it is in use. I will fix that.

Thank you so much for taking the time to do this thorough review and to make these suggestions. While I am still digesting the details I do intend to follow all (as well as the ones earlier I did not explicitly respond to).

Keeping the CLOSID associated with the pseudo-locked region will surely make the above simpler since CLOSID's are association with resource groups (represented by the directories). I would like to highlight that on some platforms there are only a few (for example, 4) CLOSIDs available. Not releasing a CLOSID would thus reduce available CLOSIDs that are already limited. These platforms do have smaller possible bitmasks though (for example, 8 possible bits), which may make light of this concern. I thus just add it as informational to the consequence of this simplification.

Quoted 5 lines

Now the remaining thing is the memory allocation and the mmap itself. I really dislike the preallocation of memory right at setup time. Ideally that should be an allocation of the application itself, but the horrid wbinvd stuff kinda prevents that. With that restriction we are more or less bound to immediate allocation and population.

Acknowledged. I am not sure if the current permissions would support such a dynamic setup though. At this time the system administrator is the one that sets up the pseudo-locked region and can through permissions of the character device provide access to these regions to user space applications.

Quoted 9 lines · Reinette Chatre

Quoted 7 lines · Thomas Gleixner

On Tue, 13 Feb 2018, Reinette Chatre wrote:

Now the remaining thing is the memory allocation and the mmap itself. I really dislike the preallocation of memory right at setup time. Ideally that should be an allocation of the application itself, but the horrid wbinvd stuff kinda prevents that. With that restriction we are more or less bound to immediate allocation and population.

Quoted 5 lines

Acknowledged. I am not sure if the current permissions would support such a dynamic setup though. At this time the system administrator is the one that sets up the pseudo-locked region and can through permissions of the character device provide access to these regions to user space applications.

You still would need some interface, e.g. character device which allows you to hand in the pointer to the user allocated memory and do the cache priming. So you could use the same permission setup for that character device.

The other problem is that we'd need to have MAP_CONTIG first so you actually can allocate physically contigous memory from user space. Mike is working on that, but it's not available today. The only way to do so today (with lots of waste) would be MAP_HUGETLB, which might be an acceptable constraint up to the point where MAP_CONTIG is available.

Quoted 21 lines · Thomas Gleixner

Quoted 9 lines · Reinette Chatre

Quoted 7 lines · Thomas Gleixner

On Tue, 13 Feb 2018, Reinette Chatre wrote:

Now the remaining thing is the memory allocation and the mmap itself. I really dislike the preallocation of memory right at setup time. Ideally that should be an allocation of the application itself, but the horrid wbinvd stuff kinda prevents that. With that restriction we are more or less bound to immediate allocation and population.

Quoted 5 lines

Acknowledged. I am not sure if the current permissions would support such a dynamic setup though. At this time the system administrator is the one that sets up the pseudo-locked region and can through permissions of the character device provide access to these regions to user space applications.

You still would need some interface, e.g. character device which allows you to hand in the pointer to the user allocated memory and do the cache priming. So you could use the same permission setup for that character device.

Quoted 5 lines

The other problem is that we'd need to have MAP_CONTIG first so you actually can allocate physically contigous memory from user space. Mike is working on that, but it's not available today. The only way to do so today (with lots of waste) would be MAP_HUGETLB, which might be an acceptable constraint up to the point where MAP_CONTIG is available.

Just to clarify, there is not any activity on exposing a general purpose MAP_CONTIG interface to user space. When initially proposed, MAP_CONTIG was shot down and the suggestion was to create a new in kernel interface to make allocation of contiguous pages easier. The initial use case was a driver which could use the new internal interface as part of it's mmap() routine to give contiguous regions to user space.

Reinette is using this new interface, but that must be for the ?immediate allocation? case you are trying to move away from. Sorry, I have not been following development of this feature.

If you would have to create a device to accept a user buffer, could you perhaps use the same device to create/hand out a contiguous mapping?

Quoted 5 lines · Thomas Gleixner

The other problem is that we'd need to have MAP_CONTIG first so you actually can allocate physically contigous memory from user space. Mike is working on that, but it's not available today. The only way to do so today (with lots of waste) would be MAP_HUGETLB, which might be an acceptable constraint up to the point where MAP_CONTIG is available.

Just to clarify, there is not any activity on exposing a general purpose MAP_CONTIG interface to user space. When initially proposed, MAP_CONTIG was shot down and the suggestion was to create a new in kernel interface to make allocation of contiguous pages easier. The initial use case was a driver which could use the new internal interface as part of it's mmap() routine to give contiguous regions to user space.

Reinette is using this new interface, but that must be for the ?immediate allocation? case you are trying to move away from. Sorry, I have not been following development of this feature.

If you would have to create a device to accept a user buffer, could you perhaps use the same device to create/hand out a contiguous mapping?

Quoted 7 lines · Reinette Chatre

Quoted 5 lines · Thomas Gleixner

On Tue, 13 Feb 2018, Reinette Chatre wrote:

Are you really sure that the life time rules of plr are correct vs. an application which still has the locked memory mapped? i.e. the following operation:

Quoted 2 lines

You are correct. I am not preventing an administrator from removing the pseudo-locked region if it is in use. I will fix that.

The removal is fine and you cannot prevent it w/o introducing a mess, but you have to make sure that the PLR and the mapped memory are not vanishing. The refcount rules I outlined are exactly doing that.

Quoted 4 lines

Thank you so much for taking the time to do this thorough review and to make these suggestions. While I am still digesting the details I do intend to follow all (as well as the ones earlier I did not explicitly respond to).

Make your mind up and tell me where I'm wrong before you implement the crap I suggested blindly, as that will just cause the next reviewer (me or someone else) to tell _you_ that it is crap :)

Quoted 9 lines

Keeping the CLOSID associated with the pseudo-locked region will surely make the above simpler since CLOSID's are association with resource groups (represented by the directories). I would like to highlight that on some platforms there are only a few (for example, 4) CLOSIDs available. Not releasing a CLOSID would thus reduce available CLOSIDs that are already limited. These platforms do have smaller possible bitmasks though (for example, 8 possible bits), which may make light of this concern. I thus just add it as informational to the consequence of this simplification.

Yes. If you have 4 CLOSIDs and only 8 CBM bits it really does not matter much.

Quoted 11 lines

Quoted 5 lines

Now the remaining thing is the memory allocation and the mmap itself. I really dislike the preallocation of memory right at setup time. Ideally that should be an allocation of the application itself, but the horrid wbinvd stuff kinda prevents that. With that restriction we are more or less bound to immediate allocation and population.

Acknowledged. I am not sure if the current permissions would support such a dynamic setup though. At this time the system administrator is the one that sets up the pseudo-locked region and can through permissions of the character device provide access to these regions to user space applications.

You still would need some interface, e.g. character device which allows you to hand in the pointer to the user allocated memory and do the cache priming. So you could use the same permission setup for that character device.

The other problem is that we'd need to have MAP_CONTIG first so you actually can allocate physically contigous memory from user space. Mike is working on that, but it's not available today. The only way to do so today (with lots of waste) would be MAP_HUGETLB, which might be an acceptable constraint up to the point where MAP_CONTIG is available.

Though this all depends on the ability to remove the wbinvd requirement. But even if we can remove that we'd still need to be aware that the cache priming loop which needs to run with interrupts disabled is expensive as well and can introduce undesired latencies. Needs all some thought...

Let's look at the existing crtl/mon groups which are each represented by a directory already.

- Adding a 'size' file to the ctrl groups would be a natural extension which makes sense for regular cache allocations as well.

- Adding a 'exclusive' flag would be an interesting feature even for the normal use case. Marking a group as exclusive prevents other groups to request CBM bits which are held by a exclusive allocation.

I'd suggest to have a file 'mode' for controlling this. The valid values would be something like 'shareable' and 'exclusive'.

When trying to set a group to exclusive mode then the schemata has to be checked for overlaps with the other schematas and in case of conflict the write fails. Once enabled subsequent writes to the schemata file need to be checked for conflicts as well.

If the exclusive setting is enabled then the CBM bits of that group are excluded from being used in other control groups.

Aside of that a file in the info directory which shows the (un)used CBM bits of all groups is really helpful for controlling all of that (even w/o pseudo locking). You have this in the 'avail' file, but there is no reason why this should only be available for pseudo locking enabled systems.

Now for the pseudo locking part.

What you need on top of the above is a new 'mode': 'locked'. That mode utilizes the 'exclusive' mode rules vs. conflict checking and the protection against allocating the associated CBM bits in other control groups.

The setup would be like this:

mkdir group
echo '$CONFIG' >group/schemata
echo 'locked' >group/mode

Setting mode to locked locks down the schemata file along with the task/cpus/cpus_list files. The task/cpu files need to be empty when entering locked mode, otherwise the operation fails. I'd even would not bother handing back the CLOSID. For simplicity the CLOSID should just stay associated with the control group until it is destroyed as any other control group.

From the above it follows that "locked" mode cannot just simply build on

I started looking at how this implementation may look and would like to confirm with you that your intentions behind the new "exclusive" and "locked" modes can be maintained. I also have a few questions.

Focusing on CAT a resource group represents a closid across all domains (cache instances) of all resources (cache layers) on the system. A full schemata reflecting the active bitmask associated with this closid for each domain of each resource is maintained. The current implementation supports partial writes to the schemata, with the assumption that only the changed values need to be updated, the others remain as is. For the current implementation this works well since what is shown by schemata reflects current hardware settings and what is written to schemata will change current hardware settings. This is done irrespective of any overlap between bitmasks of different closids (the "shareable" mode).

A change to start us off with could be to initialize the schemata with all the shareable and unused bits set for all domains when a new resource group is created.

Moving to "exclusive" mode it appears that, when enabled for a resource group, all domains of all resources are forced to have an "exclusive" region associated with this resource group (closid). This is because the schemata reflects the hardware settings of all resources and their domains and the hardware does not accept a "zero" bitmask. A user thus cannot just specify a single region of a particular cache instance as "exclusive". Does this match your intention wrt "exclusive"?

Moving on to the "locked" mode. We cannot support different pseudo-locked regions across multiple resources (eg. L2 and L3). In fact, if we would at some point in the future then a pseudo-locked region on one resource could implicitly span a second resource. Additionally, we would like to enable a user to enable a single pseudo-locked region on a single cache instance.

From the above it follows that "locked" mode cannot just simply build on top of "exclusive" mode rules (as I expressed them above) since it cannot enforce a locked region on each domain of each resource.

We would like to support something like (as you also have in your example):

mkdir group
echo "L2:1=0x3" > schemata
echo locked > mode

The above should only pseudo-lock the indicated region and not touch any other domain. The problem is that the schemata always contain non-zero bitmasks for all domains so at the time "locked" is written it is not known which cache region needs to be locked. I am currently unable to see a simple way to build on top of the current schemata design to support the "locked" mode as you intended. It does seem as though the user's intention to create a pseudo-locked region needs to be communicated before the schemata is written, but from what I understand this does not seem to be supported by the mode/schemata combination. Please do correct me where I am wrong.

To continue, when we overcome the above obstacle:
A scenario could be where a single resource group will contain all the pseudo-locked regions (to avoid wasting closids). It is not clear to me how to easily support such a usage though since the way writes to the schemata is done is "changes only". If for example, two pseudo-locked regions exists:

# mkdir group
# echo "L2:1=0x3" > schemata
# echo locked > mode
# cat schemata
L2:1=0x3
# echo "L2:0=0xf" > schemata
# cat schemata
L2:0=0xf;1=0x3

How can the user remove one of the pseudo-locked regions without affecting the other? Could we perhaps allow zero bitmask writes when a region is locked?

Another point I would like to highlight is that when we talked about keeping the closid associated with the pseudo-locked region I mentioned that some resources may have few closids (for example, 4). As discussed this seems ok when there are only 8 bits in the bitmask. What I did not highlight at that time is that the closids are limited to the smallest number supported by all resources. So, if this same platform has a second resource (with more bits in a bitmask) with more closids, they would also be limited to 4. In this case it does seem removing a closid from service would have bigger impact.

Quoted 7 lines · Reinette Chatre

Moving to "exclusive" mode it appears that, when enabled for a resource group, all domains of all resources are forced to have an "exclusive" region associated with this resource group (closid). This is because the schemata reflects the hardware settings of all resources and their domains and the hardware does not accept a "zero" bitmask. A user thus cannot just specify a single region of a particular cache instance as "exclusive". Does this match your intention wrt "exclusive"?

Interesting question. I really did not think about that yet.

Quoted 3 lines · Reinette Chatre

A change to start us off with could be to initialize the schemata with all the shareable and unused bits set for all domains when a new resource group is created.

The new resource group initialization is the least of my worries. The current mode is to use the default group setting, right?

Quoted 7 lines

Moving to "exclusive" mode it appears that, when enabled for a resource group, all domains of all resources are forced to have an "exclusive" region associated with this resource group (closid). This is because the schemata reflects the hardware settings of all resources and their domains and the hardware does not accept a "zero" bitmask. A user thus cannot just specify a single region of a particular cache instance as "exclusive". Does this match your intention wrt "exclusive"?

Interesting question. I really did not think about that yet.

Quoted 17 lines

Moving on to the "locked" mode. We cannot support different pseudo-locked regions across multiple resources (eg. L2 and L3). In fact, if we would at some point in the future then a pseudo-locked region on one resource could implicitly span a second resource. Additionally, we would like to enable a user to enable a single pseudo-locked region on a single cache instance.

From the above it follows that "locked" mode cannot just simply build on top of "exclusive" mode rules (as I expressed them above) since it cannot enforce a locked region on each domain of each resource.

We would like to support something like (as you also have in your example):

mkdir group
echo "L2:1=0x3" > schemata
echo locked > mode

Quoted 10 lines

The above should only pseudo-lock the indicated region and not touch any other domain. The problem is that the schemata always contain non-zero bitmasks for all domains so at the time "locked" is written it is not known which cache region needs to be locked. I am currently unable to see a simple way to build on top of the current schemata design to support the "locked" mode as you intended. It does seem as though the user's intention to create a pseudo-locked region needs to be communicated before the schemata is written, but from what I understand this does not seem to be supported by the mode/schemata combination. Please do correct me where I am wrong.

You could make it:

echo locksetup > mode
echo $CONF > schemata
echo locked > mode

Or something like that.

Quoted 16 lines

To continue, when we overcome the above obstacle:
A scenario could be where a single resource group will contain all the pseudo-locked regions (to avoid wasting closids). It is not clear to me how to easily support such a usage though since the way writes to the schemata is done is "changes only". If for example, two pseudo-locked regions exists:

# mkdir group
# echo "L2:1=0x3" > schemata
# echo locked > mode
# cat schemata
L2:1=0x3
# echo "L2:0=0xf" > schemata
# cat schemata
L2:0=0xf;1=0x3

Quoted 3 lines

How can the user remove one of the pseudo-locked regions without affecting the other? Could we perhaps allow zero bitmask writes when a region is locked?

That might work. Though it looks hacky.

Quoted 10 lines

Another point I would like to highlight is that when we talked about keeping the closid associated with the pseudo-locked region I mentioned that some resources may have few closids (for example, 4). As discussed this seems ok when there are only 8 bits in the bitmask. What I did not highlight at that time is that the closids are limited to the smallest number supported by all resources. So, if this same platform has a second resource (with more bits in a bitmask) with more closids, they would also be limited to 4. In this case it does seem removing a closid from service would have bigger impact.

Is that a real issue or just an academic exercise?

Let's assume its real,
so you could do the following:

mkdir group <- acquires closid
echo locksetup > mode <- Creates 'lockarea' file
echo L2:0 > lockarea
echo 'L2:0=0xf' > schemata
echo locked > mode <- locks down all files, does the lock setup
and drops closid

That would solve quite some of the other issues as well. Hmm?

Quoted 10 lines · Reinette Chatre

Quoted 7 lines

Moving to "exclusive" mode it appears that, when enabled for a resource group, all domains of all resources are forced to have an "exclusive" region associated with this resource group (closid). This is because the schemata reflects the hardware settings of all resources and their domains and the hardware does not accept a "zero" bitmask. A user thus cannot just specify a single region of a particular cache instance as "exclusive". Does this match your intention wrt "exclusive"?

Interesting question. I really did not think about that yet.

Actually we could solve that problem similar to the locked one and share most of the functionality:

mkdir group
echo exclusive > mode
echo L3:0 > restrict

and for locked:

mkdir group
echo locksetup > mode
echo L2:0 > restrict
echo 'L2:0=0xf' > schemata
echo locked > mode

The 'restrict' file (feel free to come up with a better name) is only available/writeable in exclusive and locksetup mode. In case of exclusive mode it can contain several domains/resources, but in locked mode its only allowed to contain a single domain/resource.

A write to schemata for exclusive or locksetup mode will apply the exclusiveness restrictions only to the resources/domains selected in the 'restrict' file.

Quoted 11 lines · Thomas Gleixner

Let's assume its real,
so you could do the following:

mkdir group <- acquires closid
echo locksetup > mode <- Creates 'lockarea' file
echo L2:0 > lockarea
echo 'L2:0=0xf' > schemata
echo locked > mode <- locks down all files, does the lock setup
and drops closid

That would solve quite some of the other issues as well. Hmm?

At this time the resource group, represented by a resctrl directory, is tightly associated with the closid. I'll take a closer look at what it will take to separate them.

Could you please elaborate on the purpose of the "lockarea" file? It does seem to duplicate the information in the schemata written in the subsequent line.

If we do go this route then it seems that there would be one pseudo-locked region per resource group, not multiple ones as I had in my examples above.

Quoted 7 lines · Thomas Gleixner

Quoted 3 lines · Reinette Chatre

A change to start us off with could be to initialize the schemata with all the shareable and unused bits set for all domains when a new resource group is created.

The new resource group initialization is the least of my worries. The current mode is to use the default group setting, right?

No. When a new group is created a closid is assigned to it. The schemata it is initialized with is the schemata the previous group with the same closid had. At the beginning, yes, it is the default, but later you get something like this:

# mkdir asd
# cat asd/schemata
L2:0=ff;1=ff
# echo 'L2:0=0xf;1=0xfc' > asd/schemata
# cat asd/schemata
L2:0=0f;1=fc
# rmdir asd
# mkdir qwe
# cat qwe/schemata
L2:0=0f;1=fc

The reason why I suggested this initialization is to have the defaults work on resource group creation. I assume a new resource group would be created with "shareable" mode so its schemata should not overlap with any "exclusive" or "locked". Since the bitmasks used by the previous group with this closid may not be shareable I considered it safer to initialize with "shareable" mode with known shareable/unused bitmasks. A potential issue with this idea is that the creation of a group may now result in the programming of the hardware with settings these defaults.

Quoted 9 lines

Quoted 7 lines

Moving to "exclusive" mode it appears that, when enabled for a resource group, all domains of all resources are forced to have an "exclusive" region associated with this resource group (closid). This is because the schemata reflects the hardware settings of all resources and their domains and the hardware does not accept a "zero" bitmask. A user thus cannot just specify a single region of a particular cache instance as "exclusive". Does this match your intention wrt "exclusive"?

Interesting question. I really did not think about that yet.

Quoted 7 lines

You could make it:

echo locksetup > mode
echo $CONF > schemata
echo locked > mode

Or something like that.

Indeed ... the final command may perhaps not be needed? Since the user expressed intent to create pseudo-locked region by writing "locksetup" the pseudo-locking can be done when the schemata is written. I think it would be simpler to act when the schemata is written since we know exactly at that point which regions should be pseudo-locked. After the schemata is stored the user's choice is just merged with the larger schemata representing all resources/domains. We could set mode to "locked" on success, it can remain as "locksetup" on failure of creating the pseudo-locked region. We could perhaps also consider a name change "locksetup" -> "lockrsv" since after the first pseudo-locked region is created on a domain then all the other domains associated with this class of service need to have some special state since no task will ever run on them with that class of service so we would not want their bits (which will not be zero) to be taken into account when checking for "shareable" or "exclusive".

This could also support multiple pseudo-locked regions. For example:
# #Create first pseudo-locked region

# echo locksetup > mode
# echo L2:0=0xf > schemata
# echo $?
0
# cat mode
locked # will be locksetup on failure
# cat schemata
L2:0=0xf #only show pseudo-locked regions
# #Create second pseudo-locked region
# # Not necessary to write "locksetup" again
# echo L2:1=0xf > schemata #will trigger the pseudo-locking of new region
# echo $?
1 # just for example, this could succeed also
# cat mode
locked
# cat schemata
L2:0=0xf

Schemata shown to user would be only the pseudo-locked region(s), unless there is none, then nothing will be returned.

I'll think about this more, but if we do go the route of releasing closids as suggested below it may change a lot.

This is a real issue. The pros and cons of using a global CLOSID across all resources are documented in the comments preceding:
arch/x86/kernel/cpu/intel_rdt_rdtgroup.c:closid_init()

The issue I mention was foreseen, to quote from there "Our choices on how to configure each resource become progressively more limited as the number of resources grows".

Quoted 10 lines

Let's assume its real,
so you could do the following:

mkdir group <- acquires closid
echo locksetup > mode <- Creates 'lockarea' file
echo L2:0 > lockarea
echo 'L2:0=0xf' > schemata
echo locked > mode <- locks down all files, does the lock setup
and drops closid

Quoted 1 line

That would solve quite some of the other issues as well. Hmm?

At this time the resource group, represented by a resctrl directory, is tightly associated with the closid. I'll take a closer look at what it will take to separate them.

Could you please elaborate on the purpose of the "lockarea" file? It does seem to duplicate the information in the schemata written in the subsequent line.

If we do go this route then it seems that there would be one pseudo-locked region per resource group, not multiple ones as I had in my examples above.

An alternative to the hardware programming on creation of resource group could also be to reset the bitmasks of the closid to be shareable/unused bits at the time the closid is released.

Quoted 15 lines

Actually we could solve that problem similar to the locked one and share most of the functionality:

mkdir group
echo exclusive > mode
echo L3:0 > restrict

and for locked:

mkdir group
echo locksetup > mode
echo L2:0 > restrict
echo 'L2:0=0xf' > schemata
echo locked > mode

Quoted 8 lines

The 'restrict' file (feel free to come up with a better name) is only available/writeable in exclusive and locksetup mode. In case of exclusive mode it can contain several domains/resources, but in locked mode its only allowed to contain a single domain/resource.

A write to schemata for exclusive or locksetup mode will apply the exclusiveness restrictions only to the resources/domains selected in the 'restrict' file.

I think I understand for the exclusive case. Here the introduction of the restrict file helps. I will run through a few examples to ensure I understand it. For the pseudo-locking cases I do have the questions and comments above. Here I likely may be missing something but I'll keep dissecting how this would work to clear up my understanding.

Quoted 10 lines · Reinette Chatre

Quoted 9 lines · Thomas Gleixner

Quoted 7 lines · Reinette Chatre

Moving to "exclusive" mode it appears that, when enabled for a resource group, all domains of all resources are forced to have an "exclusive" region associated with this resource group (closid). This is because the schemata reflects the hardware settings of all resources and their domains and the hardware does not accept a "zero" bitmask. A user thus cannot just specify a single region of a particular cache instance as "exclusive". Does this match your intention wrt "exclusive"?

Quoted 1 line

Quoted 1 line

Interesting question. I really did not think about that yet.

Second thoughts on that: I think for a start we can go the simple route and just say: exclusive covers all cache levels.

Quoted 9 lines

Quoted 8 lines

You could make it:

echo locksetup > mode
echo $CONF > schemata
echo locked > mode

Or something like that.

Quoted 15 lines

Indeed ... the final command may perhaps not be needed? Since the user expressed intent to create pseudo-locked region by writing "locksetup" the pseudo-locking can be done when the schemata is written. I think it would be simpler to act when the schemata is written since we know exactly at that point which regions should be pseudo-locked. After the schemata is stored the user's choice is just merged with the larger schemata representing all resources/domains. We could set mode to "locked" on success, it can remain as "locksetup" on failure of creating the pseudo-locked region. We could perhaps also consider a name change "locksetup" -> "lockrsv" since after the first pseudo-locked region is created on a domain then all the other domains associated with this class of service need to have some special state since no task will ever run on them with that class of service so we would not want their bits (which will not be zero) to be taken into account when checking for "shareable" or "exclusive".

Works for me.

I think dropping the closid makes sense. Once the thing is locked it's done and nothing can be changed anymore, except removal of course. That also gives you a 1:1 mapping between resource group and lockdevice.

Quoted 19 lines

This is a real issue. The pros and cons of using a global CLOSID across all resources are documented in the comments preceding:
arch/x86/kernel/cpu/intel_rdt_rdtgroup.c:closid_init()

The issue I mention was foreseen, to quote from there "Our choices on how to configure each resource become progressively more limited as the number of resources grows".

Quoted 10 lines

Let's assume its real,
so you could do the following:

mkdir group <- acquires closid
echo locksetup > mode <- Creates 'lockarea' file
echo L2:0 > lockarea
echo 'L2:0=0xf' > schemata
echo locked > mode <- locks down all files, does the lock setup
and drops closid

Quoted 5 lines

Quoted 1 line

That would solve quite some of the other issues as well. Hmm?

At this time the resource group, represented by a resctrl directory, is tightly associated with the closid. I'll take a closer look at what it will take to separate them.

Shouldn't be that hard.

Quoted 3 lines

Could you please elaborate on the purpose of the "lockarea" file? It does seem to duplicate the information in the schemata written in the subsequent line.

No. The lockarea or restrict file (as I named it later, but feel free to come up with something more intuitive) is there to tell which part of the resource zoo should be made exclusive/locked. That makes the whole write to schemata file and validate whether this is really exclusive way simpler.

Quoted 3 lines

If we do go this route then it seems that there would be one pseudo-locked region per resource group, not multiple ones as I had in my examples above.

Correct.

Quoted 3 lines

An alternative to the hardware programming on creation of resource group could also be to reset the bitmasks of the closid to be shareable/unused bits at the time the closid is released.

That does not help because the default/shareable/unused bits can change between release of a CLOSID and reallocation.

Quoted 24 lines

Quoted 15 lines

Actually we could solve that problem similar to the locked one and share most of the functionality:

mkdir group
echo exclusive > mode
echo L3:0 > restrict

and for locked:

mkdir group
echo locksetup > mode
echo L2:0 > restrict
echo 'L2:0=0xf' > schemata
echo locked > mode

Quoted 8 lines

The 'restrict' file (feel free to come up with a better name) is only available/writeable in exclusive and locksetup mode. In case of exclusive mode it can contain several domains/resources, but in locked mode its only allowed to contain a single domain/resource.

A write to schemata for exclusive or locksetup mode will apply the exclusiveness restrictions only to the resources/domains selected in the 'restrict' file.

Quoted 5 lines

I think I understand for the exclusive case. Here the introduction of the restrict file helps. I will run through a few examples to ensure I understand it. For the pseudo-locking cases I do have the questions and comments above. Here I likely may be missing something but I'll keep dissecting how this would work to clear up my understanding.

I came up with this under the assumptions:

1) One locked region per resource group
2) Drop closid after locking

Then the restrict file makes a lot of sense because it would give a clear selection of the possible resource to lock.

Quoted 4 lines · Thomas Gleixner

I came up with this under the assumptions:

1) One locked region per resource group
2) Drop closid after locking

I am also now working under these assumptions ...

Quoted 2 lines

Then the restrict file makes a lot of sense because it would give a clear selection of the possible resource to lock.

... but I am still stuck on why this restrict file is needed at this time. Surely it would be needed if later we add the more flexible exclusive mode, but I do not understand how it helps the locked mode.

Let's look at the existing crtl/mon groups which are each represented by a directory already.

- Adding a 'size' file to the ctrl groups would be a natural extension which makes sense for regular cache allocations as well.

Quoted 6 lines · Thomas Gleixner

Let's look at the existing crtl/mon groups which are each represented by a directory already.

- Adding a 'size' file to the ctrl groups would be a natural extension which makes sense for regular cache allocations as well.

I would like to clarify how you envision the value of "size" computed. A resource group may have several resources associated with it. Some of these resources may indeed overlap, for example, if there is L2 and L3 CAT capable resources on the system. Similarly when CDP is enabled, there would be overlap in bitmasks referring to the same cache locations but treated as different resources. Indeed, there may in the future be some resources that are capable of allocation but not cache specifically that could also be handled within a single resource group.

Summarizing all of these cases with a single "size" associated with the resource group does not seem straightforward to me.

Quoted 17 lines · Reinette Chatre

Quoted 6 lines · Thomas Gleixner

Let's look at the existing crtl/mon groups which are each represented by a directory already.

- Adding a 'size' file to the ctrl groups would be a natural extension which makes sense for regular cache allocations as well.

I would like to clarify how you envision the value of "size" computed. A resource group may have several resources associated with it. Some of these resources may indeed overlap, for example, if there is L2 and L3 CAT capable resources on the system. Similarly when CDP is enabled, there would be overlap in bitmasks referring to the same cache locations but treated as different resources. Indeed, there may in the future be some resources that are capable of allocation but not cache specifically that could also be handled within a single resource group.

Quoted 2 lines

Summarizing all of these cases with a single "size" associated with the resource group does not seem straightforward to me.

We have the schemata file which covers everthing. So the size file inside a resource group should show the sizes for each domain/resource as well.

L2:0=128K;1=256K;
L3:0=1M;1=2M;

L3DATA:0=128K
L3CODE:0=128K

or such. That would be consistent with the schemata file. If there are resources which cannot be expressed in size, like MBA then you simply do not print them.

At the top level you want to show the inuse areas. I'd go for straight bitmap display there:

L2:0=00011100;1=11111111;
L3:0=11001100;1=11111111;

If L3 CDP is enabled then you can show:

L3:0=1DCCDC00;1=DDDD00CC;

where:

0 = unused
1 = overlapping C/D
C = code
D = data

Quoted 42 lines · Thomas Gleixner

Quoted 17 lines · Reinette Chatre

Quoted 6 lines · Thomas Gleixner

Let's look at the existing crtl/mon groups which are each represented by a directory already.

- Adding a 'size' file to the ctrl groups would be a natural extension which makes sense for regular cache allocations as well.

I would like to clarify how you envision the value of "size" computed. A resource group may have several resources associated with it. Some of these resources may indeed overlap, for example, if there is L2 and L3 CAT capable resources on the system. Similarly when CDP is enabled, there would be overlap in bitmasks referring to the same cache locations but treated as different resources. Indeed, there may in the future be some resources that are capable of allocation but not cache specifically that could also be handled within a single resource group.

Quoted 2 lines

Summarizing all of these cases with a single "size" associated with the resource group does not seem straightforward to me.

We have the schemata file which covers everthing. So the size file inside a resource group should show the sizes for each domain/resource as well.

L2:0=128K;1=256K;
L3:0=1M;1=2M;

L3DATA:0=128K
L3CODE:0=128K

or such. That would be consistent with the schemata file. If there are resources which cannot be expressed in size, like MBA then you simply do not print them.

At the top level you want to show the inuse areas. I'd go for straight bitmap display there:

L2:0=00011100;1=11111111;
L3:0=11001100;1=11111111;

If L3 CDP is enabled then you can show:

Quoted 8 lines

L3:0=1DCCDC00;1=DDDD00CC;

where:

0 = unused
1 = overlapping C/D
C = code
D = data

Hit send too early....

For the locked case this would add:

L = locked

On 2/28/2018 9:59 AM, Thomas Gleixner wrote:
I hesitated doing something like this because during the review of this series there was resistance to using sysfs files for multiple values. I will proceed with your suggestion noting that it is tied with schemata file.

I do not think we need these special labels for CDP though. From what I understand when CDP is enabled there will be two new resources in the info directory. For example,

info/L3DATA/
info/L3CODE/

Each would have its own file(s) noting which bits are in use.

At this time I am also not enabling pseudo-locking when CDP is enabled so the locked label is not needed.

There is already the file "shareable_bits" in the info directory associated with each resource. At the moment it only reflects the bits that could be used by other entities on the system. Considering its name and us now introducing the idea of "shareable" I was thinking of adding all "shareable" bits (hardware and software) of this resource to this file. This still leaves the new "inuse_bits" (or perhaps "unused_bits") info file that will communicate the "exclusive" and "locked" bits in addition so what is in "shareable_bits". Between these two files users should have information needed to choose regions for their tasks. These would all use bitmap displays.