All of lore.kernel.org
 help / color / mirror / Atom feed
* [v3 PATCH 0/1] x86/resctrl: Add "*" shorthand to set io_alloc CBM for all domains
@ 2025-12-31  2:35 Aaron Tomlin
  2025-12-31  2:35 ` [v3 PATCH 1/1] " Aaron Tomlin
  0 siblings, 1 reply; 6+ messages in thread
From: Aaron Tomlin @ 2025-12-31  2:35 UTC (permalink / raw)
  To: tony.luck, reinette.chatre, Dave.Martin, james.morse, babu.moger,
	tglx, mingo, bp, dave.hansen
  Cc: dave.martin, sean, linux-kernel

Hi Babu, Tony, Reinette,

This patch introduces a wildcard domain ID selector "*" for the
io_alloc_cbm interface. It allows a user to update the Capacity Bitmask
(CBM) across all cache domains in a single operation.

Currently, configuring io_alloc_cbm requires an explicit ID for each
domain, which is cumbersome on systems with high core counts and numerous
cache clusters. Supporting a wildcard selector simplifies automation and
management tasks.

For example, a user can now write "*=0" to the io_alloc_cbm file to program
every domain to the hardware-defined minimum CBM. Note that the value
provided must still adhere to the constraints defined in the resource's
min_cbm_bits.

Please let me know your thoughts.


Changes since v2 [1]:
 - Dropped return -EINVAL for a missing seq_show implementation
   (Reinette Chatre)
 - Dropped helpers to check io_alloc support and enabled state
   (Reinette Chatre)
 - Removed additional complexity (Babu Moger)
 - Introduced the "*" wildcard for io_alloc_cbm to allow updating all
   cache domains (Reinette Chatre)
 - Replaced goto-based line parsing with a while loop to support
   multi-domain and wildcard iterations
 - Replaced memcpy() with direct structure assignment

Changes since v1 [2]:
 - Updated each helper for consistency (Babu Moger)
 - Refactored the loop logic in function resctrl_io_alloc_parse_line()
   to improve readability
 - Added inline keyword to each helper
 - Added inline keyword to function parse_domain_cbm()

[1]: https://lore.kernel.org/lkml/20251215230257.1798865-1-atomlin@atomlin.com/
[2]: https://lore.kernel.org/lkml/20251126171653.1004321-1-atomlin@atomlin.com/

Aaron Tomlin (1):
  x86/resctrl: Add "*" shorthand to set io_alloc CBM for all domains

 Documentation/filesystems/resctrl.rst |  8 ++++
 fs/resctrl/ctrlmondata.c              | 60 ++++++++++++++-------------
 2 files changed, 40 insertions(+), 28 deletions(-)

-- 
2.51.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [v3 PATCH 1/1] x86/resctrl: Add "*" shorthand to set io_alloc CBM for all domains
  2025-12-31  2:35 [v3 PATCH 0/1] x86/resctrl: Add "*" shorthand to set io_alloc CBM for all domains Aaron Tomlin
@ 2025-12-31  2:35 ` Aaron Tomlin
  2026-01-06 16:49   ` Babu Moger
  2026-01-08 18:45   ` Reinette Chatre
  0 siblings, 2 replies; 6+ messages in thread
From: Aaron Tomlin @ 2025-12-31  2:35 UTC (permalink / raw)
  To: tony.luck, reinette.chatre, Dave.Martin, james.morse, babu.moger,
	tglx, mingo, bp, dave.hansen
  Cc: dave.martin, sean, linux-kernel

Introduce a wildcard domain ID selector "*" for the io_alloc_cbm
interface. This allows a user to update the Capacity Bitmask (CBM)
across all cache domains in a single operation.

Currently, configuring io_alloc_cbm requires an explicit ID for each
domain, which is cumbersome on systems with high core counts and
numerous cache clusters. Supporting a wildcard selector simplifies
automation and management tasks.

For example, a user can now write "*=0" to the io_alloc_cbm file to
program every domain to the hardware-defined minimum CBM. Note that the
value provided must still adhere to the constraints defined in the
resource's min_cbm_bits.

Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 Documentation/filesystems/resctrl.rst |  8 ++++
 fs/resctrl/ctrlmondata.c              | 60 ++++++++++++++-------------
 2 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
index 8c8ce678148a..e4d92c865e44 100644
--- a/Documentation/filesystems/resctrl.rst
+++ b/Documentation/filesystems/resctrl.rst
@@ -215,6 +215,14 @@ related to allocation:
 			# cat /sys/fs/resctrl/info/L3/io_alloc_cbm
 			0=00ff;1=000f
 
