Re: [RESEND PATCH v4 14/15] fs/resctrl: Allow user space to write kmode_cpus/kmode_cpus_list

["Moger, Babu" <bmoger@amd.com>]

Hi Reinette,

On 8/10/2026 10:49 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 7/7/26 2:50 PM, Babu Moger wrote:
>> kmode_cpus and kmode_cpus_list expose the CPU scope for the rdtgroup bound
>> to the active kernel-mode policy. They are currently read-only, so changing
>> the scope requires rebinding through info/kernel_mode, which reprograms the
>> whole binding instead of only the CPUs whose state changes.
>>
>> Make kmode_cpus and kmode_cpus_list writable. Parse writes as a bitmap or
>> CPU range list. Reject pseudo-locked and pseudo-lock-setup groups, writes
>> to a group other than resctrl_kcfg.k_rdtgrp (including stale file
>> descriptors left open across an info/kernel_mode change), malformed input,
>> and masks that name offline CPUs.
>>
>> Update the bound group's kmode_cpu_mask and reprogram hardware
>> incrementally: disable kernel-mode association on CPUs in the old mask but
>> not the new mask, and enable it on CPUs in the new mask but not the old
>> mask.
>>
>> Document the interface in Documentation/filesystems/resctrl.rst.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> v4: Empty masks are now allowed and updated masks are in rdtgroup->kmode_cpu_mask.
>>      Updated the changelog.
>>
>> v3: New patch to add "kmode_cpus" and "kmode_cpus_list" to support
>>      kernel_modes.
>> ---
>>   Documentation/filesystems/resctrl.rst |  30 +++++
>>   fs/resctrl/rdtgroup.c                 | 151 +++++++++++++++++++++++++-
>>   2 files changed, 179 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
>> index 5a13814d1325..4a2bdd74d4aa 100644
>> --- a/Documentation/filesystems/resctrl.rst
>> +++ b/Documentation/filesystems/resctrl.rst
>> @@ -676,6 +676,36 @@ All groups contain the following files:
>>   "cpus_list":
>>   	Just like "cpus", only using ranges of CPUs instead of bitmasks.
>>   
>> +"kmode_cpus":
>> +	Visible only on the rdtgroup currently bound to the active kernel
> 
> "Visible" -> "Accessible"?
> "on" -> "within"?
> 
> rdtgroup -> "resource group"
> bound -> "assigned"

Sure.

> 
>> +	mode (see "info/kernel_mode"); hidden on every other rdtgroup,
>> +	including when "inherit_ctrl_and_mon" is active.
> 
> No need to mention that it is hidden in other groups.

ok

> 
>> +
>> +	Bitmask of the logical CPUs scoped for this group's kernel-mode
> 
> What does the "scoped" distinction mean?

"assigned"

> 
>> +	binding.  At bind time through info/kernel_mode, every currently
> 
> What is "bind time"? Could "bind time through" be replaced with "assigned via"?
> Can "assign" be used instead of "bind" throughout this text?

Sure.

> 
>> +	online CPU is included in the scope.  CPUs that come online later
>> +	are automatically added to the scope and programmed with the binding.
> 
> I do not think "scope" is the right term here.

ack

> 
>> +
>> +	Writing a mask reprograms the binding incrementally: it enables on
> 
> I interpret "incrementally" as the write _adds_ the new CPUs to the mask, which
> is not what the implementation does.

Will change it.

> 
>> +	the CPUs newly added by the write and disables on the CPUs dropped
>> +	from the previous mask.  An empty mask disables the binding on all
>> +	CPUs in the current scope.  The mask must contain only online CPUs;
> 
> "CPUs in the current scope" what does "in the current scope" refer to?

Will change it to "assigned"

