Re: [RESEND PATCH v4 10/15] fs/resctrl: Reset the kernel-mode binding when an rdtgroup is removed

[Babu Moger]

Hi Reinette,

On 8/10/26 22:29, Reinette Chatre wrote:
> Hi Babu,
>
> On 7/7/26 2:50 PM, Babu Moger wrote:
> > Resctrl keeps track of the rdtgroup backing the active global kernel-mode
> > policy, including the group's CPU mask used to program kernel-mode
> > associations.
> >
> > When that rdtgroup is deleted, resctrl must update both its internal state
> > and the hardware configuration associated with the group's kmode_cpu_mask.
>
> Instead of updating the hardware configuration itself resctrl fs will request
> the underlying architecture to do so.

Yes. Sure.

> > Introduce rdtgroup_config_kmode_reset() to disable the active kernel-mode
> > association for the bound group's CPU mask and to clear its kernel-mode
> > state. Invoke this helper from rdtgroup_kmode_detach(), and reset
> > resctrl_kcfg to INHERIT_CTRL_AND_MON. Ensure rdtgroup_kmode_detach() is
> > called during group removal and filesystem teardown, before the bound
> > rdtgroup is freed.
>
> (Same comment as before - please do not describe the code details and write in
> imperative tone. Please check all changelogs.)

Ack.

> I think it will be helpful to add reminder why "reset" call needs to
> include all the existing assignments. This could be part of resctrl fs "contract"
> with architecture on when and how it calls resctrl_arch_configure_kmode().

Sure.>
> > Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> > ---
> > v4: Re-wrote the changelog.
> >      Added the call free_all_child_rdtgrp() and rmdir_all_sub()
> >      Simplified the code comments.
> >
> > v3: New patch to handle the kernel_mode clean up.
> > ---
> >   fs/resctrl/rdtgroup.c | 62 +++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 62 insertions(+)
> >
> > diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> > index 0d5c94169d03..21659fd75850 100644
> > --- a/fs/resctrl/rdtgroup.c
> > +++ b/fs/resctrl/rdtgroup.c
> > @@ -1127,6 +1127,61 @@ static int resctrl_kernel_mode_show(struct kernfs_open_file *of,
> >   	return ret;
> >   }
> >   
> > +/**
> > + * rdtgroup_config_kmode_reset() - Tear down the kernel-mode binding on @rdtgrp
> > + * @rdtgrp:	Resctrl group whose kernel-mode binding is being released.
> > + *		May be %NULL when no group is currently bound, in which case
> > + *		this is a no-op.
> > + * @kmode:	Kernel-mode policy currently active on @rdtgrp.
> > + *
> > + * Reset the kernel-mode binding on the CPUs in @rdtgrp's @kmode_cpu_mask.
> > + */
> > +static void rdtgroup_config_kmode_reset(struct rdtgroup *rdtgrp,
>
> So far the word "active" has been used a lot when referring to the "active"
> kernel mode. To complement this I think it will make this work easier to
> understand if this is renamed to rdtgroup_kmode_deactivate() and to match
> it rdtgroup_config_kmode() could be renamed to rdtgroup_kmode_activate()?

Sure.

> > +					enum resctrl_kernel_mode kmode)
> > +{
> > +	bool assign_mon = false;
> > +
> > +	if (!rdtgrp)
> > +		return;
> > +
> > +	if (kmode == INHERIT_CTRL_AND_MON)
> > +		goto out_clear;
>
> When kmode is INHERIT_CTRL_AND_MON then there should not be a resource group
> assigned so the work done at "out_clear" is not necessary?

Correct. Not required.

> > +
> > +	if (kmode == GLOBAL_ASSIGN_CTRL_ASSIGN_MON_PER_CPU)
> > +		assign_mon = true;
>
> Could use the same assignment as resctrl_kmode_online_cpu() and kmode_cpus_write() ?

Sure.

Thanks
Babu