[PATCH v7 07/10] fs/resctrl: Add user interface to enable/disable io_alloc feature

Babu Moger posted 10 patches 2 months, 4 weeks ago
There is a newer version of this series
[PATCH v7 07/10] fs/resctrl: Add user interface to enable/disable io_alloc feature
Posted by Babu Moger 2 months, 4 weeks ago
"io_alloc" feature in resctrl enables direct insertion of data from I/O
devices into the cache.

Introduce user interface to enable/disable io_alloc feature.

On AMD systems, io_alloc feature is backed by SDCIAE (L3 Smart Data Cache
Injection Allocation Enforcement). When enabled, SDCIAE directs all SDCI
lines to be placed into the L3 cache partitions specified by the register
corresponding to the highest CLOSID supported by the resource. With CDP
enabled, io_alloc routes I/O traffic using the highest CLOSID assigned to
the instruction cache (L3CODE).

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v7: Separated resctrl_io_alloc_show and bit_usage changes in two separate
    patches.
    Added resctrl_io_alloc_closid_supported() to verify io_alloc CLOSID.
    Initialized the schema for both CDP_DATA and CDP_CODE when CDP is enabled.

v6: Changed the subject to fs/resctrl:

v5: Resolved conflicts due to recent resctrl FS/ARCH code restructure.
    Used rdt_kn_name to get the rdtgroup name instead of accesssing it directly
    while printing group name used by the io_alloc_closid.

    Updated bit_usage to reflect the io_alloc CBM as discussed in the thread:
    https://lore.kernel.org/lkml/3ca0a5dc-ad9c-4767-9011-b79d986e1e8d@intel.com/
    Modified rdt_bit_usage_show() to read io_alloc_cbm in hw_shareable, ensuring
    that bit_usage accurately represents the CBMs.

    Updated the code to modify io_alloc either with L3CODE or L3DATA.
    https://lore.kernel.org/lkml/c00c00ea-a9ac-4c56-961c-dc5bf633476b@intel.com/

v4: Updated the change log.
    Updated the user doc.
    The "io_alloc" interface will report "enabled/disabled/not supported".
    Updated resctrl_io_alloc_closid_get() to verify the max closid availability.
    Updated the documentation for "shareable_bits" and "bit_usage".
    Introduced io_alloc_init() to initialize fflags.
    Printed the group name when io_alloc enablement fails.

    NOTE: io_alloc is about specific CLOS. rdt_bit_usage_show() is not designed
    handle bit_usage for specific CLOS. Its about overall system. So, we cannot
    really tell the user which CLOS is shared across both hardware and software.
    We need to discuss this.

v3: Rewrote the change to make it generic.
    Rewrote the documentation in resctrl.rst to be generic and added
    AMD feature details in the end.
    Added the check to verify if MAX CLOSID availability on the system.
    Added CDP check to make sure io_alloc is configured in CDP_CODE.
    Added resctrl_io_alloc_closid_free() to free the io_alloc CLOSID.
    Added errors in few cases when CLOSID allocation fails.
    Fixes splat reported when info/L3/bit_usage is accesed when io_alloc
    is enabled.
    https://lore.kernel.org/lkml/SJ1PR11MB60837B532254E7B23BC27E84FC052@SJ1PR11MB6083.namprd11.prod.outlook.com/

v2: Renamed the feature to "io_alloc".
    Added generic texts for the feature in commit log and resctrl.rst doc.
    Added resctrl_io_alloc_init_cat() to initialize io_alloc to default
    values when enabled.
    Fixed io_alloc show functinality to display only on L3 resource.
---
 Documentation/filesystems/resctrl.rst |   8 ++
 fs/resctrl/rdtgroup.c                 | 149 +++++++++++++++++++++++++-
 2 files changed, 156 insertions(+), 1 deletion(-)

diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
index 354e6a00fa45..189c1ccb92d6 100644
--- a/Documentation/filesystems/resctrl.rst
+++ b/Documentation/filesystems/resctrl.rst
@@ -157,6 +157,14 @@ related to allocation:
 			"not supported":
 			      Support not available on the system.
 
+		The feature can be modified by writing to the interface, for example:
+
+		To enable:
+			# echo 1 > /sys/fs/resctrl/info/L3/io_alloc
+
+		To disable:
+			# echo 0 > /sys/fs/resctrl/info/L3/io_alloc
+
 		The underlying implementation may reduce resources available to
 		general (CPU) cache allocation. See architecture specific notes
 		below. Depending on usage requirements the feature can be enabled
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index d7c4417b4516..06c854caa55c 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -70,6 +70,7 @@ static struct seq_buf last_cmd_status;
 static char last_cmd_status_buf[512];
 
 static int rdtgroup_setup_root(struct rdt_fs_context *ctx);
+static int rdtgroup_init_cat(struct resctrl_schema *s, u32 closid);
 
 static void rdtgroup_destroy_root(void);
 