> 
>> +	masks naming offline CPUs are rejected.
>> +	Errors are reported in "info/last_cmd_status".  Example::
>> +
>> +	  # mkdir ctrl1
>> +	  # echo "global_assign_ctrl_inherit_mon_per_cpu:group=ctrl1//" \
>> +	        > info/kernel_mode
>> +	  # echo 0-3 > ctrl1/kmode_cpus_list
>> +	  # cat ctrl1/kmode_cpus
>> +	  f
>> +	  # cat ctrl1/kmode_cpus_list
>> +	  0-3
>> +
>> +"kmode_cpus_list":
>> +	Just like "kmode_cpus", only using ranges of CPUs instead of bitmasks.
>> +	Writable with the same semantics and restrictions as "kmode_cpus".
>> +
>>   
>>   When control is enabled all CTRL_MON groups will also contain:
>>   
>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index 7b06c3b3f00e..8ecd107368b3 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -423,6 +423,151 @@ static int rdtgroup_kmode_cpus_show(struct kernfs_open_file *of,
>>   	return ret;
>>   }
>>   
>> +/**
>> + * kmode_cpus_write() - Update @rdtgrp's kmode_cpu_mask from @newmask
>> + * @rdtgrp:	Resctrl group whose kmode_cpu_mask is being updated.
>> + * @kmode:	Kernel-mode policy currently active on @rdtgrp.
>> + * @newmask:	Set of online CPUs scoped for @rdtgrp's kernel-mode binding.
>> + * @tmpmask:	Caller-allocated scratch cpumask used to compute the
>> + *		incremental enable/disable deltas; contents on entry are
>> + *		ignored and on return are unspecified.
> 
> ah - this is where "incremental" comes from. I think this is an implementation
> detail that should be invisible to user space.

Ok. Will change.

> 
>> + *
>> + * Compute the difference between @rdtgrp->kmode_cpu_mask and @newmask
>> + * and call resctrl_arch_configure_kmode() only on the CPUs whose enable
>> + * state actually changes:
>> + *
>> + *   - disable on (old & ~new)
>> + *   - enable on (new & ~old)
>> + *
>> + * Then copy @newmask into @rdtgrp->kmode_cpu_mask so subsequent
>> + * show/write operations reflect the updated scope.
> 
> This can be seen from the code.

ack.

> 
>> + */
>> +static void kmode_cpus_write(struct rdtgroup *rdtgrp, enum resctrl_kernel_mode kmode,
>> +			     cpumask_var_t newmask, cpumask_var_t tmpmask)
>> +{
>> +	bool assign_mon = (kmode == GLOBAL_ASSIGN_CTRL_ASSIGN_MON_PER_CPU);
>> +	u32 closid, rmid;
>> +
>> +	closid = rdtgrp->closid;
>> +	rmid = rdtgrp->mon.rmid;
>> +
>> +	/* CPUs dropped from this group: old & ~newmask. */
>> +	cpumask_andnot(tmpmask, &rdtgrp->kmode_cpu_mask, newmask);
>> +	if (!cpumask_empty(tmpmask))
>> +		resctrl_arch_configure_kmode(tmpmask, closid, rmid, assign_mon, false);
>> +
>> +	/* CPUs newly added: newmask & ~old. */
>> +	cpumask_andnot(tmpmask, newmask, &rdtgrp->kmode_cpu_mask);
>> +	if (!cpumask_empty(tmpmask))
>> +		resctrl_arch_configure_kmode(tmpmask, closid, rmid, assign_mon, true);
>> +
>> +	cpumask_copy(&rdtgrp->kmode_cpu_mask, newmask);
>> +}
>> +
>> +/**
>> + * rdtgroup_kmode_cpus_write() - Sysfs write handler for kmode_cpus[_list]
>> + * @of:		kernfs open file (selects bitmap vs range-list parsing via
>> + *		is_cpu_list()).
>> + * @buf:	NUL-terminated input from userspace.
>> + * @nbytes:	Length of @buf, returned on success.
>> + * @off:	File offset (unused).
>> + *
>> + * Parses @buf into a cpumask and rejects:
>> + *   - pseudo-locked / pseudo-lock-setup groups,
>> + *   - writes when INHERIT_CTRL_AND_MON is active or to a group other than
>> + *     resctrl_kcfg.k_rdtgrp (stale fds opened before an info/kernel_mode
>> + *     change),
>> + *   - malformed input,
>> + *   - masks containing offline CPUs.
> 
> This just describes the code and seems unnecessarry.

ok. will remove.

> 
> 
>> + *
>> + * Validated masks are passed to kmode_cpus_write() to update
>> + * @rdtgrp->kmode_cpu_mask and reprogram hardware incrementally.
>> + * Errors are reported in last_cmd_status.
>> + *
>> + * Return: @nbytes on success, -ENOENT if the group has been deleted,
>> + * -EINVAL for pseudo-locked or pseudo-lock-setup groups, malformed input, or
>> + * offline CPUs in the requested mask, -EBUSY if INHERIT_CTRL_AND_MON is active
>> + * or the group is not resctrl_kcfg.k_rdtgrp, and -ENOMEM if the scratch
>> + * cpumasks cannot be allocated.
>> + */
>> +static ssize_t rdtgroup_kmode_cpus_write(struct kernfs_open_file *of,
>> +					 char *buf, size_t nbytes, loff_t off)
>> +{
>> +	cpumask_var_t tmpmask, newmask;
>> +	struct rdtgroup *rdtgrp;
>> +	int ret;
>> +
>> +	if (!buf)
>> +		return -EINVAL;
>> +
>> +	if (!zalloc_cpumask_var(&tmpmask, GFP_KERNEL))
>> +		return -ENOMEM;
>> +	if (!zalloc_cpumask_var(&newmask, GFP_KERNEL)) {
>> +		free_cpumask_var(tmpmask);
>> +		return -ENOMEM;
>> +	}
> 
> Please see related recent changes to resctrl:
> commit 242c0ab4d51d ("fs/resctrl: Change last_cmd_status custom during input parsing")
> 
> For comparison you can view latest
> implementation of rdtgroup_cpus_write().

Ok. Will look into it.

> 
>> +
>> +	rdtgrp = rdtgroup_kn_lock_live(of->kn);
>> +	if (!rdtgrp) {
>> +		ret = -ENOENT;
>> +		goto unlock;
>> +	}
>> +
>> +	rdt_last_cmd_clear();
> 
> rdtgroup_kn_lock_live() now calls rdt_last_cmd_clear().

Yes. Will update.

> 
>> +
>> +	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED ||
>> +	    rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) {
>> +		ret = -EINVAL;
>> +		rdt_last_cmd_puts("Pseudo-locked group cannot host kernel-mode binding\n");
>> +		goto unlock;
>> +	}
> 
> Similar to previous comment I believe this sprinkling of pseudo-locked mode
> checks can be avoided by ensuring that (a) a pseudo-locked/locksetup group
> cannot be assigned to a kernel mode, and (b) a group assigned to a kernel mode
> cannot have its mode changed.

ack.

> 
> In this case then resctrl_kmode_cfg::k_rdtgroup can never be a pseudo-locked group
> and the if (resctrl_kcfg.k_rdtgrp != rdtgrp) check below would be sufficient?

Yes.

> 
>> +
>> +	if (resctrl_kcfg.kmode_cur == INHERIT_CTRL_AND_MON) {
>> +		ret = -EBUSY;
>> +		rdt_last_cmd_puts("No active kernel-mode binding\n");
> 
> I do not know where this "binding" term came from and all of a sudden it is
> everywhere. The inconsistent constantly changing terms used in this series makes
> it difficult to follow.

Yes. Will change it to "assigned"

> 
>> +		goto unlock;
>> +	}
>> +
>> +	/*
>> +	 * The visibility layer (kernfs_show()) prevents fresh open() on a
> 
> Visibility layer? Another new term. After this introduction it is the only
> instance of this term in all kernel source.

Will remove this. Let me rewrite the comment.

Thanks
Babu