+		Set each CBM to a specified value.
+
+		A special value "*" is required to represent all cache IDs.
+
+		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/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
index b2d178d3556e..7b3119f91b9d 100644
--- a/fs/resctrl/ctrlmondata.c
+++ b/fs/resctrl/ctrlmondata.c
@@ -873,41 +873,45 @@ 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')
-		return 0;
+	while (line && line[0] != '\0') {
+		update_all = false;
+		dom = strsep(&line, ";");
+		id = strsep(&dom, "=");
 
-	dom = strsep(&line, ";");
-	id = strsep(&dom, "=");
-	if (!dom || kstrtoul(id, 10, &dom_id)) {
-		rdt_last_cmd_puts("Missing '=' or non-numeric domain\n");
-		return -EINVAL;
-	}
+		if (id && !strcmp(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) {
-			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]));
+		dom = strim(dom);
+		list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
+			if (update_all || 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);
+					d->staged_config[peer_type] =
+						d->staged_config[s->conf_type];
+				}
+
+				if (!update_all)
+					break;
 			}
-			goto next;
 		}
 	}
 
-	return -EINVAL;
+	return 0;
 }
 
 ssize_t resctrl_io_alloc_cbm_write(struct kernfs_open_file *of, char *buf,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [v3 PATCH 1/1] x86/resctrl: Add "*" shorthand to set io_alloc CBM for all domains
  2025-12-31  2:35 ` [v3 PATCH 1/1] " Aaron Tomlin
@ 2026-01-06 16:49   ` Babu Moger
  2026-01-07  1:28     ` Aaron Tomlin
  2026-01-08 18:45   ` Reinette Chatre
  1 sibling, 1 reply; 6+ messages in thread
From: Babu Moger @ 2026-01-06 16:49 UTC (permalink / raw)
  To: Aaron Tomlin, tony.luck, reinette.chatre, Dave.Martin,
	james.morse, babu.moger, tglx, mingo, bp, dave.hansen
  Cc: sean, linux-kernel

Hi Aaron,

On 12/30/25 20:35, Aaron Tomlin wrote:
> Introduce a wildcard domain ID selector "*" for the io_alloc_cbm
> interface. This allows a user to update the Capacity Bitmask (CBM)
> across all cache domains in a single operation.
>
> Currently, configuring io_alloc_cbm requires an explicit ID for each
> domain, which is cumbersome on systems with high core counts and
> numerous cache clusters. Supporting a wildcard selector simplifies
> automation and management tasks.
>
> For example, a user can now write "*=0" to the io_alloc_cbm file to
> program every domain to the hardware-defined minimum CBM. Note that the
> value provided must still adhere to the constraints defined in the
> resource's min_cbm_bits.
>
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
>   Documentation/filesystems/resctrl.rst |  8 ++++
>   fs/resctrl/ctrlmondata.c              | 60 ++++++++++++++-------------
>   2 files changed, 40 insertions(+), 28 deletions(-)
>
> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
> index 8c8ce678148a..e4d92c865e44 100644
> --- a/Documentation/filesystems/resctrl.rst
> +++ b/Documentation/filesystems/resctrl.rst
> @@ -215,6 +215,14 @@ related to allocation:
>   			# cat /sys/fs/resctrl/info/L3/io_alloc_cbm
>   			0=00ff;1=000f
>   
> +		Set each CBM to a specified value.
> +
> +		A special value "*" is required to represent all cache IDs.
> +
> +		Example::
> +
> +			# echo "*=0" > /sys/fs/resctrl/info/L3/io_alloc_cbm

To be consistent add the output also.

  # cat /sys/fs/resctrl/info/L3/io_alloc_cbm
      0=0;1=0

> +
>   		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/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index b2d178d3556e..7b3119f91b9d 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -873,41 +873,45 @@ 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')
> -		return 0;
> +	while (line && line[0] != '\0') {
> +		update_all = false;
> +		dom = strsep(&line, ";");
> +		id = strsep(&dom, "=");
>   
> -	dom = strsep(&line, ";");
> -	id = strsep(&dom, "=");
> -	if (!dom || kstrtoul(id, 10, &dom_id)) {
> -		rdt_last_cmd_puts("Missing '=' or non-numeric domain\n");
> -		return -EINVAL;
> -	}
> +		if (id && !strcmp(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) {
> -			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]));
> +		dom = strim(dom);
> +		list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
> +			if (update_all || 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);
> +					d->staged_config[peer_type] =
> +						d->staged_config[s->conf_type];
> +				}
> +
> +				if (!update_all)
> +					break;
>   			}
> -			goto next;
>   		}
>   	}
>   
> -	return -EINVAL;
> +	return 0;
>   }