@@ -232,6 +233,16 @@ bool closid_allocated(unsigned int closid)
 	return !test_bit(closid, closid_free_map);
 }
 
+static bool resctrl_io_alloc_closid_alloc(u32 io_alloc_closid)
+{
+	return __test_and_clear_bit(io_alloc_closid, closid_free_map);
+}
+
+static void resctrl_io_alloc_closid_free(u32 io_alloc_closid)
+{
+	closid_free(io_alloc_closid);
+}
+
 /**
  * rdtgroup_mode_by_closid - Return mode of resource group with closid
  * @closid: closid if the resource group
@@ -1030,6 +1041,16 @@ static int rdt_shareable_bits_show(struct kernfs_open_file *of,
 	return 0;
 }
 
+/*
+ * resctrl_io_alloc_closid_supported() - io_alloc feature utilizes the
+ * highest CLOSID value to direct I/O traffic. Ensure that io_alloc_closid
+ * is in the supported range.
+ */
+static bool resctrl_io_alloc_closid_supported(u32 io_alloc_closid)
+{
+	return io_alloc_closid < closids_supported();
+}
+
 /*
  * resctrl_io_alloc_closid() - io_alloc feature routes I/O traffic using
  * the highest available CLOSID. Retrieve the maximum CLOSID supported by the
@@ -1858,6 +1879,131 @@ static int resctrl_io_alloc_show(struct kernfs_open_file *of,
 	return 0;
 }
 
+/*
+ * Initialize io_alloc CLOSID cache resource CBM with all usable (shared
+ * and unused) cache portions.
+ */
+static int resctrl_io_alloc_init_cat(struct rdt_resource *r,
+				     struct resctrl_schema *s, u32 closid)
+{
+	int ret;
+
+	rdt_staged_configs_clear();
+
+	ret = rdtgroup_init_cat(s, closid);
+	if (ret < 0)
+		goto out;
+
+	ret = resctrl_arch_update_domains(r, closid);
+
+out:
+	rdt_staged_configs_clear();
+	return ret;
+}
+
+static const char *rdtgroup_name_by_closid(int closid)
+{
+	struct rdtgroup *rdtgrp;
+
+	list_for_each_entry(rdtgrp, &rdt_all_groups, rdtgroup_list) {
+		if (rdtgrp->closid == closid)
+			return rdt_kn_name(rdtgrp->kn);
+	}
+
+	return NULL;
+}
+
+static struct resctrl_schema *resctrl_get_schema(enum resctrl_conf_type type)
+{
+	struct resctrl_schema *schema;
+
+	list_for_each_entry(schema, &resctrl_schema_all, list) {
+		if (schema->conf_type == type)
+			return schema;
+	}
+
+	return NULL;
+}
+
+static ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
+				      size_t nbytes, loff_t off)
+{
+	struct resctrl_schema *s = rdt_kn_parent_priv(of->kn);
+	enum resctrl_conf_type peer_type;
+	struct rdt_resource *r = s->res;
+	struct resctrl_schema *peer_s;
+	char const *grp_name;
+	u32 io_alloc_closid;
+	bool enable;
+	int ret;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret)
+		return ret;
+
+	cpus_read_lock();
+	mutex_lock(&rdtgroup_mutex);
+
+	rdt_last_cmd_clear();
+
+	if (!r->cache.io_alloc_capable) {
+		rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
+		ret = -ENODEV;
+		goto out_unlock;
+	}
+
+	io_alloc_closid = resctrl_io_alloc_closid(r);
+	if (!resctrl_io_alloc_closid_supported(io_alloc_closid)) {
+		rdt_last_cmd_printf("io_alloc CLOSID (ctrl_hw_id) %d is not available\n",
+				    io_alloc_closid);
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	/* If the feature is already up to date, no action is needed. */
+	if (resctrl_arch_get_io_alloc_enabled(r) == enable)
+		goto out_unlock;
+
+	if (enable) {
+		if (!resctrl_io_alloc_closid_alloc(io_alloc_closid)) {
+			grp_name = rdtgroup_name_by_closid(io_alloc_closid);
+			WARN_ON_ONCE(!grp_name);
+			rdt_last_cmd_printf("CLOSID (ctrl_hw_id) %d for io_alloc is used by %s group\n",
+					    io_alloc_closid, grp_name ? grp_name : "another");
+			ret = -ENOSPC;
+			goto out_unlock;
+		}
+
+		/* Initialize schema for both CDP_DATA and CDP_CODE when CDP is enabled */
+		if (resctrl_arch_get_cdp_enabled(r->rid)) {
+			peer_type = resctrl_peer_type(s->conf_type);
+			peer_s = resctrl_get_schema(peer_type);
+			if (peer_s)
+				ret = resctrl_io_alloc_init_cat(r, peer_s, io_alloc_closid);
+		}
+
+		if (!ret)
+			ret = resctrl_io_alloc_init_cat(r, s, io_alloc_closid);
+
+		if (ret) {
+			rdt_last_cmd_puts("Failed to initialize io_alloc allocations\n");
+			resctrl_io_alloc_closid_free(io_alloc_closid);
+			goto out_unlock;
+		}
+
+	} else {
+		resctrl_io_alloc_closid_free(io_alloc_closid);
+	}
+
+	ret = resctrl_arch_io_alloc_enable(r, enable);
+
+out_unlock:
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+
+	return ret ?: nbytes;
+}
+
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
@@ -1950,9 +2096,10 @@ static struct rftype res_common_files[] = {
 	},
 	{
 		.name		= "io_alloc",
-		.mode		= 0444,
+		.mode		= 0644,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= resctrl_io_alloc_show,
+		.write          = resctrl_io_alloc_write,
 	},
 	{
 		.name		= "max_threshold_occupancy",
-- 
2.34.1
Re: [PATCH v7 07/10] fs/resctrl: Add user interface to enable/disable io_alloc feature
Posted by Reinette Chatre 2 months, 2 weeks ago
Hi Babu,

On 7/10/25 10:16 AM, Babu Moger wrote:
> "io_alloc" feature in resctrl enables direct insertion of data from I/O
> devices into the cache.
> 
> Introduce user interface to enable/disable io_alloc feature.

I think it is worth a mention *why* a user may want to disable this feature and
why is not just always enabled. Here it can be highlighted that this feature
may take resources (CLOSID) away from general (CPU) cache allocation and since
this may be scarce enabling user to disable this feature supports different use cases.

> 
> On AMD systems, io_alloc feature is backed by SDCIAE (L3 Smart Data Cache
> Injection Allocation Enforcement). When enabled, SDCIAE directs all SDCI
> lines to be placed into the L3 cache partitions specified by the register
> corresponding to the highest CLOSID supported by the resource. With CDP
> enabled, io_alloc routes I/O traffic using the highest CLOSID assigned to
> the instruction cache (L3CODE).

This is a lot of architecture specific text for a resctrl fs patch  ... I think
you are trying to motivate the resctrl fs implementation. Similar motivation
as proposed for cover letter can be used here to help explain the implementation
choices.

> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---

...

> ---
>  Documentation/filesystems/resctrl.rst |   8 ++
>  fs/resctrl/rdtgroup.c                 | 149 +++++++++++++++++++++++++-
>  2 files changed, 156 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
> index 354e6a00fa45..189c1ccb92d6 100644
> --- a/Documentation/filesystems/resctrl.rst
> +++ b/Documentation/filesystems/resctrl.rst
> @@ -157,6 +157,14 @@ related to allocation:
>  			"not supported":
>  			      Support not available on the system.
>  
> +		The feature can be modified by writing to the interface, for example:
> +
> +		To enable:
> +			# echo 1 > /sys/fs/resctrl/info/L3/io_alloc
> +
> +		To disable:
> +			# echo 0 > /sys/fs/resctrl/info/L3/io_alloc
> +
>  		The underlying implementation may reduce resources available to
>  		general (CPU) cache allocation. See architecture specific notes
>  		below. Depending on usage requirements the feature can be enabled
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index d7c4417b4516..06c854caa55c 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -70,6 +70,7 @@ static struct seq_buf last_cmd_status;
>  static char last_cmd_status_buf[512];
>  
>  static int rdtgroup_setup_root(struct rdt_fs_context *ctx);
> +static int rdtgroup_init_cat(struct resctrl_schema *s, u32 closid);
>  
>  static void rdtgroup_destroy_root(void);
>  
> @@ -232,6 +233,16 @@ bool closid_allocated(unsigned int closid)
>  	return !test_bit(closid, closid_free_map);
>  }
>  
> +static bool resctrl_io_alloc_closid_alloc(u32 io_alloc_closid)
> +{
> +	return __test_and_clear_bit(io_alloc_closid, closid_free_map);
> +}
> +
> +static void resctrl_io_alloc_closid_free(u32 io_alloc_closid)
> +{
> +	closid_free(io_alloc_closid);
> +}

