Re: [PATCH v5 13/16] fs/resctrl: Deactivate the kernel mode association when a group is removed

[Babu Moger]

Hi Reinette,

On 9/16/26 00:36, Reinette Chatre wrote:
> Hi Babu,
>
> On 8/26/26 12:32 PM, Babu Moger wrote:
> > +/**
> > + * rdtgroup_kmode_detach() - Detach @rdtgrp from a kernel mode association
> > + * @rdtgrp: Resctrl group being removed or torn down.
> > + *
> > + * If @rdtgrp backs the active kernel mode association, disable the
> > + * hardware association programmed for the group's kmode_cpu_mask,
> > + * clear the associated kernel mode state, and restore
> > + * RESCTRL_INHERIT_USER as the active mode.
>
> I am not going to keep pointing these out but please check entire series
> and remove all these verbatim code descriptions from function comments.

Ack.

> > + */
> > +static void rdtgroup_kmode_detach(struct rdtgroup *rdtgrp)
>
> Similar to previous comments this only seems relevant to the
> global per-CPU kernel mode so function can be made specific to it.

ok.

> > +{
> > +	if (!rdtgrp || !rdtgrp->kmode)
> > +		return;
> > +
> > +	if (resctrl_kcfg.active.k_rdtgrp != rdtgrp) {
>
> Related to comment in previous patch this double check of rdtgrp->kmode
> and resctrl_kcfg.active.k_rdtgrp seems to support that only one is needed?

Yes. We can remove rdtgrp->kmode field.

> > +		pr_warn("resctrl: kernel mode group not valid\n");
>
> Please just fix this file's pr_fmt(fmt) instead.


Hmm... It looks like we have a pr_fmt(fmt) inconsistency across multiple 
files.

Some files use:

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt


while others use:

#define pr_fmt(fmt) "resctrl: " fmt


My understanding is that these will produce different log prefixes. For 
example, KBUILD_MODNAME will typically use the object file name, whereas 
"resctrl" provides a consistent subsystem-wide prefix.

Do you think we should standardize all files to use the same prefix? If 
so, I believe that would be better handled as a separate cleanup patch 
rather than being mixed into this change.

Thanks,
Babu