This patch introduces a special domain ID selector "*" for io_alloc_cbm
that allows setting the CBM of each cache domain to its minimum number
of consecutive bits in a single operation. For example, writing "*=0" to
/sys/fs/resctrl/info/L3/io_alloc_cbm programs each domain's CBM to the
hardware-defined minimum, greatly simplifying automation and management
tasks. The user is required to specify the correct minimum stored in
/sys/fs/resctrl/info/L3/min_cbm_bits.

Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 Documentation/filesystems/resctrl.rst     |  10 ++
 arch/x86/kernel/cpu/resctrl/core.c        |   2 +-
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  23 ++--
 fs/resctrl/ctrlmondata.c                  | 141 ++++++++++++++++------
 fs/resctrl/internal.h                     |  13 ++
 fs/resctrl/rdtgroup.c                     |   2 +-
 include/linux/resctrl.h                   |  30 ++++-
 7 files changed, 174 insertions(+), 47 deletions(-)

diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
index 8c8ce678148a..9c128c8fba86 100644
--- a/Documentation/filesystems/resctrl.rst
+++ b/Documentation/filesystems/resctrl.rst
@@ -215,6 +215,16 @@ related to allocation:
 			# cat /sys/fs/resctrl/info/L3/io_alloc_cbm
 			0=00ff;1=000f
 