I do not think these should be helpers/wrappers with a separate
namespace. It will make the code easier to understand if it is clear that
the "io_alloc" CLOSID is allocated from the same "pool" as the CLOSID for
control groups. 

I would thus propose a specific, for example closid_alloc_fixed(u32 closid)
helper, and just call closid_free() directly.

> +
>  /**
>   * rdtgroup_mode_by_closid - Return mode of resource group with closid
>   * @closid: closid if the resource group
> @@ -1030,6 +1041,16 @@ static int rdt_shareable_bits_show(struct kernfs_open_file *of,
>  	return 0;
>  }
>  
> +/*
> + * resctrl_io_alloc_closid_supported() - io_alloc feature utilizes the
> + * highest CLOSID value to direct I/O traffic. Ensure that io_alloc_closid
> + * is in the supported range.
> + */
> +static bool resctrl_io_alloc_closid_supported(u32 io_alloc_closid)

Please move to ctrlmondata.c

> +{
> +	return io_alloc_closid < closids_supported();
> +}
> +
>  /*
>   * resctrl_io_alloc_closid() - io_alloc feature routes I/O traffic using
>   * the highest available CLOSID. Retrieve the maximum CLOSID supported by the
> @@ -1858,6 +1879,131 @@ static int resctrl_io_alloc_show(struct kernfs_open_file *of,
>  	return 0;
>  }
>  
> +/*
> + * Initialize io_alloc CLOSID cache resource CBM with all usable (shared
> + * and unused) cache portions.
> + */
> +static int resctrl_io_alloc_init_cat(struct rdt_resource *r,

Mixing two features (io_alloc and CAT) in the function name is confusing.
How about resctrl_io_alloc_init_cbm() and move to ctrlmondata.c?

> +				     struct resctrl_schema *s, u32 closid)

No need to provide resource as parameter, it can be determined from schema.

> +{
> +	int ret;
> +
> +	rdt_staged_configs_clear();
> +
> +	ret = rdtgroup_init_cat(s, closid);
> +	if (ret < 0)
> +		goto out;

More below, but I think this flow can be simplified by moving the logic
handling CDP here. If CDP is enabled for the resource then a successful
rdtgroup_init_cat() can just be followed by a snippet that copies the
staged config of the CDP type to the staged config of its peer type.
Their CBMs are supposed to be identical so there is no reason for all the
rdtgroup_init_cat() processing to be repeated. resctrl_arch_update_domains()
can handle updating both in a single call.

> +
> +	ret = resctrl_arch_update_domains(r, closid);
> +
> +out:
> +	rdt_staged_configs_clear();
> +	return ret;
> +}
> +
> +static const char *rdtgroup_name_by_closid(int closid)

This seems generic enough and appropriate for rdtgroup.c

> +{
> +	struct rdtgroup *rdtgrp;
> +
> +	list_for_each_entry(rdtgrp, &rdt_all_groups, rdtgroup_list) {
> +		if (rdtgrp->closid == closid)
> +			return rdt_kn_name(rdtgrp->kn);
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct resctrl_schema *resctrl_get_schema(enum resctrl_conf_type type)

This also seems generic enough and appropriate for rdtgroup.c

> +{
> +	struct resctrl_schema *schema;
> +
> +	list_for_each_entry(schema, &resctrl_schema_all, list) {
> +		if (schema->conf_type == type)
> +			return schema;
> +	}
> +
> +	return NULL;
> +}
> +
> +static ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
> +				      size_t nbytes, loff_t off)
> +{
> +	struct resctrl_schema *s = rdt_kn_parent_priv(of->kn);
> +	enum resctrl_conf_type peer_type;
> +	struct rdt_resource *r = s->res;
> +	struct resctrl_schema *peer_s;
> +	char const *grp_name;
> +	u32 io_alloc_closid;
> +	bool enable;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &enable);
> +	if (ret)
> +		return ret;
> +
> +	cpus_read_lock();
> +	mutex_lock(&rdtgroup_mutex);
> +
> +	rdt_last_cmd_clear();
> +
> +	if (!r->cache.io_alloc_capable) {
> +		rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
> +		ret = -ENODEV;
> +		goto out_unlock;
> +	}
> +
> +	io_alloc_closid = resctrl_io_alloc_closid(r);
> +	if (!resctrl_io_alloc_closid_supported(io_alloc_closid)) {
> +		rdt_last_cmd_printf("io_alloc CLOSID (ctrl_hw_id) %d is not available\n",
> +				    io_alloc_closid);
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +
> +	/* If the feature is already up to date, no action is needed. */
> +	if (resctrl_arch_get_io_alloc_enabled(r) == enable)
> +		goto out_unlock;
> +
> +	if (enable) {
> +		if (!resctrl_io_alloc_closid_alloc(io_alloc_closid)) {
> +			grp_name = rdtgroup_name_by_closid(io_alloc_closid);
> +			WARN_ON_ONCE(!grp_name);
> +			rdt_last_cmd_printf("CLOSID (ctrl_hw_id) %d for io_alloc is used by %s group\n",
> +					    io_alloc_closid, grp_name ? grp_name : "another");
> +			ret = -ENOSPC;
> +			goto out_unlock;
> +		}
> +
> +		/* Initialize schema for both CDP_DATA and CDP_CODE when CDP is enabled */
> +		if (resctrl_arch_get_cdp_enabled(r->rid)) {

I think this block can be removed to simplify the flow by moving the CDP handling to 
resctrl_io_alloc_init_cat().

> +			peer_type = resctrl_peer_type(s->conf_type);
> +			peer_s = resctrl_get_schema(peer_type);
> +			if (peer_s)
> +				ret = resctrl_io_alloc_init_cat(r, peer_s, io_alloc_closid);
> +		}
> +
> +		if (!ret)
> +			ret = resctrl_io_alloc_init_cat(r, s, io_alloc_closid);
> +
> +		if (ret) {
> +			rdt_last_cmd_puts("Failed to initialize io_alloc allocations\n");
> +			resctrl_io_alloc_closid_free(io_alloc_closid);
> +			goto out_unlock;
> +		}
> +
> +	} else {
> +		resctrl_io_alloc_closid_free(io_alloc_closid);
> +	}
> +
> +	ret = resctrl_arch_io_alloc_enable(r, enable);
> +
> +out_unlock:
> +	mutex_unlock(&rdtgroup_mutex);
> +	cpus_read_unlock();
> +
> +	return ret ?: nbytes;
> +}
> +
>  /* rdtgroup information files for one cache resource. */
>  static struct rftype res_common_files[] = {
>  	{
> @@ -1950,9 +2096,10 @@ static struct rftype res_common_files[] = {
>  	},
>  	{
>  		.name		= "io_alloc",
> -		.mode		= 0444,
> +		.mode		= 0644,
>  		.kf_ops		= &rdtgroup_kf_single_ops,
>  		.seq_show	= resctrl_io_alloc_show,
> +		.write          = resctrl_io_alloc_write,
>  	},
>  	{
>  		.name		= "max_threshold_occupancy",

Reinette
Re: [PATCH v7 07/10] fs/resctrl: Add user interface to enable/disable io_alloc feature
Posted by Moger, Babu 2 months, 1 week ago
Hi Reinette,

On 7/21/2025 6:40 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 7/10/25 10:16 AM, Babu Moger wrote:
>> "io_alloc" feature in resctrl enables direct insertion of data from I/O
>> devices into the cache.
>>
>> Introduce user interface to enable/disable io_alloc feature.
> 
> I think it is worth a mention *why* a user may want to disable this feature and
> why is not just always enabled. Here it can be highlighted that this feature
> may take resources (CLOSID) away from general (CPU) cache allocation and since
> this may be scarce enabling user to disable this feature supports different use cases.
> 

Sure.

>>
>> On AMD systems, io_alloc feature is backed by SDCIAE (L3 Smart Data Cache
>> Injection Allocation Enforcement). When enabled, SDCIAE directs all SDCI
>> lines to be placed into the L3 cache partitions specified by the register
>> corresponding to the highest CLOSID supported by the resource. With CDP
>> enabled, io_alloc routes I/O traffic using the highest CLOSID assigned to
>> the instruction cache (L3CODE).
> 
> This is a lot of architecture specific text for a resctrl fs patch  ... I think
> you are trying to motivate the resctrl fs implementation. Similar motivation
> as proposed for cover letter can be used here to help explain the implementation
> choices.

Updated the whole changelog.

fs/resctrl: Add user interface to enable/disable io_alloc feature

"io_alloc" feature in resctrl enables direct insertion of data from I/O
devices into the cache.

Introduce user interface to enable/disable io_alloc feature.

On AMD systems, when io_alloc is enabled, the highest CLOSID is reserved
exclusively for I/O allocation traffic and is no longer available for
general CPU cache allocation. This feature is disabled by default. Users
are encouraged to enable it only when running workloads that can benefit
from this functionality.

Since CLOSIDs are managed by resctrl fs, it is least invasive to make the
"io_alloc is supported by maximum supported CLOSID" part of the initial
resctrl fs support for io_alloc. Take care not to expose this use of
CLOSID for io_alloc to user space so that this is not required from other
architectures that may support io_alloc differently in the future.

Signed-off-by: Babu Moger <babu.moger@amd.com>

> 
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
> 
> ...
> 
>> ---
>>   Documentation/filesystems/resctrl.rst |   8 ++
>>   fs/resctrl/rdtgroup.c                 | 149 +++++++++++++++++++++++++-
>>   2 files changed, 156 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
>> index 354e6a00fa45..189c1ccb92d6 100644
>> --- a/Documentation/filesystems/resctrl.rst
>> +++ b/Documentation/filesystems/resctrl.rst
>> @@ -157,6 +157,14 @@ related to allocation:
>>   			"not supported":
>>   			      Support not available on the system.
>>   
>> +		The feature can be modified by writing to the interface, for example:
>> +
>> +		To enable:
>> +			# echo 1 > /sys/fs/resctrl/info/L3/io_alloc
>> +
>> +		To disable:
>> +			# echo 0 > /sys/fs/resctrl/info/L3/io_alloc
>> +
>>   		The underlying implementation may reduce resources available to
>>   		general (CPU) cache allocation. See architecture specific notes
>>   		below. Depending on usage requirements the feature can be enabled
>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index d7c4417b4516..06c854caa55c 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -70,6 +70,7 @@ static struct seq_buf last_cmd_status;
>>   static char last_cmd_status_buf[512];
>>   
>>   static int rdtgroup_setup_root(struct rdt_fs_context *ctx);
>> +static int rdtgroup_init_cat(struct resctrl_schema *s, u32 closid);
>>   
>>   static void rdtgroup_destroy_root(void);
>>   
>> @@ -232,6 +233,16 @@ bool closid_allocated(unsigned int closid)
>>   	return !test_bit(closid, closid_free_map);
>>   }
>>   
>> +static bool resctrl_io_alloc_closid_alloc(u32 io_alloc_closid)
>> +{
>> +	return __test_and_clear_bit(io_alloc_closid, closid_free_map);
>> +}
>> +
>> +static void resctrl_io_alloc_closid_free(u32 io_alloc_closid)
>> +{
>> +	closid_free(io_alloc_closid);
>> +}
> 
> I do not think these should be helpers/wrappers with a separate
> namespace. It will make the code easier to understand if it is clear that
> the "io_alloc" CLOSID is allocated from the same "pool" as the CLOSID for
> control groups.
> 
> I would thus propose a specific, for example closid_alloc_fixed(u32 closid)
> helper, and just call closid_free() directly.
> 

Sure.

>> +
>>   /**
>>    * rdtgroup_mode_by_closid - Return mode of resource group with closid
>>    * @closid: closid if the resource group
>> @@ -1030,6 +1041,16 @@ static int rdt_shareable_bits_show(struct kernfs_open_file *of,
>>   	return 0;
>>   }
>>   
>> +/*
>> + * resctrl_io_alloc_closid_supported() - io_alloc feature utilizes the
>> + * highest CLOSID value to direct I/O traffic. Ensure that io_alloc_closid
>> + * is in the supported range.
>> + */
>> +static bool resctrl_io_alloc_closid_supported(u32 io_alloc_closid)
> 
> Please move to ctrlmondata.c
> 

Sure.

>> +{
>> +	return io_alloc_closid < closids_supported();
>> +}
>> +
>>   /*
>>    * resctrl_io_alloc_closid() - io_alloc feature routes I/O traffic using
>>    * the highest available CLOSID. Retrieve the maximum CLOSID supported by the
>> @@ -1858,6 +1879,131 @@ static int resctrl_io_alloc_show(struct kernfs_open_file *of,
>>   	return 0;
>>   }
>>   
>> +/*
>> + * Initialize io_alloc CLOSID cache resource CBM with all usable (shared
>> + * and unused) cache portions.
>> + */
>> +static int resctrl_io_alloc_init_cat(struct rdt_resource *r,
> 
> Mixing two features (io_alloc and CAT) in the function name is confusing.
> How about resctrl_io_alloc_init_cbm() and move to ctrlmondata.c?

Yes.

> 
>> +				     struct resctrl_schema *s, u32 closid)
> 
> No need to provide resource as parameter, it can be determined from schema.

Sure.

> 
>> +{
>> +	int ret;
>> +
>> +	rdt_staged_configs_clear();
>> +
>> +	ret = rdtgroup_init_cat(s, closid);
>> +	if (ret < 0)
>> +		goto out;
> 
> More below, but I think this flow can be simplified by moving the logic
> handling CDP here. If CDP is enabled for the resource then a successful
> rdtgroup_init_cat() can just be followed by a snippet that copies the
> staged config of the CDP type to the staged config of its peer type.
> Their CBMs are supposed to be identical so there is no reason for all the
> rdtgroup_init_cat() processing to be repeated. resctrl_arch_update_domains()
> can handle updating both in a single call.

Yes.

> 
>> +
>> +	ret = resctrl_arch_update_domains(r, closid);
>> +
>> +out:
>> +	rdt_staged_configs_clear();
>> +	return ret;
>> +}
>> +
>> +static const char *rdtgroup_name_by_closid(int closid)
> 
> This seems generic enough and appropriate for rdtgroup.c
> 
>> +{
>> +	struct rdtgroup *rdtgrp;
>> +
>> +	list_for_each_entry(rdtgrp, &rdt_all_groups, rdtgroup_list) {
>> +		if (rdtgrp->closid == closid)
>> +			return rdt_kn_name(rdtgrp->kn);
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>> +static struct resctrl_schema *resctrl_get_schema(enum resctrl_conf_type type)
> 
> This also seems generic enough and appropriate for rdtgroup.c
> 
>> +{
>> +	struct resctrl_schema *schema;
>> +
>> +	list_for_each_entry(schema, &resctrl_schema_all, list) {
>> +		if (schema->conf_type == type)
>> +			return schema;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>> +static ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
>> +				      size_t nbytes, loff_t off)
>> +{
>> +	struct resctrl_schema *s = rdt_kn_parent_priv(of->kn);
>> +	enum resctrl_conf_type peer_type;
>> +	struct rdt_resource *r = s->res;
>> +	struct resctrl_schema *peer_s;
>> +	char const *grp_name;
>> +	u32 io_alloc_closid;
>> +	bool enable;
>> +	int ret;
>> +
>> +	ret = kstrtobool(buf, &enable);
>> +	if (ret)
>> +		return ret;
>> +
>> +	cpus_read_lock();
>> +	mutex_lock(&rdtgroup_mutex);
>> +
>> +	rdt_last_cmd_clear();
>> +
>> +	if (!r->cache.io_alloc_capable) {
>> +		rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
>> +		ret = -ENODEV;
>> +		goto out_unlock;
>> +	}
>> +
>> +	io_alloc_closid = resctrl_io_alloc_closid(r);
>> +	if (!resctrl_io_alloc_closid_supported(io_alloc_closid)) {
>> +		rdt_last_cmd_printf("io_alloc CLOSID (ctrl_hw_id) %d is not available\n",
>> +				    io_alloc_closid);
>> +		ret = -EINVAL;
>> +		goto out_unlock;
>> +	}
>> +
>> +	/* If the feature is already up to date, no action is needed. */
>> +	if (resctrl_arch_get_io_alloc_enabled(r) == enable)
>> +		goto out_unlock;
>> +
>> +	if (enable) {
>> +		if (!resctrl_io_alloc_closid_alloc(io_alloc_closid)) {
>> +			grp_name = rdtgroup_name_by_closid(io_alloc_closid);
>> +			WARN_ON_ONCE(!grp_name);
>> +			rdt_last_cmd_printf("CLOSID (ctrl_hw_id) %d for io_alloc is used by %s group\n",
>> +					    io_alloc_closid, grp_name ? grp_name : "another");
>> +			ret = -ENOSPC;
>> +			goto out_unlock;
>> +		}
>> +
>> +		/* Initialize schema for both CDP_DATA and CDP_CODE when CDP is enabled */
>> +		if (resctrl_arch_get_cdp_enabled(r->rid)) {
> 
> I think this block can be removed to simplify the flow by moving the CDP handling to
> resctrl_io_alloc_init_cat().

Sure. Also moved the whole function to fs/resctrl/ctrlmondata.c.

> 
>> +			peer_type = resctrl_peer_type(s->conf_type);
>> +			peer_s = resctrl_get_schema(peer_type);
>> +			if (peer_s)
>> +				ret = resctrl_io_alloc_init_cat(r, peer_s, io_alloc_closid);
>> +		}
>> +
>> +		if (!ret)
>> +			ret = resctrl_io_alloc_init_cat(r, s, io_alloc_closid);
>> +
>> +		if (ret) {
>> +			rdt_last_cmd_puts("Failed to initialize io_alloc allocations\n");
>> +			resctrl_io_alloc_closid_free(io_alloc_closid);
>> +			goto out_unlock;
>> +		}
>> +
>> +	} else {
>> +		resctrl_io_alloc_closid_free(io_alloc_closid);
>> +	}
>> +
>> +	ret = resctrl_arch_io_alloc_enable(r, enable);
>> +
>> +out_unlock:
>> +	mutex_unlock(&rdtgroup_mutex);
>> +	cpus_read_unlock();
>> +
>> +	return ret ?: nbytes;
>> +}
>> +
>>   /* rdtgroup information files for one cache resource. */
>>   static struct rftype res_common_files[] = {
>>   	{
>> @@ -1950,9 +2096,10 @@ static struct rftype res_common_files[] = {
>>   	},
>>   	{
>>   		.name		= "io_alloc",
>> -		.mode		= 0444,
>> +		.mode		= 0644,
>>   		.kf_ops		= &rdtgroup_kf_single_ops,
>>   		.seq_show	= resctrl_io_alloc_show,
>> +		.write          = resctrl_io_alloc_write,
>>   	},
>>   	{
>>   		.name		= "max_threshold_occupancy",
> 
> Reinette
> 

Thanks
Babu
Re: [PATCH v7 07/10] fs/resctrl: Add user interface to enable/disable io_alloc feature
Posted by Reinette Chatre 2 months ago
Hi Babu,

On 7/31/25 3:52 PM, Moger, Babu wrote:
> Hi Reinette,
> 
> On 7/21/2025 6:40 PM, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 7/10/25 10:16 AM, Babu Moger wrote:
>>> "io_alloc" feature in resctrl enables direct insertion of data from I/O
>>> devices into the cache.
>>>
>>> Introduce user interface to enable/disable io_alloc feature.
>>
>> I think it is worth a mention *why* a user may want to disable this feature and
>> why is not just always enabled. Here it can be highlighted that this feature
>> may take resources (CLOSID) away from general (CPU) cache allocation and since
>> this may be scarce enabling user to disable this feature supports different use cases.
>>
> 
> Sure.
> 
>>>
>>> On AMD systems, io_alloc feature is backed by SDCIAE (L3 Smart Data Cache
>>> Injection Allocation Enforcement). When enabled, SDCIAE directs all SDCI
>>> lines to be placed into the L3 cache partitions specified by the register
>>> corresponding to the highest CLOSID supported by the resource. With CDP
>>> enabled, io_alloc routes I/O traffic using the highest CLOSID assigned to
>>> the instruction cache (L3CODE).
>>
>> This is a lot of architecture specific text for a resctrl fs patch  ... I think
>> you are trying to motivate the resctrl fs implementation. Similar motivation
>> as proposed for cover letter can be used here to help explain the implementation
>> choices.
> 
> Updated the whole changelog.
> 
> fs/resctrl: Add user interface to enable/disable io_alloc feature
> 
> "io_alloc" feature in resctrl enables direct insertion of data from I/O
> devices into the cache.
> 
> Introduce user interface to enable/disable io_alloc feature.

The solution should be at end of changelog after description of problem it
solves.

> On AMD systems, when io_alloc is enabled, the highest CLOSID is reserved
> exclusively for I/O allocation traffic and is no longer available for
> general CPU cache allocation. This feature is disabled by default. Users

Changelog should always be in imperative tone and problem and solution should
be in separate paragraphs (above paragraph mixes problem and solution).  

For example, "Disable "io_alloc" feature by default to ensure all resources are
available for general CPU cache allocation. ..." Although I do not think this is
accurate since this patch does not do this?

> are encouraged to enable it only when running workloads that can benefit
> from this functionality.
> 
> Since CLOSIDs are managed by resctrl fs, it is least invasive to make the
> "io_alloc is supported by maximum supported CLOSID" part of the initial
> resctrl fs support for io_alloc. Take care not to expose this use of
> CLOSID for io_alloc to user space so that this is not required from other
> architectures that may support io_alloc differently in the future.
> 

The changelog requirements I refer to are documented in "Changelog" section
of Documentation/process/maintainer-tip.rst. I feel like this should be
familiar to you by now.

Reinette
Re: [PATCH v7 07/10] fs/resctrl: Add user interface to enable/disable io_alloc feature
Posted by Moger, Babu 2 months ago
Hi Reinette,

On 8/4/25 11:07, Reinette Chatre wrote:
> Hi Babu,
> 
> On 7/31/25 3:52 PM, Moger, Babu wrote:
>> Hi Reinette,
>>
>> On 7/21/2025 6:40 PM, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 7/10/25 10:16 AM, Babu Moger wrote:
>>>> "io_alloc" feature in resctrl enables direct insertion of data from I/O
>>>> devices into the cache.
>>>>
>>>> Introduce user interface to enable/disable io_alloc feature.
>>>
>>> I think it is worth a mention *why* a user may want to disable this feature and
>>> why is not just always enabled. Here it can be highlighted that this feature
>>> may take resources (CLOSID) away from general (CPU) cache allocation and since
>>> this may be scarce enabling user to disable this feature supports different use cases.
>>>
>>
>> Sure.
>>
>>>>
>>>> On AMD systems, io_alloc feature is backed by SDCIAE (L3 Smart Data Cache
>>>> Injection Allocation Enforcement). When enabled, SDCIAE directs all SDCI
>>>> lines to be placed into the L3 cache partitions specified by the register
>>>> corresponding to the highest CLOSID supported by the resource. With CDP
>>>> enabled, io_alloc routes I/O traffic using the highest CLOSID assigned to
>>>> the instruction cache (L3CODE).
>>>
>>> This is a lot of architecture specific text for a resctrl fs patch  ... I think
>>> you are trying to motivate the resctrl fs implementation. Similar motivation
>>> as proposed for cover letter can be used here to help explain the implementation
>>> choices.
>>
>> Updated the whole changelog.
>>
>> fs/resctrl: Add user interface to enable/disable io_alloc feature
>>
>> "io_alloc" feature in resctrl enables direct insertion of data from I/O
>> devices into the cache.
>>
>> Introduce user interface to enable/disable io_alloc feature.
> 
> The solution should be at end of changelog after description of problem it
> solves.

Sure. Moved this text below.

> 
>> On AMD systems, when io_alloc is enabled, the highest CLOSID is reserved
>> exclusively for I/O allocation traffic and is no longer available for
>> general CPU cache allocation. This feature is disabled by default. Users
> 
> Changelog should always be in imperative tone and problem and solution should
> be in separate paragraphs (above paragraph mixes problem and solution).  
> 
> For example, "Disable "io_alloc" feature by default to ensure all resources are
> available for general CPU cache allocation. ..." Although I do not think this is
> accurate since this patch does not do this?

Yes. Removed the text "This feature is disabled by default."
> 
>> are encouraged to enable it only when running workloads that can benefit
>> from this functionality.
>>
>> Since CLOSIDs are managed by resctrl fs, it is least invasive to make the
>> "io_alloc is supported by maximum supported CLOSID" part of the initial
>> resctrl fs support for io_alloc. Take care not to expose this use of
>> CLOSID for io_alloc to user space so that this is not required from other
>> architectures that may support io_alloc differently in the future.
>>
> 
> The changelog requirements I refer to are documented in "Changelog" section
> of Documentation/process/maintainer-tip.rst. I feel like this should be
> familiar to you by now.

Sure,  Thank you.

-- 
Thanks
Babu Moger