Re: [PATCH v2 3/3] x86/resctrl: Add "*" shorthand to set minimum io_alloc CBM for all domains

Moger, Babu
2025-12-18T21:33:07Z

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