From: Reinette Chatre <reinette.chatre@intel.com>
To: Aaron Tomlin <atomlin@atomlin.com>, <tony.luck@intel.com>,
<Dave.Martin@arm.com>, <james.morse@arm.com>,
<babu.moger@amd.com>, <tglx@linutronix.de>, <mingo@redhat.com>,
<bp@alien8.de>, <dave.hansen@linux.intel.com>
Cc: <sean@ashe.io>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/3] fs/resctrl: Add helpers to check io_alloc support and enabled state
Date: Tue, 16 Dec 2025 21:01:32 -0800 [thread overview]
Message-ID: <dcb81c71-6ced-4327-b32a-a431e31c4d93@intel.com> (raw)
In-Reply-To: <20251215230257.1798865-2-atomlin@atomlin.com>
Hi Aaron,
On 12/15/25 3:02 PM, Aaron Tomlin wrote:
> This patch introduces two helpers to validate io_alloc support and
> whether it is enabled. This reduces code duplication, clarifies
> intent, and preserves existing semantics and messages.
>
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
> fs/resctrl/ctrlmondata.c | 74 +++++++++++++++++++++++++++++-----------
> 1 file changed, 54 insertions(+), 20 deletions(-)
>
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index b2d178d3556e..d2f9a4f2d887 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -758,6 +758,50 @@ u32 resctrl_io_alloc_closid(struct rdt_resource *r)
> return resctrl_arch_get_num_closid(r) - 1;
> }
>
> +/*
> + * resctrl_io_alloc_supported() - Establish if io_alloc is supported
> + *
> + * @s: resctrl resource schema.
> + *
> + * This function must be called under the cpu hotplug lock
> + * and rdtgroup mutex
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +static inline int resctrl_io_alloc_supported(struct resctrl_schema *s)
> +{
> + struct rdt_resource *r = s->res;
> +
> + if (!r->cache.io_alloc.io_alloc_capable) {
> + rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * resctrl_io_alloc_enabled() - Establish if io_alloc is enabled
> + *
> + * @s: resctrl resource schema
> + *
> + * This function must be called under the cpu hotplug lock
> + * and rdtgroup mutex
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +static inline int resctrl_io_alloc_enabled(struct resctrl_schema *s)
> +{
> + struct rdt_resource *r = s->res;
> +
> + if (!resctrl_arch_get_io_alloc_enabled(r)) {
> + rdt_last_cmd_printf("io_alloc is not enabled on %s\n", s->name);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
> size_t nbytes, loff_t off)
> {
> @@ -777,11 +821,9 @@ ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
>
> rdt_last_cmd_clear();
>
> - if (!r->cache.io_alloc_capable) {
> - rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
> - ret = -ENODEV;
> + ret = resctrl_io_alloc_supported(s);
> + if (ret)
> goto out_unlock;
> - }
>
> /* If the feature is already up to date, no action is needed. */
> if (resctrl_arch_get_io_alloc_enabled(r) == enable)
> @@ -839,17 +881,13 @@ int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq,
>
> rdt_last_cmd_clear();
>
> - if (!r->cache.io_alloc_capable) {
> - rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
> - ret = -ENODEV;
> + ret = resctrl_io_alloc_supported(s);
> + if (ret)
> goto out_unlock;
> - }
>
> - if (!resctrl_arch_get_io_alloc_enabled(r)) {
> - rdt_last_cmd_printf("io_alloc is not enabled on %s\n", s->name);
> - ret = -EINVAL;
> + ret = resctrl_io_alloc_enabled(s);
> + if (ret)
> goto out_unlock;
> - }
>
> /*
> * When CDP is enabled, the CBMs of the highest CLOSID of CDP_CODE and
> @@ -928,17 +966,13 @@ ssize_t resctrl_io_alloc_cbm_write(struct kernfs_open_file *of, char *buf,
> mutex_lock(&rdtgroup_mutex);
> rdt_last_cmd_clear();
>
> - if (!r->cache.io_alloc_capable) {
> - rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
> - ret = -ENODEV;
> + ret = resctrl_io_alloc_supported(s);
> + if (ret)
> goto out_unlock;
> - }
>
> - if (!resctrl_arch_get_io_alloc_enabled(r)) {
> - rdt_last_cmd_printf("io_alloc is not enabled on %s\n", s->name);
> - ret = -EINVAL;
> + ret = resctrl_io_alloc_enabled(s);
> + if (ret)
> goto out_unlock;
> - }
>
> io_alloc_closid = resctrl_io_alloc_closid(r);
>
How does this patch benefit the goal of this submission, which is to set identical CBM on
all domains?
If this change benefited this submission I should find these new helpers used in later patches
but they are not. This seems to be an unrelated and unnecessary change mixed with this
submission. It does not fix a bug nor does it support this submission and thus just adds noise when
considering the new feature for inclusion. This patch can be dropped.
Reinette
next prev parent reply other threads:[~2025-12-17 5:01 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-15 23:02 [PATCH v2 0/3] x86/resctrl: Add "*" shorthand to set minimum io_alloc CBM for all domains Aaron Tomlin
2025-12-15 23:02 ` [PATCH v2 1/3] fs/resctrl: Add helpers to check io_alloc support and enabled state Aaron Tomlin
2025-12-17 5:01 ` Reinette Chatre [this message]
2025-12-18 23:22 ` Aaron Tomlin
2025-12-19 17:05 ` Reinette Chatre
2025-12-25 22:23 ` Aaron Tomlin
2025-12-15 23:02 ` [PATCH v2 2/3] fs/resctrl: Return -EINVAL for a missing seq_show implementation Aaron Tomlin
2025-12-17 5:02 ` Reinette Chatre
2025-12-18 23:12 ` Aaron Tomlin
2025-12-15 23:02 ` [PATCH v2 3/3] x86/resctrl: Add "*" shorthand to set minimum io_alloc CBM for all domains Aaron Tomlin
2025-12-17 5:53 ` Reinette Chatre
2025-12-19 0:21 ` Aaron Tomlin
2025-12-18 21:32 ` Moger, Babu
2025-12-18 21:49 ` Luck, Tony
2025-12-18 21:56 ` Moger, Babu
2025-12-19 0:31 ` Aaron Tomlin
2025-12-19 0:40 ` Moger, Babu
2025-12-18 22:59 ` Reinette Chatre
2025-12-19 0:44 ` Aaron Tomlin
2025-12-19 18:28 ` Reinette Chatre
2025-12-19 20:42 ` Luck, Tony
2025-12-19 22:00 ` Reinette Chatre
2025-12-19 23:05 ` Luck, Tony
2025-12-25 22:50 ` Aaron Tomlin
2025-12-25 22:33 ` Aaron Tomlin
2025-12-19 0:26 ` Aaron Tomlin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dcb81c71-6ced-4327-b32a-a431e31c4d93@intel.com \
--to=reinette.chatre@intel.com \
--cc=Dave.Martin@arm.com \
--cc=atomlin@atomlin.com \
--cc=babu.moger@amd.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=james.morse@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=sean@ashe.io \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox