[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bf8bb682-6a4d-4f39-916c-952719fcf48d@arm.com>
Date: Thu, 15 Jan 2026 10:32:45 +0000
From: Ben Horgan <ben.horgan@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, linux-kernel@...r.kernel.org
Cc: tony.luck@...el.com, Dave.Martin@....com, james.morse@....com,
babu.moger@....com, bp@...en8.de
Subject: Re: [PATCH v1] fs/resctrl: Add missing kconfig entry for
CONFIG_RESCTRL_ASSIGN_FIXED
Hi Reinette,
On 1/14/26 19:09, Reinette Chatre wrote:
> Hi Ben,
>
> Per subject, the Kconfig entry has not really been missing but has instead been
> intentionally left undefined which is handled correctly by IS_ENABLED(). The
> plans/usage surrounding CONFIG_RESCTRL_ASSIGN_FIXED can be found in the same
> commit used in "Fixes" tag. Adding the definition as a fix to the patch that
> documents that it is intentionally left undefined is unexpected.
>
> This patch is just following existing plan that CONFIG_RESCTRL_ASSIGN_FIXED
> will be defined at the time Arm support needs it, no? Folks using "Fixes" to
> determine if a change needs to be backported will find no benefit from this
> and terming it a fix will just cause unnecessary work.
Fair enough. I didn't follow that discussion. I'll drop the fixes tag on
a respin.
>
> On 1/13/26 6:49 AM, Ben Horgan wrote:
>> The commit 3b497c3f4f04 ("fs/resctrl: Introduce the interface to display
>> monitoring modes") introduced CONFIG_RESCTRL_ASSIGN_FIXED but did not add
>> the kconfig entry. Add this.
>>
>> Also, take the opportunity ensure that any user attempt to change the
>> assign mode fails from the resctrl code rather than delegating to the arch
>> specific code and let the user know by adding a message in last_cmd_status.
>
> Why *any* attempt? Avoiding delegating to arch seems ok as a goal but this change
> additionally changes interface with user space. Current behavior when user writes
> existing mode to the file is to just return success. For example, if current mode
> is "default" and user writes "default". This patch changes this behavior to fail
> instead. Is this intended?
Intended but as you point out, it would be best to accept the current
value without error for maximum compatibility between architectures.
I'll update the config option help text to reflect this. Was the help
text also discussed previously?
>
>> Fixes: 3b497c3f4f04 ("fs/resctrl: Introduce the interface to display monitoring modes")
>> Signed-off-by: Ben Horgan <ben.horgan@....com>
>> ---
>> fs/resctrl/Kconfig | 8 ++++++++
>> fs/resctrl/monitor.c | 6 ++++++
>> 2 files changed, 14 insertions(+)
>>
>> diff --git a/fs/resctrl/Kconfig b/fs/resctrl/Kconfig
>> index 21671301bd8a..bdf9e11502fa 100644
>> --- a/fs/resctrl/Kconfig
>> +++ b/fs/resctrl/Kconfig
>> @@ -37,3 +37,11 @@ config RESCTRL_RMID_DEPENDS_ON_CLOSID
>> Enabled by the architecture when the RMID values depend on the CLOSID.
>> This causes the CLOSID allocator to search for CLOSID with clean
>> RMID.
>> +
>> +config RESCTRL_ASSIGN_FIXED
>> + bool
>> + depends on RESCTRL_FS
>> + help
>> + Enabled by the architecture when the counter assignment mode is not
>> + configurable. This ensures that counter assignment is not advertised as
>> + configurable and configuration attempts fail.
>> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
>> index 572a9925bd6c..888e303c18a2 100644
>> --- a/fs/resctrl/monitor.c
>> +++ b/fs/resctrl/monitor.c
>> @@ -1434,6 +1434,12 @@ ssize_t resctrl_mbm_assign_mode_write(struct kernfs_open_file *of, char *buf,
>>
>> rdt_last_cmd_clear();
>>
>> + if (IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED)) {
>> + ret = -EINVAL;
>> + rdt_last_cmd_puts("assign mode is not configurable\n");
>> + goto out_unlock;
>> + }
>> +
>> if (!strcmp(buf, "default")) {
>> enable = 0;
>> } else if (!strcmp(buf, "mbm_event")) {
>
> Reinette
Thanks,
Ben
Powered by blists - more mailing lists