Tested the patch and looks good to me.

Thanks

Babu




>   
>   ssize_t resctrl_io_alloc_cbm_write(struct kernfs_open_file *of, char *buf,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [v3 PATCH 1/1] x86/resctrl: Add "*" shorthand to set io_alloc CBM for all domains
  2026-01-06 16:49   ` Babu Moger
@ 2026-01-07  1:28     ` Aaron Tomlin
  0 siblings, 0 replies; 6+ messages in thread
From: Aaron Tomlin @ 2026-01-07  1:28 UTC (permalink / raw)
  To: Babu Moger
  Cc: tony.luck, reinette.chatre, Dave.Martin, james.morse, babu.moger,
	tglx, mingo, bp, dave.hansen, sean, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 394 bytes --]

On Tue, Jan 06, 2026 at 10:49:02AM -0600, Babu Moger wrote:
Hi Babu,

> To be consistent add the output also.
> 
>  # cat /sys/fs/resctrl/info/L3/io_alloc_cbm
>      0=0;1=0

Acknowledged.

> Tested the patch and looks good to me.

Thank you. I shall prepare a follow-up patch, that incorporates the
suggested documentation amendments. 


Kind regards,
-- 
Aaron Tomlin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [v3 PATCH 1/1] x86/resctrl: Add "*" shorthand to set io_alloc CBM for all domains
  2025-12-31  2:35 ` [v3 PATCH 1/1] " Aaron Tomlin
  2026-01-06 16:49   ` Babu Moger
@ 2026-01-08 18:45   ` Reinette Chatre
  2026-01-26  3:30     ` Aaron Tomlin
  1 sibling, 1 reply; 6+ messages in thread
From: Reinette Chatre @ 2026-01-08 18:45 UTC (permalink / raw)
  To: Aaron Tomlin, tony.luck, Dave.Martin, james.morse, babu.moger,
	tglx, mingo, bp, dave.hansen
  Cc: sean, linux-kernel

Hi Aaron,

On 12/30/25 6:35 PM, Aaron Tomlin wrote:
> Introduce a wildcard domain ID selector "*" for the io_alloc_cbm
> interface. This allows a user to update the Capacity Bitmask (CBM)
> across all cache domains in a single operation.
> 
> Currently, configuring io_alloc_cbm requires an explicit ID for each
> domain, which is cumbersome on systems with high core counts and
> numerous cache clusters. Supporting a wildcard selector simplifies
> automation and management tasks.
> 
> For example, a user can now write "*=0" to the io_alloc_cbm file to
> program every domain to the hardware-defined minimum CBM. Note that the
> value provided must still adhere to the constraints defined in the
> resource's min_cbm_bits.
> 
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
>  Documentation/filesystems/resctrl.rst |  8 ++++
>  fs/resctrl/ctrlmondata.c              | 60 ++++++++++++++-------------
>  2 files changed, 40 insertions(+), 28 deletions(-)
> 
> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
> index 8c8ce678148a..e4d92c865e44 100644
> --- a/Documentation/filesystems/resctrl.rst
> +++ b/Documentation/filesystems/resctrl.rst
> @@ -215,6 +215,14 @@ related to allocation:
>  			# cat /sys/fs/resctrl/info/L3/io_alloc_cbm
>  			0=00ff;1=000f
>  
> +		Set each CBM to a specified value.
> +
> +		A special value "*" is required to represent all cache IDs.

nit: the value is not "required". Compare with, for example, "An ID of '*' configures all domains with provided CBM"

