Re: [PATCH v6 13/18] fs/resctrl: Deactivate kernel mode associations when a group is removed

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Babu,

On 10/19/26 8:47 AM, Babu Moger wrote:
> On 10/15/26 10:43, Reinette Chatre wrote:
>> On 10/1/26 8:10 AM, Babu Moger wrote:
>>> The active kernel mode association is backed by either a CTRL_MON or
>>> MON group.
>>
>> This context does not reflect the code at this point in series but what
>> behavior will be after entire series is applied. It is also written as
>> general but specific to the global per-CPU kernel mode.
>>
>>> When the backing group is removed, or when resctrl is unmounted, clear
>>> the programmed association and restore the default settings.
>>
>> Please provide more context to user and help reader to understand what the
>> patch does without needing to decipher the patch.
> 
> "The global per-CPU kernel mode, RESCTRL_GLOBAL_ENABLE_PER_CPU, allows kernel work to use allocation and monitoring associations that differ from those of the originating user task. When the mode is active, "info/kernel_mode" displays the backing mode. If the backing group is removed, or resctrl is unmounted, the association becomes invalid.
> 
> Clear the programmed association and revert the kernel mode to the default "inherit user" behavior. This ensures that kernel work continues to use the allocation and monitoring associations of the originating user task until a new kernel mode association is configured.
> 
> Document this user-visible behavior in resctrl.rst."
> 
>>
>> For example, "restore the default settings" is a very vague way to
>> describe that removing the group backing the global per-CPU kernel mode
>> automatically switches the kernel mode to "inherit user". Please explain
>> at high level what the code does and why. As a hypothetical, why not
>> keep the kernel mode as "global per-CPU kernel mode" but switch the assigned
>> group to the default group? This would match most other similar resctrl behaviors,
>> no?
> 
> My assuption, you are not asking me to change it to "global per-CPU kernel mode" when the group is removed.

Not specifically, but since the behavior is different from the typical resctrl behavior that,
for example moves a removed group's tasks etc. to the default group, it would be valuable to
know *why* this behavior was chosen.

...

>>> + */
>>> +static void rdtgroup_kmode_global_deactivate(struct rdtgroup *rdtgrp)
>>> +{
>>> +    resctrl_configure_kmode_global(&rdtgrp->kmode_cpu_mask, rdtgrp, false);
>>> +    cpumask_clear(&rdtgrp->kmode_cpu_mask);
>>> +}
>>> +
>>> +/*
>>> + * rdtgroup_kmode_global_detach() - Detach @rdtgrp from the global per-CPU mode
>>> + * @rdtgrp: Resctrl group being removed or torn down.
>>> + *
>>> + * If @rdtgrp backs the active association, disable it, clear kernel mode
>>> + * state, and restore RESCTRL_INHERIT_USER.
>>> + */
>>> +static void rdtgroup_kmode_global_detach(struct rdtgroup *rdtgrp)
>>> +{
>>> +    if (resctrl_kmode.rdtgrp != rdtgrp)
>>
>> Please do not use tests on resctrl_kmode.rdtgrp as synonym for
>> resctrl_kmode.active == RESCTRL_GLOBAL_ENABLE_PER_CPU.
> 
> Not really. This was a check to make sure kernel mode is pointing to correct group that is being removed.

The problem is this "global per-CPU kernel mode" specific function is called without ever
checking if "global per-CPU kernel mode" is active. Instead it uses one of the parameter
values as synonym for whether the "global per-CPU kernel mode" is active. What is a future
developer to understand from this? This implies there are two *interchangeable* ways to determine
whether "global per-CPU kernel mode" is active ... sometimes
resctrl_kmode.active == RESCTRL_GLOBAL_ENABLE_PER_CPU is/should be used and sometimes the value
of resctrl_kmode.rdtgrp is/should be used. How should somebody interpret this? Are these
two interchangeable tests (and thus making one redundant) or is the test dependent on the scenario
(which is not explained anywhere).

Reinette