* [PATCH v5 0/2] fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains @ 2026-02-10 21:07 Aaron Tomlin 2026-02-10 21:07 ` [PATCH v5 1/2] fs/resctrl: Report invalid domain ID when parsing io_alloc Aaron Tomlin 2026-02-10 21:07 ` [PATCH v5 2/2] fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains Aaron Tomlin 0 siblings, 2 replies; 7+ messages in thread From: Aaron Tomlin @ 2026-02-10 21:07 UTC (permalink / raw) To: tony.luck, reinette.chatre, Dave.Martin, james.morse, babu.moger, tglx, mingo, bp, dave.hansen Cc: dave.martin, sean, neelx, mproche, chjohnst, linux-kernel Hi Babu, Tony, Reinette, This series addresses two limitations in the fs/resctrl io_alloc interface: inconsistent error reporting and the lack of a mechanism for bulk configuration. The last_cmd_status file is intended to report details about the most recent filesystem operation to aid in diagnosing failures. Currently, however, when parsing io_alloc_cbm, providing an invalid domain ID causes the operation to fail with -EINVAL without updating last_cmd_status. This results in confusing behaviour where the system call fails, but the status file may misleadingly report "ok" (retained from a previous successful operation). Patch 1 addresses this by updating resctrl_io_alloc_parse_line() to explicitly write an error message to last_cmd_status when the target domain ID cannot be found. Configuring the io_alloc_cbm interface currently requires an explicit domain ID for each cache domain. On systems with high core counts and numerous cache clusters, this requirement becomes cumbersome for automation tasks that aim to apply a uniform policy across the system. Patch 2 introduces a wildcard domain ID selector "*". This enables users to programme the Capacity Bitmask (CBM) across all cache domains in a single operation (e.g., writing "*=0" to apply a mask to all domains), provided the value remains within the valid range defined by the resource. Please let me know your thoughts. Changes since v4 [1]: - Split the submission into a two-patch series to separate the fix from the new feature (Reinette Chatre) - Refactored resctrl_io_alloc_parse_line() to prioritise the dom check before string comparison, preventing potential NULL pointer dereference (Reinette Chatre) - Restructured commit messages to strictly follow the "Context, Problem, Solution" format (Reinette Chatre) - Updated the documentation to include the result of the modification (Babu Moger) - Updated the commit description to avoid implying that '0' is a universal minimum CBM (Reinette Chatre) Changes since v3 [2]: - Updated the wildcard documentation text to be clearer and less prescriptive (Reinette Chatre) - Reverted the while loop refactoring in resctrl_io_alloc_parse_line() to restore the standard parsing pattern and prevent invalid domain ID regressions (Reinette Chatre) - Added explicit validation to ensure the wildcard selector is followed by a value assignment (Reinette Chatre) - Restored memcpy() usage for configuration copying to minimise unrelated diff noise Changes since v2 [2]: - 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 [3]: - 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/20260125171752.3374930-1-atomlin@atomlin.com/ [2]: https://lore.kernel.org/lkml/20251231023549.2390630-1-atomlin@atomlin.com/ [3]: https://lore.kernel.org/lkml/20251215230257.1798865-1-atomlin@atomlin.com/ [4]: https://lore.kernel.org/lkml/20251126171653.1004321-1-atomlin@atomlin.com/ Aaron Tomlin (2): fs/resctrl: Report invalid domain ID when parsing io_alloc fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains Documentation/filesystems/resctrl.rst | 10 ++++++++++ fs/resctrl/ctrlmondata.c | 16 +++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) -- 2.51.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 1/2] fs/resctrl: Report invalid domain ID when parsing io_alloc 2026-02-10 21:07 [PATCH v5 0/2] fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains Aaron Tomlin @ 2026-02-10 21:07 ` Aaron Tomlin 2026-02-18 18:27 ` Reinette Chatre 2026-02-10 21:07 ` [PATCH v5 2/2] fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains Aaron Tomlin 1 sibling, 1 reply; 7+ messages in thread From: Aaron Tomlin @ 2026-02-10 21:07 UTC (permalink / raw) To: tony.luck, reinette.chatre, Dave.Martin, james.morse, babu.moger, tglx, mingo, bp, dave.hansen Cc: dave.martin, sean, neelx, mproche, chjohnst, linux-kernel The last_cmd_status file is intended to report details about the most recent resctrl filesystem operation, specifically to aid in diagnosing failures. However, when parsing io_alloc_cbm, if a user provides a domain ID that does not exist in the resource, the operation fails with -EINVAL without updating last_cmd_status. This results in inconsistent behaviour where the system call reports an error, but the status file may misleadingly report "ok" (retained from a previous successful operation), leaving the user unaware that the failure was caused by an invalid domain ID. Update resctrl_io_alloc_parse_line() to explicitly write an error message to last_cmd_status when the target domain ID cannot be found. Suggested-by: Reinette Chatre <reinette.chatre@intel.com> Fixes: 28fa2cce7a83 ("fs/resctrl: Introduce interface to modify io_alloc capacity bitmasks") Signed-off-by: Aaron Tomlin <atomlin@atomlin.com> --- fs/resctrl/ctrlmondata.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c index b2d178d3556e..b96b661626c2 100644 --- a/fs/resctrl/ctrlmondata.c +++ b/fs/resctrl/ctrlmondata.c @@ -907,6 +907,7 @@ static int resctrl_io_alloc_parse_line(char *line, struct rdt_resource *r, } } + rdt_last_cmd_printf("Invalid domain %lu\n", dom_id); return -EINVAL; } -- 2.51.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/2] fs/resctrl: Report invalid domain ID when parsing io_alloc 2026-02-10 21:07 ` [PATCH v5 1/2] fs/resctrl: Report invalid domain ID when parsing io_alloc Aaron Tomlin @ 2026-02-18 18:27 ` Reinette Chatre 2026-03-24 23:23 ` Aaron Tomlin 0 siblings, 1 reply; 7+ messages in thread From: Reinette Chatre @ 2026-02-18 18:27 UTC (permalink / raw) To: Aaron Tomlin, tony.luck, Dave.Martin, james.morse, babu.moger, tglx, mingo, bp, dave.hansen Cc: sean, neelx, mproche, chjohnst, linux-kernel Hi Aaron, typo in subject: "io_alloc" -> "io_alloc_cbm" On 2/10/26 1:07 PM, Aaron Tomlin wrote: > The last_cmd_status file is intended to report details about the most > recent resctrl filesystem operation, specifically to aid in diagnosing > failures. > > However, when parsing io_alloc_cbm, if a user provides a domain ID that > does not exist in the resource, the operation fails with -EINVAL without > updating last_cmd_status. This results in inconsistent behaviour where > the system call reports an error, but the status file may misleadingly "reports" -> "returns" > report "ok" (retained from a previous successful operation), leaving the This can be made specific: "the status file may misleadingly report "ok"" -> "last_cmd_status misleadingly reports" Drop "(retained from a previous successful operation)" since it is not accurate. Note that resctrl_io_alloc_cbm_write() starts with rdt_last_cmd_clear() that clears the last_cmd_status buffer and rdt_last_cmd_status_show() always prints "ok" when the buffer is empty. > user unaware that the failure was caused by an invalid domain ID. > > Update resctrl_io_alloc_parse_line() to explicitly write an error > message to last_cmd_status when the target domain ID cannot be found. Drop "Update resctrl_io_alloc_parse_line() to explicitly" that can be seen from the patch. > > Suggested-by: Reinette Chatre <reinette.chatre@intel.com> > Fixes: 28fa2cce7a83 ("fs/resctrl: Introduce interface to modify io_alloc capacity bitmasks") Please place the "Fixes:" tag first. For reference, "Ordering of commit tags" in Documentation/process/maintainer-tip.rst. > Signed-off-by: Aaron Tomlin <atomlin@atomlin.com> > --- > fs/resctrl/ctrlmondata.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c > index b2d178d3556e..b96b661626c2 100644 > --- a/fs/resctrl/ctrlmondata.c > +++ b/fs/resctrl/ctrlmondata.c > @@ -907,6 +907,7 @@ static int resctrl_io_alloc_parse_line(char *line, struct rdt_resource *r, > } > } > > + rdt_last_cmd_printf("Invalid domain %lu\n", dom_id); > return -EINVAL; > } > Thank you very much for adding the fix. The actual change looks good to me, just a couple of nits on the changelog. Reinette ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/2] fs/resctrl: Report invalid domain ID when parsing io_alloc 2026-02-18 18:27 ` Reinette Chatre @ 2026-03-24 23:23 ` Aaron Tomlin 0 siblings, 0 replies; 7+ messages in thread From: Aaron Tomlin @ 2026-03-24 23:23 UTC (permalink / raw) To: Reinette Chatre Cc: tony.luck, Dave.Martin, james.morse, babu.moger, tglx, mingo, bp, dave.hansen, sean, neelx, mproche, chjohnst, linux-kernel [-- Attachment #1: Type: text/plain, Size: 744 bytes --] On Wed, Feb 18, 2026 at 10:27:31AM -0800, Reinette Chatre wrote: > Thank you very much for adding the fix. The actual change looks good to me, > just a couple of nits on the changelog. > Hi Reinette, Firstly apologies for the delay. Thank you for the careful review and for catching those nits. I especially appreciate the technical clarification regarding rdt_last_cmd_clear() and the status buffer's behaviour—thank you for pointing that out. I have applied all of your suggested changes to the commit message, including correcting the subject line to use "io_alloc_cbm" and adjusting the commit tag order. I will ensure these updates are included in the v6 submission shortly. Kind regards, -- Aaron Tomlin [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 2/2] fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains 2026-02-10 21:07 [PATCH v5 0/2] fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains Aaron Tomlin 2026-02-10 21:07 ` [PATCH v5 1/2] fs/resctrl: Report invalid domain ID when parsing io_alloc Aaron Tomlin @ 2026-02-10 21:07 ` Aaron Tomlin 2026-02-18 18:30 ` Reinette Chatre 1 sibling, 1 reply; 7+ messages in thread From: Aaron Tomlin @ 2026-02-10 21:07 UTC (permalink / raw) To: tony.luck, reinette.chatre, Dave.Martin, james.morse, babu.moger, tglx, mingo, bp, dave.hansen Cc: dave.martin, sean, neelx, mproche, chjohnst, linux-kernel Currently, configuring the io_alloc_cbm interface requires an explicit domain ID for each cache domain. On systems with high core counts and numerous cache clusters, this requirement becomes cumbersome for automation and management tasks that aim to apply a uniform policy. Introduce a wildcard domain ID selector "*" for the io_alloc_cbm interface. This enables users to update the Capacity Bitmask (CBM) across all cache domains in a single operation. For example, a user can write "*=0" to the io_alloc_cbm file to programme every domain with the same mask. The value supplied must, however, remain within the valid range defined by the resource (e.g., min_cbm_bits). Signed-off-by: Aaron Tomlin <atomlin@atomlin.com> --- Documentation/filesystems/resctrl.rst | 10 ++++++++++ fs/resctrl/ctrlmondata.c | 15 ++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst index 8c8ce678148a..948219e58882 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 a specified value. + + An ID of "*" configures all domains with the provided CBM. + + Example:: + + # echo "*=0" > /sys/fs/resctrl/info/L3/io_alloc_cbm + # 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 b96b661626c2..f47331a97337 100644 --- a/fs/resctrl/ctrlmondata.c +++ b/fs/resctrl/ctrlmondata.c @@ -873,21 +873,26 @@ 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; + update_all = false; dom = strsep(&line, ";"); id = strsep(&dom, "="); - if (!dom || kstrtoul(id, 10, &dom_id)) { + + if (dom && !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) { + if (update_all || d->hdr.id == dom_id) { data.buf = dom; data.mode = RDT_MODE_SHAREABLE; data.closid = closid; @@ -903,10 +908,14 @@ static int resctrl_io_alloc_parse_line(char *line, struct rdt_resource *r, &d->staged_config[s->conf_type], sizeof(d->staged_config[0])); } - goto next; + if (!update_all) + goto next; } } + if (update_all) + goto next; + rdt_last_cmd_printf("Invalid domain %lu\n", dom_id); return -EINVAL; } -- 2.51.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains 2026-02-10 21:07 ` [PATCH v5 2/2] fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains Aaron Tomlin @ 2026-02-18 18:30 ` Reinette Chatre 2026-03-24 23:53 ` Aaron Tomlin 0 siblings, 1 reply; 7+ messages in thread From: Reinette Chatre @ 2026-02-18 18:30 UTC (permalink / raw) To: Aaron Tomlin, tony.luck, Dave.Martin, james.morse, babu.moger, tglx, mingo, bp, dave.hansen Cc: sean, neelx, mproche, chjohnst, linux-kernel Hi Aaron, On 2/10/26 1:07 PM, Aaron Tomlin wrote: > Currently, configuring the io_alloc_cbm interface requires an explicit (nit: "Currently" can be dropped. Expectation is that the patch starts with context that is by default the current behavior.) > domain ID for each cache domain. On systems with high core counts and > numerous cache clusters, this requirement becomes cumbersome for > automation and management tasks that aim to apply a uniform policy. > > Introduce a wildcard domain ID selector "*" for the io_alloc_cbm > interface. This enables users to update the Capacity Bitmask (CBM) "update the" -> "set the same"? > across all cache domains in a single operation. > > For example, a user can write "*=0" to the io_alloc_cbm file to This example seems redundant. The description is clear and the documentation addition found in the patch describes this scenario anyway. > programme every domain with the same mask. The value supplied must, "programme" -> "program" I suggest to drop the last sentence "The value supplied ..." since it implies that the addition cannot do its own error checking. > however, remain within the valid range defined by the resource > (e.g., min_cbm_bits). > > Signed-off-by: Aaron Tomlin <atomlin@atomlin.com> > --- > Documentation/filesystems/resctrl.rst | 10 ++++++++++ > fs/resctrl/ctrlmondata.c | 15 ++++++++++++--- > 2 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst > index 8c8ce678148a..948219e58882 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 a specified value. Above sentence seems redundant when compared to below sentence that is more specific. Can it just be dropped? > + > + An ID of "*" configures all domains with the provided CBM. > + > + Example:: Just to make this clear I suggest to add caveat: "Example on a system that does not require a minimum number of consecutive bits in the mask::" > + > + # echo "*=0" > /sys/fs/resctrl/info/L3/io_alloc_cbm > + # 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 b96b661626c2..f47331a97337 100644 > --- a/fs/resctrl/ctrlmondata.c > +++ b/fs/resctrl/ctrlmondata.c > @@ -873,21 +873,26 @@ 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; > > + update_all = false; > dom = strsep(&line, ";"); > id = strsep(&dom, "="); > - if (!dom || kstrtoul(id, 10, &dom_id)) { > + > + if (dom && !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) { > + if (update_all || d->hdr.id == dom_id) { While this code is correct in that dom_id will not be accessed when update_all is true it makes the code more difficult to understand when an uninitialized variable depends on certain flows. > data.buf = dom; > data.mode = RDT_MODE_SHAREABLE; > data.closid = closid; > @@ -903,10 +908,14 @@ static int resctrl_io_alloc_parse_line(char *line, struct rdt_resource *r, > &d->staged_config[s->conf_type], > sizeof(d->staged_config[0])); > } > - goto next; > + if (!update_all) > + goto next; > } > } > > + if (update_all) > + goto next; One comment from v4 appears to be unaddressed. Copied here for convenience: I see that this aims to support input like "*=f;*=0" but I do not see how something like this can ever succeed since parse_cbm() stages the config and should fail if any domain already has a config. Should this perhaps just return success here? This could be made more robust by only returning success if there is no more text to parse, thus failing on input like "*=f;1=f". Below is a sample fixup that addresses the comments. What do you think? diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c index 869e37f7d22a..9a7dfc48cb2e 100644 --- a/fs/resctrl/ctrlmondata.c +++ b/fs/resctrl/ctrlmondata.c @@ -954,17 +954,21 @@ 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; + unsigned long dom_id = ULONG_MAX; struct rdt_parse_data data; struct rdt_ctrl_domain *d; + bool update_all = false; char *dom = NULL, *id; - unsigned long dom_id; - bool update_all; next: if (!line || line[0] == '\0') return 0; - update_all = false; + if (update_all) { + rdt_last_cmd_puts("Configurations after global '*'\n"); + return -EINVAL; + } + dom = strsep(&line, ";"); id = strsep(&dom, "="); > + > rdt_last_cmd_printf("Invalid domain %lu\n", dom_id); > return -EINVAL; > } Reinette ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains 2026-02-18 18:30 ` Reinette Chatre @ 2026-03-24 23:53 ` Aaron Tomlin 0 siblings, 0 replies; 7+ messages in thread From: Aaron Tomlin @ 2026-03-24 23:53 UTC (permalink / raw) To: Reinette Chatre Cc: tony.luck, Dave.Martin, james.morse, babu.moger, tglx, mingo, bp, dave.hansen, sean, neelx, mproche, chjohnst, linux-kernel [-- Attachment #1: Type: text/plain, Size: 6613 bytes --] On Wed, Feb 18, 2026 at 10:30:57AM -0800, Reinette Chatre wrote: > Hi Aaron, Hi Reinette, > On 2/10/26 1:07 PM, Aaron Tomlin wrote: > > Currently, configuring the io_alloc_cbm interface requires an explicit > > (nit: "Currently" can be dropped. Expectation is that the patch starts > with context that is by default the current behavior.) Acknowledged. I have dropped "Currently," from the opening sentence of the commit message. > > > domain ID for each cache domain. On systems with high core counts and > > numerous cache clusters, this requirement becomes cumbersome for > > automation and management tasks that aim to apply a uniform policy. > > > > Introduce a wildcard domain ID selector "*" for the io_alloc_cbm > > interface. This enables users to update the Capacity Bitmask (CBM) > > "update the" -> "set the same"? Acknowledged. I have updated the phrasing to "set the same Capacity Bitmask (CBM)" in the commit message. > > across all cache domains in a single operation. > > > > For example, a user can write "*=0" to the io_alloc_cbm file to > > This example seems redundant. The description is clear and the documentation > addition found in the patch describes this scenario anyway. > > > programme every domain with the same mask. The value supplied must, > > "programme" -> "program" > Acknowledged. I completely dropped the final paragraph containing the redundant example, the typo, and the validation disclaimer from the commit message. > > however, remain within the valid range defined by the resource > > (e.g., min_cbm_bits). > > > > Signed-off-by: Aaron Tomlin <atomlin@atomlin.com> > > --- > > Documentation/filesystems/resctrl.rst | 10 ++++++++++ > > fs/resctrl/ctrlmondata.c | 15 ++++++++++++--- > > 2 files changed, 22 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst > > index 8c8ce678148a..948219e58882 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 a specified value. > > Above sentence seems redundant when compared to below sentence that is > more specific. Can it just be dropped? Yes. I removed "Set each CBM to a specified value" from resctrl.rst. > > + > > + An ID of "*" configures all domains with the provided CBM. > > + > > + Example:: > > Just to make this clear I suggest to add caveat: > "Example on a system that does not require a minimum number of consecutive bits in the mask::" > I updated the caveat preceding the example block in resctrl.rst exactly as suggested. > > + > > + # echo "*=0" > /sys/fs/resctrl/info/L3/io_alloc_cbm > > + # 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 b96b661626c2..f47331a97337 100644 > > --- a/fs/resctrl/ctrlmondata.c > > +++ b/fs/resctrl/ctrlmondata.c > > @@ -873,21 +873,26 @@ 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; > > > > + update_all = false; > > dom = strsep(&line, ";"); > > id = strsep(&dom, "="); > > - if (!dom || kstrtoul(id, 10, &dom_id)) { > > + > > + if (dom && !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) { > > + if (update_all || d->hdr.id == dom_id) { > > While this code is correct in that dom_id will not be accessed when > update_all is true it makes the code more difficult to understand > when an uninitialized variable depends on certain flows. I applied your suggested change; at declaration to eliminate the uninitialised variable state. > > data.buf = dom; > > data.mode = RDT_MODE_SHAREABLE; > > data.closid = closid; > > @@ -903,10 +908,14 @@ static int resctrl_io_alloc_parse_line(char *line, struct rdt_resource *r, > > &d->staged_config[s->conf_type], > > sizeof(d->staged_config[0])); > > } > > - goto next; > > + if (!update_all) > > + goto next; > > } > > } > > > > + if (update_all) > > + goto next; > > One comment from v4 appears to be unaddressed. Copied here for convenience: > I see that this aims to support input like "*=f;*=0" but I do not see how something like > this can ever succeed since parse_cbm() stages the config and should fail if any domain > already has a config. Should this perhaps just return success here? This could be > made more robust by only returning success if there is no more text to parse, thus failing > on input like "*=f;1=f". > > Below is a sample fixup that addresses the comments. What do you think? > > diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c > index 869e37f7d22a..9a7dfc48cb2e 100644 > --- a/fs/resctrl/ctrlmondata.c > +++ b/fs/resctrl/ctrlmondata.c > @@ -954,17 +954,21 @@ 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; > + unsigned long dom_id = ULONG_MAX; > struct rdt_parse_data data; > struct rdt_ctrl_domain *d; > + bool update_all = false; > char *dom = NULL, *id; > - unsigned long dom_id; > - bool update_all; > > next: > if (!line || line[0] == '\0') > return 0; > > - update_all = false; > + if (update_all) { > + rdt_last_cmd_puts("Configurations after global '*'\n"); > + return -EINVAL; > + } > + > dom = strsep(&line, ";"); > id = strsep(&dom, "="); > > > > + > > rdt_last_cmd_printf("Invalid domain %lu\n", dom_id); > > return -EINVAL; > > } > I have applied it exactly as suggested. Kind regards, -- Aaron Tomlin [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-03-24 23:53 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-02-10 21:07 [PATCH v5 0/2] fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains Aaron Tomlin 2026-02-10 21:07 ` [PATCH v5 1/2] fs/resctrl: Report invalid domain ID when parsing io_alloc Aaron Tomlin 2026-02-18 18:27 ` Reinette Chatre 2026-03-24 23:23 ` Aaron Tomlin 2026-02-10 21:07 ` [PATCH v5 2/2] fs/resctrl: Add "*" shorthand to set io_alloc CBM for all domains Aaron Tomlin 2026-02-18 18:30 ` Reinette Chatre 2026-03-24 23:53 ` Aaron Tomlin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®