linux-kernel - Re: [v3 PATCH 1/1] x86/resctrl: Add "*" shorthand to set io

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ecc39479-75bc-49cf-a53d-57db71f55ef9@intel.com>
Date: Thu, 8 Jan 2026 10:45:11 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: Aaron Tomlin <atomlin@...mlin.com>, <tony.luck@...el.com>,
	<Dave.Martin@....com>, <james.morse@....com>, <babu.moger@....com>,
	<tglx@...utronix.de>, <mingo@...hat.com>, <bp@...en8.de>,
	<dave.hansen@...ux.intel.com>
CC: <sean@...e.io>, <linux-kernel@...r.kernel.org>
Subject: Re: [v3 PATCH 1/1] x86/resctrl: Add "*" shorthand to set io_alloc CBM
 for all domains

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@...mlin.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