> +
> +		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/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index b2d178d3556e..7b3119f91b9d 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -873,41 +873,45 @@ 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')
> -		return 0;
> +	while (line && line[0] != '\0') {

Why is it necessary to change to a while loop? From what I can tell this is not required to
support this particular feature and thus makes this patch consist of two logical changes instead
of one (see "Separate your changes" in Documentation/process/submitting-patches.rst). Since the
focus of this patch is to support "*" the switch to a while loop distracts from the core change due
to generating so many line changes.

I have two concerns with a switch to a while loop:

The switch to the while loop has a subtle but significant consequence that changes the behavior of this
flow to return "success" when the user provides an invalid domain ID. The caller, resctrl_io_alloc_cbm_write(),
expects "success" to mean that the configuration has been staged and can be configured on hardware. Now
it may be possible that hardware configuration attempted without a configuration staged?

Apart from the user interface change the switch to the while loop changes the common pattern in
resctrl used for parsing similar content from user space. See fs/resctrl/ctrlmondata.c:parse_line(),
fs/resctrl/monitor.c:resctrl_parse_mbm_assignment(), fs/resctrl/rdtgroup.c:mon_config_write().
When working with resctrl I prefer to use common patterns. If there is an issue with a particular
pattern then all instances should be changed.

Please just change the code needed to support the new feature.

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

Have you tried this feature with some edge cases? When considering, for example, a
user just providing "*" then from this point on "dom" is assumed to point to a valid
CBM but is actually NULL. Since this parses user input it is required to be robust against
any input.

Reinette

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [v3 PATCH 1/1] x86/resctrl: Add "*" shorthand to set io_alloc CBM for all domains
  2026-01-08 18:45   ` Reinette Chatre
@ 2026-01-26  3:30     ` Aaron Tomlin
  0 siblings, 0 replies; 6+ messages in thread
From: Aaron Tomlin @ 2026-01-26  3:30 UTC (permalink / raw)
  To: Reinette Chatre
  Cc: tony.luck, Dave.Martin, james.morse, babu.moger, tglx, mingo, bp,
	dave.hansen, sean, linux-kernel

On Thu, Jan 08, 2026 at 10:45:11AM -0800, Reinette Chatre wrote:
> nit: the value is not "required". Compare with, for example, "An ID of
> '*' configures all domains with provided CBM"

Hi Reinette,

Agreed. Your phrasing is much clearer; I shall incorporate this change into
the documentation.

> Why is it necessary to change to a while loop? From what I can tell this
> is not required to support this particular feature and thus makes this
> patch consist of two logical changes instead of one (see "Separate your
> changes" in Documentation/process/submitting-patches.rst). Since the
> focus of this patch is to support "*" the switch to a while loop
> distracts from the core change due to generating so many line changes.
> 
> I have two concerns with a switch to a while loop:
> 
> The switch to the while loop has a subtle but significant consequence
> that changes the behavior of this flow to return "success" when the user
> provides an invalid domain ID. The caller, resctrl_io_alloc_cbm_write(),
> expects "success" to mean that the configuration has been staged and can
> be configured on hardware. Now it may be possible that hardware
> configuration attempted without a configuration staged?
> 
> Apart from the user interface change the switch to the while loop changes
> the common pattern in resctrl used for parsing similar content from user
> space. See fs/resctrl/ctrlmondata.c:parse_line(),
> fs/resctrl/monitor.c:resctrl_parse_mbm_assignment(),
> fs/resctrl/rdtgroup.c:mon_config_write(). When working with resctrl I
> prefer to use common patterns. If there is an issue with a particular
> pattern then all instances should be changed.
> 
> Please just change the code needed to support the new feature.

You are quite right. In hindsight, the refactoring distracted from the
primary objective of the patch and, as you noted, inadvertently introduced
a regression regarding invalid domain IDs. I also appreciate the point
regarding consistency with the existing resctrl parsing patterns.

I will revert to the original goto style in the next iteration to ensure
the patch remains minimal and consistent with the subsystem's conventions.

> Have you tried this feature with some edge cases? When considering, for
> example, a user just providing "*" then from this point on "dom" is
> assumed to point to a valid CBM but is actually NULL. Since this parses
> user input it is required to be robust against any input.

That is a fair point. I will ensure the parser is robust against malformed
input, such as a bare "*" without a corresponding value, to prevent any
potential null pointer dereferences.

I provided a v4 [1].

[1]: https://lore.kernel.org/lkml/20260125171752.3374930-1-atomlin@atomlin.com/


Kind regards,
-- 
Aaron Tomlin

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-01-26  3:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-31  2:35 [v3 PATCH 0/1] x86/resctrl: Add "*" shorthand to set io_alloc CBM for all domains Aaron Tomlin
2025-12-31  2:35 ` [v3 PATCH 1/1] " Aaron Tomlin
2026-01-06 16:49   ` Babu Moger
2026-01-07  1:28     ` Aaron Tomlin
2026-01-08 18:45   ` Reinette Chatre
2026-01-26  3:30     ` Aaron Tomlin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.