+		Set each CBM to their minimum number of consecutive bits.
+
+		A special value "*" is required to represent all cache IDs.
+		The user should specify the correct minimum stored in
+		/sys/fs/resctrl/info/L3/min_cbm_bits.
+
+		Example::
+
+			# echo "*=0" > /sys/fs/resctrl/info/L3/io_alloc_cbm
+
 		When CDP is enabled "io_alloc_cbm" associated with the CDP_DATA and CDP_CODE
 		resources may reflect the same values. For example, values read from and
 		written to /sys/fs/resctrl/info/L3DATA/io_alloc_cbm may be reflected by
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 3792ab4819dc..44aea6b534e0 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -276,7 +276,7 @@ static void rdt_get_cdp_config(int level)
 
 static void rdt_set_io_alloc_capable(struct rdt_resource *r)
 {
-	r->cache.io_alloc_capable = true;
+	r->cache.io_alloc.io_alloc_capable = true;
 }
 
 static void rdt_get_cdp_l3_config(void)
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index b20e705606b8..0f051d848422 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -57,14 +57,19 @@ int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid)
 		hw_dom = resctrl_to_arch_ctrl_dom(d);
 		msr_param.res = NULL;
 		for (t = 0; t < CDP_NUM_TYPES; t++) {
-			cfg = &hw_dom->d_resctrl.staged_config[t];
-			if (!cfg->have_new_ctrl)
-				continue;
-
-			idx = resctrl_get_config_index(closid, t);
-			if (cfg->new_ctrl == hw_dom->ctrl_val[idx])
-				continue;
-			hw_dom->ctrl_val[idx] = cfg->new_ctrl;
+			if (resctrl_should_io_alloc_min_cbm(r)) {
+				idx = resctrl_get_config_index(closid, t);
+				hw_dom->ctrl_val[idx] = apply_io_alloc_min_cbm(r);
+			} else {
+				cfg = &hw_dom->d_resctrl.staged_config[t];
+				if (!cfg->have_new_ctrl)
+					continue;
+
+				idx = resctrl_get_config_index(closid, t);
+				if (cfg->new_ctrl == hw_dom->ctrl_val[idx])
+					continue;
+				hw_dom->ctrl_val[idx] = cfg->new_ctrl;
+			}
 
 			if (!msr_param.res) {
 				msr_param.low = idx;
@@ -123,7 +128,7 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable)
 {
 	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
 
-	if (hw_res->r_resctrl.cache.io_alloc_capable &&
+	if (hw_res->r_resctrl.cache.io_alloc.io_alloc_capable &&
 	    hw_res->sdciae_enabled != enable) {
 		_resctrl_sdciae_enable(r, enable);
 		hw_res->sdciae_enabled = enable;
diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
index d2f9a4f2d887..a36b9055a1be 100644
--- a/fs/resctrl/ctrlmondata.c
+++ b/fs/resctrl/ctrlmondata.c
@@ -688,7 +688,7 @@ int resctrl_io_alloc_show(struct kernfs_open_file *of, struct seq_file *seq, voi
 
 	mutex_lock(&rdtgroup_mutex);
 
-	if (r->cache.io_alloc_capable) {
+	if (r->cache.io_alloc.io_alloc_capable) {
 		if (resctrl_arch_get_io_alloc_enabled(r))
 			seq_puts(seq, "enabled\n");
 		else
@@ -712,13 +712,31 @@ static bool resctrl_io_alloc_closid_supported(u32 io_alloc_closid)
 	return io_alloc_closid < closids_supported();
 }
 
+/**
+ * resctrl_sync_cdp_peer - Mirror staged configuration to the CDP peer type
+ *
+ * @s: resctrl schema
+ * @d: resctrl control domain
+ *
+ * The caller must ensure CDP is enabled for the resource and must be
+ * called under the cpu hotplug lock and rdtgroup mutex
+ */
+static void resctrl_sync_cdp_peer(struct resctrl_schema *s, struct rdt_ctrl_domain *d)
+{
+	enum resctrl_conf_type peer_type;
+
+	peer_type = resctrl_peer_type(s->conf_type);
+	memcpy(&d->staged_config[peer_type],
+	       &d->staged_config[s->conf_type],
+	       sizeof(d->staged_config[0]));
+}
+
 /*
  * Initialize io_alloc CLOSID cache resource CBM with all usable (shared
  * and unused) cache portions.
  */
 static int resctrl_io_alloc_init_cbm(struct resctrl_schema *s, u32 closid)
 {
-	enum resctrl_conf_type peer_type;
 	struct rdt_resource *r = s->res;
 	struct rdt_ctrl_domain *d;
 	int ret;
@@ -731,11 +749,8 @@ static int resctrl_io_alloc_init_cbm(struct resctrl_schema *s, u32 closid)
 
 	/* Keep CDP_CODE and CDP_DATA of io_alloc CLOSID's CBM in sync. */
 	if (resctrl_arch_get_cdp_enabled(r->rid)) {
-		peer_type = resctrl_peer_type(s->conf_type);
 		list_for_each_entry(d, &s->res->ctrl_domains, hdr.list)
-			memcpy(&d->staged_config[peer_type],
-			       &d->staged_config[s->conf_type],
-			       sizeof(d->staged_config[0]));
+			resctrl_sync_cdp_peer(s, d);
 	}
 
 	ret = resctrl_arch_update_domains(r, closid);
@@ -903,49 +918,103 @@ int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq,
 	return ret;
 }
 
+/**
+ * parse_domain_cbm - Parse "domain=cbm" and return either side
+ *
+ * @line_ptr: Pointer to the input string
+ * @out_val: Output from parsed value (either domain ID or CDM)
+ * @which: Selector for which side to parse
+ *
+ * It is assumed that *line_ptr and *out_val are valid.
+ *
+ * Return: 0 on success and advance *line_ptr to point past the
+ * delimiter, EINVAL on parsing error
+ */
+static inline int parse_domain_cbm(char **line_ptr, unsigned long *out_val,
+				   enum resctrl_kv_sel which)
+{
+	char *rhs, *lhs, *tok;
+	unsigned int base;
+
+	rhs = *line_ptr;
+
+	lhs = strsep(&rhs, "=");
+	if (!lhs || !rhs)
+		goto err;
+
+	if (which == RDT_PARSE_DOMAIN) {
+		tok = lhs;
+		base = 10;
+	} else {
+		tok = rhs;
+		base = 16;
+	}
+	tok = strim(tok);
+
+	if (kstrtoul(tok, base, out_val))
+		goto err;
+
+	*line_ptr = rhs;
+	return 0;
+err:
+	rdt_last_cmd_puts("Invalid domain=cbm: missing '=' or non-numeric value\n");
+	return -EINVAL;
+}
+
 static int resctrl_io_alloc_parse_line(char *line,  struct rdt_resource *r,
 				       struct resctrl_schema *s, u32 closid)
 {
-	enum resctrl_conf_type peer_type;
 	struct rdt_parse_data data;
 	struct rdt_ctrl_domain *d;
-	char *dom = NULL, *id;
-	unsigned long dom_id;
+	char *cbm = NULL;
+	unsigned long out_val;
+	int ret;
 
-next:
-	if (!line || line[0] == '\0')
-		return 0;
+	if (line[0] == '*') {
+		ret = parse_domain_cbm(&line, &out_val, RDT_PARSE_CBM);
+		if (ret)
+			return ret;
 
-	dom = strsep(&line, ";");
-	id = strsep(&dom, "=");
-	if (!dom || kstrtoul(id, 10, &dom_id)) {
-		rdt_last_cmd_puts("Missing '=' or non-numeric domain\n");
+		if (out_val == r->cache.min_cbm_bits) {
+			r->cache.io_alloc.io_alloc_min_cbm = true;
+			return 0;
+		}
+		rdt_last_cmd_puts("Invalid io_alloc min CBM\n");
 		return -EINVAL;
 	}
 
-	dom = strim(dom);
-	list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
-		if (d->hdr.id == dom_id) {
-			data.buf = dom;
-			data.mode = RDT_MODE_SHAREABLE;
-			data.closid = closid;
-			if (parse_cbm(&data, s, d))
-				return -EINVAL;
-			/*
-			 * Keep io_alloc CLOSID's CBM of CDP_CODE and CDP_DATA
-			 * in sync.
-			 */
-			if (resctrl_arch_get_cdp_enabled(r->rid)) {
-				peer_type = resctrl_peer_type(s->conf_type);
-				memcpy(&d->staged_config[peer_type],
-				       &d->staged_config[s->conf_type],
-				       sizeof(d->staged_config[0]));
+	while (line && line[0] != '\0') {
+		bool found = false;
+
+		cbm = strsep(&line, ";");
+		ret = parse_domain_cbm(&cbm, &out_val, RDT_PARSE_DOMAIN);
+		if (ret)
+			return ret;
+
+		cbm = strim(cbm);
+		list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
+			if (d->hdr.id == out_val) {
+				data.buf = cbm;
+				data.mode = RDT_MODE_SHAREABLE;
+				data.closid = closid;
+				if (parse_cbm(&data, s, d))
+					return -EINVAL;
+				/*
+				 * Keep io_alloc CLOSID's CBM of CDP_CODE and CDP_DATA
+				 * in sync.
+				 */
+				if (resctrl_arch_get_cdp_enabled(r->rid))
+					resctrl_sync_cdp_peer(s, d);
+				found = true;
+				break;
 			}
-			goto next;
 		}
+
+		if (!found)
+			return -EINVAL;
 	}
 
-	return -EINVAL;
+	return 0;
 }
 
 ssize_t resctrl_io_alloc_cbm_write(struct kernfs_open_file *of, char *buf,
@@ -982,6 +1051,8 @@ ssize_t resctrl_io_alloc_cbm_write(struct kernfs_open_file *of, char *buf,
 		goto out_clear_configs;
 
 	ret = resctrl_arch_update_domains(r, io_alloc_closid);
+	if (resctrl_should_io_alloc_min_cbm(r))
+		r->cache.io_alloc.io_alloc_min_cbm = false;
 
 out_clear_configs:
 	rdt_staged_configs_clear();
diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
index bff4a54ae333..3846737992bb 100644
--- a/fs/resctrl/internal.h
+++ b/fs/resctrl/internal.h
@@ -143,6 +143,19 @@ enum rdt_group_type {
 	RDT_NUM_GROUP,
 };
 
+/**
+ * enum resctrl_kv_sel - Selector for "domain=cbm" key/value parsing
+ * @RDT_PARSE_DOMAIN: Parse the left-hand side (domain ID)
+ * @RDT_PARSE_CBM: Parse the right-hand side (CBM value)
+ *
+ * Used by parsers of key/value pairs formatted as "domain=cbm" to indicate
+ * which token should be extracted and converted.
+ */
+enum resctrl_kv_sel {
+	RDT_PARSE_DOMAIN,
+	RDT_PARSE_CBM,
+};
+
 /**
  * enum rdtgrp_mode - Mode of a RDT resource group
  * @RDT_MODE_SHAREABLE: This resource group allows sharing of its allocations
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index e1dc63b2218f..83ce3bafa5cb 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -2196,7 +2196,7 @@ static void io_alloc_init(void)
 {
 	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
 
-	if (r->cache.io_alloc_capable) {
+	if (r->cache.io_alloc.io_alloc_capable) {
 		resctrl_file_fflags_init("io_alloc", RFTYPE_CTRL_INFO |
 					 RFTYPE_RES_CACHE);
 		resctrl_file_fflags_init("io_alloc_cbm",
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 54701668b3df..9e75959565dc 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -215,7 +215,10 @@ struct resctrl_cache {
 	unsigned int	shareable_bits;
 	bool		arch_has_sparse_bitmasks;
 	bool		arch_has_per_cpu_cfg;
-	bool		io_alloc_capable;
+	struct {
+		bool io_alloc_capable;
+		bool io_alloc_min_cbm;
+	} io_alloc;
 };
 
 /**
@@ -415,6 +418,31 @@ static inline bool resctrl_is_mbm_event(enum resctrl_event_id eventid)
 		eventid <= QOS_L3_MBM_LOCAL_EVENT_ID);
 }
 
+/**
+ * apply_io_alloc_min_cbm() - Apply minimum io_alloc CBM
+ *
+ * @r: resctrl resource
+ *
+ * Return: Minimum number of consecutive io_alloc CBM bits to be set.
+ */
+static inline u32 apply_io_alloc_min_cbm(struct rdt_resource *r)
+{
+	return r->cache.min_cbm_bits;
+}
+
+/**
+ * resctrl_should_io_alloc_min_cbm() - Should the minimum io_alloc
+ *				       CBM be applied
+ * @r: resctrl resource
+ *
+ * Return: True if the minimum number of consecutive
+ * bits to be set in the io_alloc CBM should be applied.
+ */
+static inline bool resctrl_should_io_alloc_min_cbm(struct rdt_resource *r)
+{
+	return r->cache.io_alloc.io_alloc_min_cbm;
+}
+
 u32 resctrl_get_mon_evt_cfg(enum resctrl_event_id eventid);
 
 /* Iterate over all memory bandwidth events */
-- 
2.51.0

Hi Aaron,

On 12/15/2025 5:02 PM, Aaron Tomlin wrote:
> This patch introduces a special domain ID selector "*" for io_alloc_cbm
> that allows setting the CBM of each cache domain to its minimum number
> of consecutive bits in a single operation. For example, writing "*=0" to
> /sys/fs/resctrl/info/L3/io_alloc_cbm programs each domain's CBM to the
> hardware-defined minimum, greatly simplifying automation and management
> tasks. The user is required to specify the correct minimum stored in
> /sys/fs/resctrl/info/L3/min_cbm_bits.
> 
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
>   Documentation/filesystems/resctrl.rst     |  10 ++
>   arch/x86/kernel/cpu/resctrl/core.c        |   2 +-
>   arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  23 ++--
>   fs/resctrl/ctrlmondata.c                  | 141 ++++++++++++++++------
>   fs/resctrl/internal.h                     |  13 ++
>   fs/resctrl/rdtgroup.c                     |   2 +-
>   include/linux/resctrl.h                   |  30 ++++-
>   7 files changed, 174 insertions(+), 47 deletions(-)
> 
> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
> index 8c8ce678148a..9c128c8fba86 100644
> --- a/Documentation/filesystems/resctrl.rst
> +++ b/Documentation/filesystems/resctrl.rst
> @@ -215,6 +215,16 @@ related to allocation:
>   			# cat /sys/fs/resctrl/info/L3/io_alloc_cbm
>   			0=00ff;1=000f
>   
> +		Set each CBM to their minimum number of consecutive bits.
> +
> +		A special value "*" is required to represent all cache IDs.
> +		The user should specify the correct minimum stored in
> +		/sys/fs/resctrl/info/L3/min_cbm_bits.
> +
> +		Example::
> +
> +			# echo "*=0" > /sys/fs/resctrl/info/L3/io_alloc_cbm
> +
>   		When CDP is enabled "io_alloc_cbm" associated with the CDP_DATA and CDP_CODE
>   		resources may reflect the same values. For example, values read from and
>   		written to /sys/fs/resctrl/info/L3DATA/io_alloc_cbm may be reflected by
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 3792ab4819dc..44aea6b534e0 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -276,7 +276,7 @@ static void rdt_get_cdp_config(int level)
>   
>   static void rdt_set_io_alloc_capable(struct rdt_resource *r)
>   {
> -	r->cache.io_alloc_capable = true;
> +	r->cache.io_alloc.io_alloc_capable = true;
>   }
>   
>   static void rdt_get_cdp_l3_config(void)
> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> index b20e705606b8..0f051d848422 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -57,14 +57,19 @@ int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid)
>   		hw_dom = resctrl_to_arch_ctrl_dom(d);
>   		msr_param.res = NULL;
>   		for (t = 0; t < CDP_NUM_TYPES; t++) {
> -			cfg = &hw_dom->d_resctrl.staged_config[t];
> -			if (!cfg->have_new_ctrl)
> -				continue;
> -
> -			idx = resctrl_get_config_index(closid, t);
> -			if (cfg->new_ctrl == hw_dom->ctrl_val[idx])
> -				continue;
> -			hw_dom->ctrl_val[idx] = cfg->new_ctrl;
> +			if (resctrl_should_io_alloc_min_cbm(r)) {
> +				idx = resctrl_get_config_index(closid, t);
> +				hw_dom->ctrl_val[idx] = apply_io_alloc_min_cbm(r);
> +			} else {
> +				cfg = &hw_dom->d_resctrl.staged_config[t];
> +				if (!cfg->have_new_ctrl)
> +					continue;
> +
> +				idx = resctrl_get_config_index(closid, t);
> +				if (cfg->new_ctrl == hw_dom->ctrl_val[idx])
> +					continue;
> +				hw_dom->ctrl_val[idx] = cfg->new_ctrl;
> +			}
>   
>   			if (!msr_param.res) {
>   				msr_param.low = idx;
> @@ -123,7 +128,7 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable)
>   {
>   	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>   
> -	if (hw_res->r_resctrl.cache.io_alloc_capable &&
> +	if (hw_res->r_resctrl.cache.io_alloc.io_alloc_capable &&
>   	    hw_res->sdciae_enabled != enable) {
>   		_resctrl_sdciae_enable(r, enable);
>   		hw_res->sdciae_enabled = enable;
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index d2f9a4f2d887..a36b9055a1be 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -688,7 +688,7 @@ int resctrl_io_alloc_show(struct kernfs_open_file *of, struct seq_file *seq, voi
>   
>   	mutex_lock(&rdtgroup_mutex);
>   
> -	if (r->cache.io_alloc_capable) {
> +	if (r->cache.io_alloc.io_alloc_capable) {
>   		if (resctrl_arch_get_io_alloc_enabled(r))
>   			seq_puts(seq, "enabled\n");
>   		else
> @@ -712,13 +712,31 @@ static bool resctrl_io_alloc_closid_supported(u32 io_alloc_closid)
>   	return io_alloc_closid < closids_supported();
>   }
>   
> +/**
> + * resctrl_sync_cdp_peer - Mirror staged configuration to the CDP peer type
> + *
> + * @s: resctrl schema
> + * @d: resctrl control domain
> + *
> + * The caller must ensure CDP is enabled for the resource and must be
> + * called under the cpu hotplug lock and rdtgroup mutex
> + */
> +static void resctrl_sync_cdp_peer(struct resctrl_schema *s, struct rdt_ctrl_domain *d)
> +{
> +	enum resctrl_conf_type peer_type;
> +
> +	peer_type = resctrl_peer_type(s->conf_type);
> +	memcpy(&d->staged_config[peer_type],
> +	       &d->staged_config[s->conf_type],
> +	       sizeof(d->staged_config[0]));
> +}
> +
>   /*
>    * Initialize io_alloc CLOSID cache resource CBM with all usable (shared
>    * and unused) cache portions.
>    */
>   static int resctrl_io_alloc_init_cbm(struct resctrl_schema *s, u32 closid)
>   {
> -	enum resctrl_conf_type peer_type;
>   	struct rdt_resource *r = s->res;
>   	struct rdt_ctrl_domain *d;
>   	int ret;
> @@ -731,11 +749,8 @@ static int resctrl_io_alloc_init_cbm(struct resctrl_schema *s, u32 closid)
>   
>   	/* Keep CDP_CODE and CDP_DATA of io_alloc CLOSID's CBM in sync. */
>   	if (resctrl_arch_get_cdp_enabled(r->rid)) {
> -		peer_type = resctrl_peer_type(s->conf_type);
>   		list_for_each_entry(d, &s->res->ctrl_domains, hdr.list)
> -			memcpy(&d->staged_config[peer_type],
> -			       &d->staged_config[s->conf_type],
> -			       sizeof(d->staged_config[0]));
> +			resctrl_sync_cdp_peer(s, d);
>   	}
>   
>   	ret = resctrl_arch_update_domains(r, closid);
> @@ -903,49 +918,103 @@ int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq,
>   	return ret;
>   }
>   
> +/**
> + * parse_domain_cbm - Parse "domain=cbm" and return either side
> + *
> + * @line_ptr: Pointer to the input string
> + * @out_val: Output from parsed value (either domain ID or CDM)
> + * @which: Selector for which side to parse
> + *
> + * It is assumed that *line_ptr and *out_val are valid.
> + *
> + * Return: 0 on success and advance *line_ptr to point past the
> + * delimiter, EINVAL on parsing error
> + */
> +static inline int parse_domain_cbm(char **line_ptr, unsigned long *out_val,
> +				   enum resctrl_kv_sel which)
> +{
> +	char *rhs, *lhs, *tok;
> +	unsigned int base;
> +
> +	rhs = *line_ptr;
> +
> +	lhs = strsep(&rhs, "=");
> +	if (!lhs || !rhs)
> +		goto err;
> +
> +	if (which == RDT_PARSE_DOMAIN) {
> +		tok = lhs;
> +		base = 10;
> +	} else {
> +		tok = rhs;
> +		base = 16;
> +	}
> +	tok = strim(tok);
> +
> +	if (kstrtoul(tok, base, out_val))
> +		goto err;
> +
> +	*line_ptr = rhs;
> +	return 0;
> +err:
> +	rdt_last_cmd_puts("Invalid domain=cbm: missing '=' or non-numeric value\n");
> +	return -EINVAL;
> +}
> +
>   static int resctrl_io_alloc_parse_line(char *line,  struct rdt_resource *r,
>   				       struct resctrl_schema *s, u32 closid)
>   {
> -	enum resctrl_conf_type peer_type;
>   	struct rdt_parse_data data;
>   	struct rdt_ctrl_domain *d;
> -	char *dom = NULL, *id;
> -	unsigned long dom_id;
> +	char *cbm = NULL;
> +	unsigned long out_val;
> +	int ret;
>   
> -next:
> -	if (!line || line[0] == '\0')
> -		return 0;
> +	if (line[0] == '*') {
> +		ret = parse_domain_cbm(&line, &out_val, RDT_PARSE_CBM);
> +		if (ret)
> +			return ret;
>   
> -	dom = strsep(&line, ";");
> -	id = strsep(&dom, "=");
> -	if (!dom || kstrtoul(id, 10, &dom_id)) {
> -		rdt_last_cmd_puts("Missing '=' or non-numeric domain\n");
> +		if (out_val == r->cache.min_cbm_bits) {
> +			r->cache.io_alloc.io_alloc_min_cbm = true;
> +			return 0;
> +		}
> +		rdt_last_cmd_puts("Invalid io_alloc min CBM\n");
>   		return -EINVAL;
>   	}
>   
> -	dom = strim(dom);
> -	list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
> -		if (d->hdr.id == dom_id) {
> -			data.buf = dom;
> -			data.mode = RDT_MODE_SHAREABLE;
> -			data.closid = closid;
> -			if (parse_cbm(&data, s, d))
> -				return -EINVAL;
> -			/*
> -			 * Keep io_alloc CLOSID's CBM of CDP_CODE and CDP_DATA
> -			 * in sync.
> -			 */
> -			if (resctrl_arch_get_cdp_enabled(r->rid)) {
> -				peer_type = resctrl_peer_type(s->conf_type);
> -				memcpy(&d->staged_config[peer_type],
> -				       &d->staged_config[s->conf_type],
> -				       sizeof(d->staged_config[0]));
> +	while (line && line[0] != '\0') {
> +		bool found = false;
> +
> +		cbm = strsep(&line, ";");
> +		ret = parse_domain_cbm(&cbm, &out_val, RDT_PARSE_DOMAIN);
> +		if (ret)
> +			return ret;
> +
> +		cbm = strim(cbm);
> +		list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
> +			if (d->hdr.id == out_val) {
> +				data.buf = cbm;
> +				data.mode = RDT_MODE_SHAREABLE;
> +				data.closid = closid;
> +				if (parse_cbm(&data, s, d))
> +					return -EINVAL;
> +				/*
> +				 * Keep io_alloc CLOSID's CBM of CDP_CODE and CDP_DATA
> +				 * in sync.
> +				 */
> +				if (resctrl_arch_get_cdp_enabled(r->rid))
> +					resctrl_sync_cdp_peer(s, d);
> +				found = true;
> +				break;
>   			}
> -			goto next;
>   		}
> +
> +		if (!found)
> +			return -EINVAL;
>   	}
>   
> -	return -EINVAL;
> +	return 0;
>   }

Can't we just simplify to this patch without adding new data structures?

diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
index f7608121f5e3..d113d8a3ee02 100644
--- a/fs/resctrl/ctrlmondata.c
+++ b/fs/resctrl/ctrlmondata.c
@@ -874,6 +874,7 @@ static int resctrl_io_alloc_parse_line(char *line, 
struct rdt_resource *r,
         struct rdt_ctrl_domain *d;
         char *dom = NULL, *id;
         unsigned long dom_id;
+       bool update_all;

  next:
         if (!line || line[0] == '\0')
@@ -881,14 +882,18 @@ static int resctrl_io_alloc_parse_line(char *line, 
  struct rdt_resource *r,

         dom = strsep(&line, ";");
         id = strsep(&dom, "=");
-       if (!dom || kstrtoul(id, 10, &dom_id)) {
+       update_all = false;
+
+       if (id && *id == '*') {
+               update_all = true;
+       } else if (!dom || kstrtoul(id, 10, &dom_id)) {
                 rdt_last_cmd_puts("Missing '=' or non-numeric domain\n");
                 return -EINVAL;
         }

         dom = strim(dom);
         list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
-               if (d->hdr.id == dom_id) {
+               if (update_all || (d->hdr.id == dom_id)) {
                         data.buf = dom;
                         data.mode = RDT_MODE_SHAREABLE;
                         data.closid = closid;


What do you think?

thanks
Babu

On Thu, Dec 18, 2025 at 03:32:56PM -0600, Moger, Babu wrote:
> Can't we just simplify to this patch without adding new data structures?

Hi Babu,

Indeed, we certainly can.

> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index f7608121f5e3..d113d8a3ee02 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -874,6 +874,7 @@ static int resctrl_io_alloc_parse_line(char *line,
> struct rdt_resource *r,
>         struct rdt_ctrl_domain *d;
>         char *dom = NULL, *id;
>         unsigned long dom_id;
> +       bool update_all;
> 
>  next:
>         if (!line || line[0] == '\0')
> @@ -881,14 +882,18 @@ static int resctrl_io_alloc_parse_line(char *line,
> struct rdt_resource *r,
> 
>         dom = strsep(&line, ";");
>         id = strsep(&dom, "=");
> -       if (!dom || kstrtoul(id, 10, &dom_id)) {
> +       update_all = false;
> +
> +       if (id && *id == '*') {
> +               update_all = true;
> +       } else if (!dom || kstrtoul(id, 10, &dom_id)) {
>                 rdt_last_cmd_puts("Missing '=' or non-numeric domain\n");
>                 return -EINVAL;
>         }
> 
>         dom = strim(dom);
>         list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
> -               if (d->hdr.id == dom_id) {
> +               if (update_all || (d->hdr.id == dom_id)) {
>                         data.buf = dom;
>                         data.mode = RDT_MODE_SHAREABLE;
>                         data.closid = closid;

The above is suitable. I will prepare a follow-up series once our wider
discussion has concluded and we have reached a consensus on the
architectural direction.


Kind regards,
-- 
Aaron Tomlin

> +       if (id && *id == '*') {

That will accept arbitrary chars after the "*".

	if (id && !strcmp(id, "*"))

> What do you think?

Why is io_alloc special? The same simple change could apply to parse_line() to allow
setting all domains in a resource to be set to the same value:

# echo "L3:*=fff" > schemata

# echo "MB:*=50" > schemata

-Tony

Hi Tony,

On 12/18/25 1:49 PM, Luck, Tony wrote:
>> +       if (id && *id == '*') {
> 
> That will accept arbitrary chars after the "*".
> 
> 	if (id && !strcmp(id, "*"))
> 
>> What do you think?
> 
> Why is io_alloc special? The same simple change could apply to parse_line() to allow
> setting all domains in a resource to be set to the same value:
> 
> # echo "L3:*=fff" > schemata
> 
> # echo "MB:*=50" > schemata

If I remember correctly the idea was to limit this feature to io_alloc to avoid needing
to deal with L2 asymmetric domains [1].

Reinette

[1] https://lore.kernel.org/lkml/SJ1PR11MB60833A27A1B8057CDDFB1B2BFCCFA@SJ1PR11MB6083.namprd11.prod.outlook.com/

On Thu, Dec 18, 2025 at 02:59:59PM -0800, Reinette Chatre wrote:
> If I remember correctly the idea was to limit this feature to io_alloc to
> avoid needing to deal with L2 asymmetric domains [1].

Hi Reinette,

You are quite right; I am in complete agreement with your assessment. The
primary intention behind limiting the scope to io_alloc was indeed to avoid
the complexities associated with L2 asymmetric domains.

Are we all in alignment to focus this feature entirely on io_alloc for the
time being? If so, I will be pleased to prepare a follow-up series that
reflects this consensus once our wider discussion has concluded.

> 
> [1] https://lore.kernel.org/lkml/SJ1PR11MB60833A27A1B8057CDDFB1B2BFCCFA@SJ1PR11MB6083.namprd11.prod.outlook.com/
> 

Kind regards,
-- 
Aaron Tomlin

Hi Aaron and Tony,

On 12/18/25 4:44 PM, Aaron Tomlin wrote:
> On Thu, Dec 18, 2025 at 02:59:59PM -0800, Reinette Chatre wrote:
>> If I remember correctly the idea was to limit this feature to io_alloc to
>> avoid needing to deal with L2 asymmetric domains [1].
> 
> Hi Reinette,
> 
> You are quite right; I am in complete agreement with your assessment. The
> primary intention behind limiting the scope to io_alloc was indeed to avoid
> the complexities associated with L2 asymmetric domains.
> 
> Are we all in alignment to focus this feature entirely on io_alloc for the
> time being? If so, I will be pleased to prepare a follow-up series that

From my side I am ok to limit this to io_alloc. Of course, this does not prevent
cache allocation from supporting this syntax in the future.

Tony: did you perhaps imply with examples in [2] that '*' only be supported by
L3 and MB, not L2? Can it be guaranteed that L3 will never be asymmetric? Not that
it is a blocker though, as discussed earlier there are ways [3] to support
'*' when domains may be asymmetric. That proposal is only reasonable if considering the
feature as "let user set same CBM on all domains" that just happens to support the "reset
to min" use case for L3 io_alloc. I assume even on asymmetric domains the min would be the
same? If there is indeed a requirement to support "reset to defaults" for general cache
allocation then this feature would not work for asymmetric domains as you highlighted in [4].
Although, a "reset to defaults" for cache allocation use case may be better handled by
removing and recreating the resource group since the defaults will take into account
any exclusive allocations?

Reinette


> reflects this consensus once our wider discussion has concluded.
> 
>>
>> [1] https://lore.kernel.org/lkml/SJ1PR11MB60833A27A1B8057CDDFB1B2BFCCFA@SJ1PR11MB6083.namprd11.prod.outlook.com/
[2] https://lore.kernel.org/all/SJ1PR11MB6083CCA2A7904E459B1AA35DFCA8A@SJ1PR11MB6083.namprd11.prod.outlook.com/
[3] https://lore.kernel.org/lkml/f4a043d2-9cb0-41c9-a45d-31f96fd007d5@amd.com/
[4] https://lore.kernel.org/lkml/SJ1PR11MB60836AB4270419338FBB4D1EFCCEA@SJ1PR11MB6083.namprd11.prod.outlook.com/

On Fri, Dec 19, 2025 at 10:28:32AM -0800, Reinette Chatre wrote:
> From my side I am ok to limit this to io_alloc. Of course, this does not
> prevent cache allocation from supporting this syntax in the future.

Hi Reinette,

Understood.

I have taken the feedback on board and shall prepare a follow-up patch
that reflects this narrower focus, alongside the removal of the
aforementioned refactoring patch.


Kind regards,
-- 
Aaron Tomlin

On Fri, Dec 19, 2025 at 10:28:32AM -0800, Reinette Chatre wrote:
> Hi Aaron and Tony,
> 
> On 12/18/25 4:44 PM, Aaron Tomlin wrote:
> > On Thu, Dec 18, 2025 at 02:59:59PM -0800, Reinette Chatre wrote:
> >> If I remember correctly the idea was to limit this feature to io_alloc to
> >> avoid needing to deal with L2 asymmetric domains [1].
> > 
> > Hi Reinette,
> > 
> > You are quite right; I am in complete agreement with your assessment. The
> > primary intention behind limiting the scope to io_alloc was indeed to avoid
> > the complexities associated with L2 asymmetric domains.
> > 
> > Are we all in alignment to focus this feature entirely on io_alloc for the
> > time being? If so, I will be pleased to prepare a follow-up series that
> 
> From my side I am ok to limit this to io_alloc. Of course, this does not prevent
> cache allocation from supporting this syntax in the future.
> 
> Tony: did you perhaps imply with examples in [2] that '*' only be supported by
> L3 and MB, not L2? Can it be guaranteed that L3 will never be asymmetric? Not that
> it is a blocker though, as discussed earlier there are ways [3] to support

I'd forgotten the L2 asymmetry issue. If we wanted to enable "*" more
generally, then resctrl would have to limit it to symmetric resources
or to allow setting values that work for all domains in an asymmetric
resource).  But that seems more complexity in the kernel for something
than can easily be handled in user space. E.g. to reset L3 to ffff

# sed -n -e '/L3:/s/\(=[0-9a-f][0-9a-f]*\)/=ffff/gp' schemata > schemata

> '*' when domains may be asymmetric. That proposal is only reasonable if considering the
> feature as "let user set same CBM on all domains" that just happens to support the "reset
> to min" use case for L3 io_alloc. I assume even on asymmetric domains the min would be the
> same? If there is indeed a requirement to support "reset to defaults" for general cache
> allocation then this feature would not work for asymmetric domains as you highlighted in [4].

> Although, a "reset to defaults" for cache allocation use case may be better handled by
> removing and recreating the resource group since the defaults will take into account
> any exclusive allocations?

Removing the directory would free the RMID and allocate a new one when you
recreated it. Losing cache occupancy information completely, and disturbing
memory bandwidth monitoring. Also leaving the user to hunt down tasks
that were reassigned to the default CLOS and reassign them. So too many
side effects.
> 
> Reinette
> 
> 
> > reflects this consensus once our wider discussion has concluded.
> > 
> >>
> >> [1] https://lore.kernel.org/lkml/SJ1PR11MB60833A27A1B8057CDDFB1B2BFCCFA@SJ1PR11MB6083.namprd11.prod.outlook.com/
> [2] https://lore.kernel.org/all/SJ1PR11MB6083CCA2A7904E459B1AA35DFCA8A@SJ1PR11MB6083.namprd11.prod.outlook.com/
> [3] https://lore.kernel.org/lkml/f4a043d2-9cb0-41c9-a45d-31f96fd007d5@amd.com/
> [4] https://lore.kernel.org/lkml/SJ1PR11MB60836AB4270419338FBB4D1EFCCEA@SJ1PR11MB6083.namprd11.prod.outlook.com/

-Tony

Hi Tony,

On 12/19/25 12:42 PM, Luck, Tony wrote:
> On Fri, Dec 19, 2025 at 10:28:32AM -0800, Reinette Chatre wrote:
>> Hi Aaron and Tony,
>>
>> On 12/18/25 4:44 PM, Aaron Tomlin wrote:
>>> On Thu, Dec 18, 2025 at 02:59:59PM -0800, Reinette Chatre wrote:
>>>> If I remember correctly the idea was to limit this feature to io_alloc to
>>>> avoid needing to deal with L2 asymmetric domains [1].
>>>
>>> Hi Reinette,
>>>
>>> You are quite right; I am in complete agreement with your assessment. The
>>> primary intention behind limiting the scope to io_alloc was indeed to avoid
>>> the complexities associated with L2 asymmetric domains.
>>>
>>> Are we all in alignment to focus this feature entirely on io_alloc for the
>>> time being? If so, I will be pleased to prepare a follow-up series that
>>
>> From my side I am ok to limit this to io_alloc. Of course, this does not prevent
>> cache allocation from supporting this syntax in the future.
>>
>> Tony: did you perhaps imply with examples in [2] that '*' only be supported by
>> L3 and MB, not L2? Can it be guaranteed that L3 will never be asymmetric? Not that
>> it is a blocker though, as discussed earlier there are ways [3] to support
> 
> I'd forgotten the L2 asymmetry issue. If we wanted to enable "*" more
> generally, then resctrl would have to limit it to symmetric resources
> or to allow setting values that work for all domains in an asymmetric
> resource).  But that seems more complexity in the kernel for something
> than can easily be handled in user space. E.g. to reset L3 to ffff
> 
> # sed -n -e '/L3:/s/\(=[0-9a-f][0-9a-f]*\)/=ffff/gp' schemata > schemata

ack. 

Can I interpret this as you being ok to limit support for '*' syntax to io_alloc (for now?)?
As I understand the intended implementation discussed here will indeed just allow setting values
that will work for all domains ... all or nothing. So, if L3 does become asymmetric along the
way then the implementation does seem to remain reasonable. 


>> '*' when domains may be asymmetric. That proposal is only reasonable if considering the
>> feature as "let user set same CBM on all domains" that just happens to support the "reset
>> to min" use case for L3 io_alloc. I assume even on asymmetric domains the min would be the
>> same? If there is indeed a requirement to support "reset to defaults" for general cache
>> allocation then this feature would not work for asymmetric domains as you highlighted in [4].
> 
>> Although, a "reset to defaults" for cache allocation use case may be better handled by
>> removing and recreating the resource group since the defaults will take into account
>> any exclusive allocations?
> 
> Removing the directory would free the RMID and allocate a new one when you
> recreated it. Losing cache occupancy information completely, and disturbing
> memory bandwidth monitoring. Also leaving the user to hunt down tasks
> that were reassigned to the default CLOS and reassign them. So too many
> side effects.

I agree. Even so, without knowing more details about use case it is difficult to reason about
user space expectations here. This is regarding a "reset to defaults" use case that was raised in
[4]. Did you raise that concern based on insights into requests from users to support this?
I think we would need to create new syntax if resctrl needs to support such use case.

Reinette


>>
>> Reinette
>>
>>
>>> reflects this consensus once our wider discussion has concluded.
>>>
>>>>
>>>> [1] https://lore.kernel.org/lkml/SJ1PR11MB60833A27A1B8057CDDFB1B2BFCCFA@SJ1PR11MB6083.namprd11.prod.outlook.com/
>> [2] https://lore.kernel.org/all/SJ1PR11MB6083CCA2A7904E459B1AA35DFCA8A@SJ1PR11MB6083.namprd11.prod.outlook.com/
>> [3] https://lore.kernel.org/lkml/f4a043d2-9cb0-41c9-a45d-31f96fd007d5@amd.com/
>> [4] https://lore.kernel.org/lkml/SJ1PR11MB60836AB4270419338FBB4D1EFCCEA@SJ1PR11MB6083.namprd11.prod.outlook.com/
> 
> -Tony

On Fri, Dec 19, 2025 at 02:00:32PM -0800, Reinette Chatre wrote:
> Hi Tony,
> 
> On 12/19/25 12:42 PM, Luck, Tony wrote:
> > On Fri, Dec 19, 2025 at 10:28:32AM -0800, Reinette Chatre wrote:
> >> Hi Aaron and Tony,
> >>
> >> On 12/18/25 4:44 PM, Aaron Tomlin wrote:
> >>> On Thu, Dec 18, 2025 at 02:59:59PM -0800, Reinette Chatre wrote:
> >>>> If I remember correctly the idea was to limit this feature to io_alloc to
> >>>> avoid needing to deal with L2 asymmetric domains [1].
> >>>
> >>> Hi Reinette,
> >>>
> >>> You are quite right; I am in complete agreement with your assessment. The
> >>> primary intention behind limiting the scope to io_alloc was indeed to avoid
> >>> the complexities associated with L2 asymmetric domains.
> >>>
> >>> Are we all in alignment to focus this feature entirely on io_alloc for the
> >>> time being? If so, I will be pleased to prepare a follow-up series that
> >>
> >> From my side I am ok to limit this to io_alloc. Of course, this does not prevent
> >> cache allocation from supporting this syntax in the future.
> >>
> >> Tony: did you perhaps imply with examples in [2] that '*' only be supported by
> >> L3 and MB, not L2? Can it be guaranteed that L3 will never be asymmetric? Not that
> >> it is a blocker though, as discussed earlier there are ways [3] to support
> > 
> > I'd forgotten the L2 asymmetry issue. If we wanted to enable "*" more
> > generally, then resctrl would have to limit it to symmetric resources
> > or to allow setting values that work for all domains in an asymmetric
> > resource).  But that seems more complexity in the kernel for something
> > than can easily be handled in user space. E.g. to reset L3 to ffff
> > 
> > # sed -n -e '/L3:/s/\(=[0-9a-f][0-9a-f]*\)/=ffff/gp' schemata > schemata
> 
> ack. 
> 
> Can I interpret this as you being ok to limit support for '*' syntax to io_alloc (for now?)?
> As I understand the intended implementation discussed here will indeed just allow setting values
> that will work for all domains ... all or nothing. So, if L3 does become asymmetric along the
> way then the implementation does seem to remain reasonable. 

I still don't see a good reason for the kernel to handle any of this.

Having some resctrl control files accept wildcards while others don't
seems like a confusing interface. Is there something I'm missing that
makes io_alloc harder for users to handle than L3 or MB in schemata?

Babu's simpler implementation for io_alloc makes me more comfortable
with this. But I'm still unconvinced that wildcards must be handled
in kernel code.

> 
> >> '*' when domains may be asymmetric. That proposal is only reasonable if considering the
> >> feature as "let user set same CBM on all domains" that just happens to support the "reset
> >> to min" use case for L3 io_alloc. I assume even on asymmetric domains the min would be the
> >> same? If there is indeed a requirement to support "reset to defaults" for general cache
> >> allocation then this feature would not work for asymmetric domains as you highlighted in [4].
> > 
> >> Although, a "reset to defaults" for cache allocation use case may be better handled by
> >> removing and recreating the resource group since the defaults will take into account
> >> any exclusive allocations?
> > 
> > Removing the directory would free the RMID and allocate a new one when you
> > recreated it. Losing cache occupancy information completely, and disturbing
> > memory bandwidth monitoring. Also leaving the user to hunt down tasks
> > that were reassigned to the default CLOS and reassign them. So too many
> > side effects.
> 
> I agree. Even so, without knowing more details about use case it is difficult to reason about
> user space expectations here. This is regarding a "reset to defaults" use case that was raised in
> [4]. Did you raise that concern based on insights into requests from users to support this?
> I think we would need to create new syntax if resctrl needs to support such use case.

I don't have any requests from users.

> 
> Reinette
> 
> 
> >>
> >> Reinette
> >>
> >>
> >>> reflects this consensus once our wider discussion has concluded.
> >>>
> >>>>
> >>>> [1] https://lore.kernel.org/lkml/SJ1PR11MB60833A27A1B8057CDDFB1B2BFCCFA@SJ1PR11MB6083.namprd11.prod.outlook.com/
> >> [2] https://lore.kernel.org/all/SJ1PR11MB6083CCA2A7904E459B1AA35DFCA8A@SJ1PR11MB6083.namprd11.prod.outlook.com/
> >> [3] https://lore.kernel.org/lkml/f4a043d2-9cb0-41c9-a45d-31f96fd007d5@amd.com/
> >> [4] https://lore.kernel.org/lkml/SJ1PR11MB60836AB4270419338FBB4D1EFCCEA@SJ1PR11MB6083.namprd11.prod.outlook.com/
> > 
> > -Tony
> 

-Tony

On Fri, Dec 19, 2025 at 03:05:34PM -0800, Luck, Tony wrote:
> I still don't see a good reason for the kernel to handle any of this.
> 
> Having some resctrl control files accept wildcards while others don't
> seems like a confusing interface. Is there something I'm missing that
> makes io_alloc harder for users to handle than L3 or MB in schemata?
> 
> Babu's simpler implementation for io_alloc makes me more comfortable
> with this. But I'm still unconvinced that wildcards must be handled
> in kernel code.

Hi Tony,

I appreciate your reservations regarding where the responsibility for such
abstractions should lie. However, I am of the view that forcing the user to
manually specify every domain for a global policy represents an unnecessary
hurdle that the kernel is uniquely positioned to clear.

I shall indeed employ Babu's suggestion in a follow-up patch for review, as
the resulting simplicity of the implementation should, I hope, alleviate
any concerns regarding the addition of technical debt.


Kind regards,
-- 
Aaron Tomlin

On 12/18/2025 3:49 PM, Luck, Tony wrote:
>> +       if (id && *id == '*') {
> 
> That will accept arbitrary chars after the "*".
> 
> 	if (id && !strcmp(id, "*"))

oh.ok. Thanks

> 
>> What do you think?
> 
> Why is io_alloc special? The same simple change could apply to parse_line() to allow
> setting all domains in a resource to be set to the same value:
> 

Yes. I am fine with that approach. Treat "*" equivalent for all the 
resources not just io_alloc.

> # echo "L3:*=fff" > schemata
> 
> # echo "MB:*=50" > schemata
> 
> -Tony
> 
>

On Thu, Dec 18, 2025 at 03:56:24PM -0600, Moger, Babu wrote:
> > Why is io_alloc special? The same simple change could apply to
> > parse_line() to allow setting all domains in a resource to be set to
> > the same value:

Hi Tony, Babu,

> Yes. I am fine with that approach. Treat "*" equivalent for all the
> resources not just io_alloc.

No. While I appreciate the desire for a more uniform interface, I would
strongly suggest that this particular change remain applicable only to
io_alloc for the time being. If I recall correctly, the architectural
requirements for other resources differ enough that the suggested approach
may not be universally applicable or appropriate in every case, no?


Kind regards,
-- 
Aaron Tomlin

Hi Aaron,

On 12/18/2025 6:31 PM, Aaron Tomlin wrote:
> On Thu, Dec 18, 2025 at 03:56:24PM -0600, Moger, Babu wrote:
>>> Why is io_alloc special? The same simple change could apply to
>>> parse_line() to allow setting all domains in a resource to be set to
>>> the same value:
> 
> Hi Tony, Babu,
> 
>> Yes. I am fine with that approach. Treat "*" equivalent for all the
>> resources not just io_alloc.
> 
> No. While I appreciate the desire for a more uniform interface, I would
> strongly suggest that this particular change remain applicable only to
> io_alloc for the time being. If I recall correctly, the architectural
> requirements for other resources differ enough that the suggested approach
> may not be universally applicable or appropriate in every case, no?
> 

Yes. That is correct. Lets do this only for io_alloc.

Reinette commented on the already.

https://lore.kernel.org/lkml/a76c2eee-2a64-42ab-b81c-90638321cd15@intel.com/

Thanks
Babu

Hi Aaron,

On 12/15/25 3:02 PM, Aaron Tomlin wrote:
> This patch introduces a special domain ID selector "*" for io_alloc_cbm

Kernel code and patches to kernel have a couple of style requirements that
are documented. It takes some getting used to but it is required that submissions
follow the kernel style in order to be considered for inclusion.
Documentation relevant to above are:
Documentation/process/submitting-patches.rst: Consider whole document but specifically
search for "This patch" for information related to above.
Documentation/process/maintainer-tip.rst: Consider whole document but specifically
consider "Changelog" relevant to this part.

> that allows setting the CBM of each cache domain to its minimum number
> of consecutive bits in a single operation. For example, writing "*=0" to

Is there a reason to limit this implementation to only support the minimum CBM?
This feature can easily enable a user to set identical CBM across all domains
without the CBM being required to be the minimum CBM, no?

> /sys/fs/resctrl/info/L3/io_alloc_cbm programs each domain's CBM to the
> hardware-defined minimum, greatly simplifying automation and management
> tasks. The user is required to specify the correct minimum stored in
> /sys/fs/resctrl/info/L3/min_cbm_bits.
> 
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
>  Documentation/filesystems/resctrl.rst     |  10 ++
>  arch/x86/kernel/cpu/resctrl/core.c        |   2 +-
>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  23 ++--
>  fs/resctrl/ctrlmondata.c                  | 141 ++++++++++++++++------
>  fs/resctrl/internal.h                     |  13 ++
>  fs/resctrl/rdtgroup.c                     |   2 +-
>  include/linux/resctrl.h                   |  30 ++++-
>  7 files changed, 174 insertions(+), 47 deletions(-)
> 
> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
> index 8c8ce678148a..9c128c8fba86 100644
> --- a/Documentation/filesystems/resctrl.rst
> +++ b/Documentation/filesystems/resctrl.rst
> @@ -215,6 +215,16 @@ related to allocation:
>  			# cat /sys/fs/resctrl/info/L3/io_alloc_cbm
>  			0=00ff;1=000f
>  
> +		Set each CBM to their minimum number of consecutive bits.
> +
> +		A special value "*" is required to represent all cache IDs.
> +		The user should specify the correct minimum stored in
> +		/sys/fs/resctrl/info/L3/min_cbm_bits.
> +
> +		Example::
> +
> +			# echo "*=0" > /sys/fs/resctrl/info/L3/io_alloc_cbm
> +
>  		When CDP is enabled "io_alloc_cbm" associated with the CDP_DATA and CDP_CODE
>  		resources may reflect the same values. For example, values read from and
>  		written to /sys/fs/resctrl/info/L3DATA/io_alloc_cbm may be reflected by
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 3792ab4819dc..44aea6b534e0 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -276,7 +276,7 @@ static void rdt_get_cdp_config(int level)
>  
>  static void rdt_set_io_alloc_capable(struct rdt_resource *r)
>  {
> -	r->cache.io_alloc_capable = true;
> +	r->cache.io_alloc.io_alloc_capable = true;
>  }
>  
>  static void rdt_get_cdp_l3_config(void)
> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> index b20e705606b8..0f051d848422 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -57,14 +57,19 @@ int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid)
>  		hw_dom = resctrl_to_arch_ctrl_dom(d);
>  		msr_param.res = NULL;
>  		for (t = 0; t < CDP_NUM_TYPES; t++) {
> -			cfg = &hw_dom->d_resctrl.staged_config[t];
> -			if (!cfg->have_new_ctrl)
> -				continue;
> -
> -			idx = resctrl_get_config_index(closid, t);
> -			if (cfg->new_ctrl == hw_dom->ctrl_val[idx])
> -				continue;
> -			hw_dom->ctrl_val[idx] = cfg->new_ctrl;
> +			if (resctrl_should_io_alloc_min_cbm(r)) {
> +				idx = resctrl_get_config_index(closid, t);
> +				hw_dom->ctrl_val[idx] = apply_io_alloc_min_cbm(r);
> +			} else {
> +				cfg = &hw_dom->d_resctrl.staged_config[t];
> +				if (!cfg->have_new_ctrl)
> +					continue;
> +
> +				idx = resctrl_get_config_index(closid, t);
> +				if (cfg->new_ctrl == hw_dom->ctrl_val[idx])
> +					continue;
> +				hw_dom->ctrl_val[idx] = cfg->new_ctrl;
> +			}
>  

No. resctrl_arch_update_domains() is a helper to just program configurations. Please avoid bleeding
feature specific handling into dedicated helpers. 

>  			if (!msr_param.res) {
>  				msr_param.low = idx;
> @@ -123,7 +128,7 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable)
>  {
>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>  
> -	if (hw_res->r_resctrl.cache.io_alloc_capable &&
> +	if (hw_res->r_resctrl.cache.io_alloc.io_alloc_capable &&
>  	    hw_res->sdciae_enabled != enable) {
>  		_resctrl_sdciae_enable(r, enable);
>  		hw_res->sdciae_enabled = enable;
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index d2f9a4f2d887..a36b9055a1be 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -688,7 +688,7 @@ int resctrl_io_alloc_show(struct kernfs_open_file *of, struct seq_file *seq, voi
>  
>  	mutex_lock(&rdtgroup_mutex);
>  
> -	if (r->cache.io_alloc_capable) {
> +	if (r->cache.io_alloc.io_alloc_capable) {
>  		if (resctrl_arch_get_io_alloc_enabled(r))
>  			seq_puts(seq, "enabled\n");
>  		else
> @@ -712,13 +712,31 @@ static bool resctrl_io_alloc_closid_supported(u32 io_alloc_closid)
>  	return io_alloc_closid < closids_supported();
>  }
>  
> +/**
> + * resctrl_sync_cdp_peer - Mirror staged configuration to the CDP peer type
> + *
> + * @s: resctrl schema
> + * @d: resctrl control domain
> + *
> + * The caller must ensure CDP is enabled for the resource and must be
> + * called under the cpu hotplug lock and rdtgroup mutex
> + */
> +static void resctrl_sync_cdp_peer(struct resctrl_schema *s, struct rdt_ctrl_domain *d)
> +{
> +	enum resctrl_conf_type peer_type;
> +
> +	peer_type = resctrl_peer_type(s->conf_type);
> +	memcpy(&d->staged_config[peer_type],
> +	       &d->staged_config[s->conf_type],
> +	       sizeof(d->staged_config[0]));
> +}
> +
>  /*
>   * Initialize io_alloc CLOSID cache resource CBM with all usable (shared
>   * and unused) cache portions.
>   */
>  static int resctrl_io_alloc_init_cbm(struct resctrl_schema *s, u32 closid)
>  {
> -	enum resctrl_conf_type peer_type;
>  	struct rdt_resource *r = s->res;
>  	struct rdt_ctrl_domain *d;
>  	int ret;
> @@ -731,11 +749,8 @@ static int resctrl_io_alloc_init_cbm(struct resctrl_schema *s, u32 closid)
>  
>  	/* Keep CDP_CODE and CDP_DATA of io_alloc CLOSID's CBM in sync. */
>  	if (resctrl_arch_get_cdp_enabled(r->rid)) {
> -		peer_type = resctrl_peer_type(s->conf_type);
>  		list_for_each_entry(d, &s->res->ctrl_domains, hdr.list)
> -			memcpy(&d->staged_config[peer_type],
> -			       &d->staged_config[s->conf_type],
> -			       sizeof(d->staged_config[0]));
> +			resctrl_sync_cdp_peer(s, d);
>  	}
>  
>  	ret = resctrl_arch_update_domains(r, closid);
> @@ -903,49 +918,103 @@ int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq,
>  	return ret;
>  }
>  
> +/**
> + * parse_domain_cbm - Parse "domain=cbm" and return either side
> + *
> + * @line_ptr: Pointer to the input string
> + * @out_val: Output from parsed value (either domain ID or CDM)
> + * @which: Selector for which side to parse
> + *
> + * It is assumed that *line_ptr and *out_val are valid.
> + *
> + * Return: 0 on success and advance *line_ptr to point past the
> + * delimiter, EINVAL on parsing error
> + */
> +static inline int parse_domain_cbm(char **line_ptr, unsigned long *out_val,
> +				   enum resctrl_kv_sel which)
> +{
> +	char *rhs, *lhs, *tok;
> +	unsigned int base;
> +
> +	rhs = *line_ptr;
> +
> +	lhs = strsep(&rhs, "=");
> +	if (!lhs || !rhs)
> +		goto err;
> +
> +	if (which == RDT_PARSE_DOMAIN) {
> +		tok = lhs;
> +		base = 10;
> +	} else {
> +		tok = rhs;
> +		base = 16;
> +	}
> +	tok = strim(tok);
> +
> +	if (kstrtoul(tok, base, out_val))
> +		goto err;
> +
> +	*line_ptr = rhs;
> +	return 0;
> +err:
> +	rdt_last_cmd_puts("Invalid domain=cbm: missing '=' or non-numeric value\n");
> +	return -EINVAL;
> +}

Why is this new parser needed? 

> +
>  static int resctrl_io_alloc_parse_line(char *line,  struct rdt_resource *r,
>  				       struct resctrl_schema *s, u32 closid)
>  {
> -	enum resctrl_conf_type peer_type;
>  	struct rdt_parse_data data;
>  	struct rdt_ctrl_domain *d;
> -	char *dom = NULL, *id;
> -	unsigned long dom_id;
> +	char *cbm = NULL;
> +	unsigned long out_val;
> +	int ret;
>  
> -next:
> -	if (!line || line[0] == '\0')
> -		return 0;
> +	if (line[0] == '*') {
> +		ret = parse_domain_cbm(&line, &out_val, RDT_PARSE_CBM);
> +		if (ret)
> +			return ret;
>  
> -	dom = strsep(&line, ";");
> -	id = strsep(&dom, "=");
> -	if (!dom || kstrtoul(id, 10, &dom_id)) {
> -		rdt_last_cmd_puts("Missing '=' or non-numeric domain\n");
> +		if (out_val == r->cache.min_cbm_bits) {

This does not look right. If I understand correctly out_val contains the user
provided value/CBM, that is if user provided "*=3" then out_val contains "3". This value
is in turn compared against the *number of bits* required. First, if indeed just forcing
user to set the minimum CBM (which I already mentioned seems restrictive) then if out_val
is "3" and min_cbm_bits is "2" then this check will fail while the user indeed did set
the minimum CBM, no? Additionally, user providing value of 6 or C or any
other value with two consecutive bits set would also be a valid value as a "minimum", no?

This additional parsing and checks adds unnecessary complexity. Just keep original parsing
that relies on parse_cbm() that calls cbm_validate() that ensures the provided CBM is valid for
this resource while taking min_cbm_bits into account.


> +			r->cache.io_alloc.io_alloc_min_cbm = true;
> +			return 0;

If I understand correctly the parsing discards user provided value. If this check thinks it needs to
program the min CBM it appears to program the min CBM via a backdoor in the config write helper that
circumvents the flow used by everything else in resctrl that first stages the values and then calls
the config helper to write to hardware. This is not acceptable. Please integrate any new feature into
existing flows that follow existing patterns. Punching a feature specific back door via this new state
(io_alloc_min_cbm) is not appropriate.

> +		}
> +		rdt_last_cmd_puts("Invalid io_alloc min CBM\n");
>  		return -EINVAL;
>  	}
>  
> -	dom = strim(dom);
> -	list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
> -		if (d->hdr.id == dom_id) {
> -			data.buf = dom;
> -			data.mode = RDT_MODE_SHAREABLE;
> -			data.closid = closid;
> -			if (parse_cbm(&data, s, d))
> -				return -EINVAL;
> -			/*
> -			 * Keep io_alloc CLOSID's CBM of CDP_CODE and CDP_DATA
> -			 * in sync.
> -			 */
> -			if (resctrl_arch_get_cdp_enabled(r->rid)) {
> -				peer_type = resctrl_peer_type(s->conf_type);
> -				memcpy(&d->staged_config[peer_type],
> -				       &d->staged_config[s->conf_type],
> -				       sizeof(d->staged_config[0]));
> +	while (line && line[0] != '\0') {
> +		bool found = false;
> +
> +		cbm = strsep(&line, ";");
> +		ret = parse_domain_cbm(&cbm, &out_val, RDT_PARSE_DOMAIN);
> +		if (ret)
> +			return ret;
> +
> +		cbm = strim(cbm);
> +		list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
> +			if (d->hdr.id == out_val) {
> +				data.buf = cbm;
> +				data.mode = RDT_MODE_SHAREABLE;
> +				data.closid = closid;
> +				if (parse_cbm(&data, s, d))
> +					return -EINVAL;
> +				/*
> +				 * Keep io_alloc CLOSID's CBM of CDP_CODE and CDP_DATA
> +				 * in sync.
> +				 */
> +				if (resctrl_arch_get_cdp_enabled(r->rid))
> +					resctrl_sync_cdp_peer(s, d);
> +				found = true;
> +				break;
>  			}
> -			goto next;
>  		}
> +
> +		if (!found)
> +			return -EINVAL;
>  	}
>  
> -	return -EINVAL;
> +	return 0;
>  }

I agree with Babu [1]. This is a lot of complexity just to support handling of an additional character.
resctrl does a lot of parsing of per-domain user space input and the current implementation follows the
same pattern used throughout resctrl. Introducing a new pattern and parser unique to IO alloc feature adds
unnecessary complexity. Simple and consistent is better. I considered your response [2] but to me this
code is not more readable than a simple implementation built on top of existing, familiar, and consistent
patterns. Another motivation from [2] is that this is easier to maintain but I fail to see that. About the
motivation from [2] for "possible enhancements down the line" ... when/if need for an enhancement
is required then code can be refactored to support it.

On a workflow note: please allow for discussions about your submission. Taking a long time to respond and then
responding that you disagree with feedback immediately followed by a new version of the series that does not
address feedback is not productive.

Reinette

[1] https://lore.kernel.org/lkml/239e3a89-1075-439b-bf1c-d2be5be5c30c@amd.com/
[2] https://lore.kernel.org/lkml/5xlapgkp5bktan7xhy6l6b7c4qgeje7weu4cy6cbuux5npwijo@lhf5uvtuns5k/

On Tue, Dec 16, 2025 at 09:53:20PM -0800, Reinette Chatre wrote:
Hi Reinette,

> Kernel code and patches to kernel have a couple of style requirements that
> are documented. It takes some getting used to but it is required that submissions
> follow the kernel style in order to be considered for inclusion.
> Documentation relevant to above are:
> Documentation/process/submitting-patches.rst: Consider whole document but specifically
> search for "This patch" for information related to above.
> Documentation/process/maintainer-tip.rst: Consider whole document but specifically
> consider "Changelog" relevant to this part.

Thank you for the guidance regarding the kernel style and submission requirements.

While I do have experience in kernel development, I appreciate the reminder
that strict adherence to the documented process is paramount. I will
carefully re-read the specified sections to ensure full compliance with the
expected standards.

> Is there a reason to limit this implementation to only support the minimum CBM?
> This feature can easily enable a user to set identical CBM across all domains
> without the CBM being required to be the minimum CBM, no?

You are quite right to raise this point.

Indeed, supporting arbitrary identical CBM values across domains was the
original intent of this work. I appreciate the suggestion and fully agree
that the implementation should be expanded to allow for this additional
flexibility, rather than being restricted to the minimum CBM value.

> No. resctrl_arch_update_domains() is a helper to just program
> configurations. Please avoid bleeding feature specific handling into
> dedicated helpers. 

Acknowledged.

> Why is this new parser needed? 

It can be dropped. Kindly ignore. 

> This does not look right. If I understand correctly out_val contains the
> user provided value/CBM, that is if user provided "*=3" then out_val
> contains "3". This value is in turn compared against the *number of bits*
> required. First, if indeed just forcing user to set the minimum CBM
> (which I already mentioned seems restrictive) then if out_val is "3" and
> min_cbm_bits is "2" then this check will fail while the user indeed did
> set the minimum CBM, no? Additionally, user providing value of 6 or C or
> any other value with two consecutive bits set would also be a valid value
> as a "minimum", no?

You are quite right to point this out; I appreciate the thorough review of
the validation logic. My original intention with this implementation was to
provide a mechanism to "reset" the configuration to the minimum supported
CBM only. However, the restriction itself is unnecessary.

> This additional parsing and checks adds unnecessary complexity. Just keep
> original parsing that relies on parse_cbm() that calls cbm_validate()
> that ensures the provided CBM is valid for this resource while taking
> min_cbm_bits into account.

Acknowledged.

> I agree with Babu [1]. This is a lot of complexity just to support
> handling of an additional character. resctrl does a lot of parsing of
> per-domain user space input and the current implementation follows the
> same pattern used throughout resctrl. Introducing a new pattern and
> parser unique to IO alloc feature adds unnecessary complexity. Simple and
> consistent is better. I considered your response [2] but to me this code
> is not more readable than a simple implementation built on top of
> existing, familiar, and consistent patterns. Another motivation from [2]
> is that this is easier to maintain but I fail to see that. About the
> motivation from [2] for "possible enhancements down the line" ... when/if
> need for an enhancement is required then code can be refactored to
> support it.

Acknowledged.

> On a workflow note: please allow for discussions about your submission.
> Taking a long time to respond and then responding that you disagree with
> feedback immediately followed by a new version of the series that does
> not address feedback is not productive.

Understood.


Kind regards,
-- 
Aaron Tomlin