Message-ID: <239e3a89-1075-439b-bf1c-d2be5be5c30c@amd.com>
Date: Fri, 5 Dec 2025 13:30:09 -0600
From: "Moger, Babu" <bmoger@....com>
To: Aaron Tomlin <atomlin@...mlin.com>, tony.luck@...el.com,
 reinette.chatre@...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: [PATCH 3/3] x86/resctrl: Add "*" shorthand to set minimum
 io_alloc CBM for all domains

Hi Aaron,

On 11/26/2025 11:16 AM, 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@...mlin.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                  | 108 +++++++++++++++++-----
>   fs/resctrl/internal.h                     |  13 +++
>   fs/resctrl/rdtgroup.c                     |   2 +-
>   include/linux/resctrl.h                   |  30 +++++-
>   7 files changed, 155 insertions(+), 33 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 5f6f96d70e4a..b3abd781899f 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,29 +918,82 @@ 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 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 *dom = NULL;
> +	unsigned long out_val;
> +	int ret;
> +
> +	if (line[0] == '*') {
> +		ret = parse_domain_cbm(&line, &out_val, RDT_PARSE_CBM);
> +		if (ret)
> +			return ret;
>   
> +		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;
> +	}
>   next:
>   	if (!line || line[0] == '\0')
>   		return 0;
>   
>   	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;
> -	}
> +	ret = parse_domain_cbm(&line, &out_val, RDT_PARSE_DOMAIN);
> +	if (ret)
> +		return ret;
>   

I feel this the lot of changes to take care of one extra character 
change. I feel code can be simplified to just this function.

  id  = strim(id);
  dom = strim(dom);

  if (!strcmp(id, "*"))
      all = true;
   else if (kstrtoul(id, 10, &dom_id)) {
             rdt_last_cmd_puts("Non-numeric domain\n");
             return -EINVAL;
   }

   data.buf   = dom;
   data.mode  = RDT_MODE_SHAREABLE;
   data.closid = closid;

   if (all)
      apply the CBMs to all the doamins.
   else
      Apply the CBMs to just one domain.


Any problems with this approach?

Thanks

Babu



>   	dom = strim(dom);
>   	list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
> -		if (d->hdr.id == dom_id) {
> +		if (d->hdr.id == out_val) {
>   			data.buf = dom;
>   			data.mode = RDT_MODE_SHAREABLE;
>   			data.closid = closid;
> @@ -935,12 +1003,8 @@ static int resctrl_io_alloc_parse_line(char *line,  struct rdt_resource *r,
>   			 * 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]));
> -			}
> +			if (resctrl_arch_get_cdp_enabled(r->rid))
> +				resctrl_sync_cdp_peer(s, d);
>   			goto next;
>   		}
>   	}
> @@ -982,6 +1046,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 */

Powered by blists - more mailing lists