Re: [PATCH v1 01/31] x86/resctrl: Fix allocation of cleanest CLOSID on platforms with no monitors

[Dave Martin]

On Tue, Apr 09, 2024 at 10:05:33AM +0200, David Hildenbrand wrote:
> On 21.03.24 17:50, James Morse wrote:
> > commit 6eac36bb9eb0 ("x86/resctrl: Allocate the cleanest CLOSID by
> > searching closid_num_dirty_rmid") added a Kconfig option that causes
> > resctrl to search for the CLOSID with the fewest dirty cache lines when
> > creating a new control group. This depends on the values read from the
> > llc_occupancy counters.

[...]

> It's weird to not see RESCTRL_RMID_DEPENDS_ON_CLOSID appear in any Kconfig file.
> I guess it will all make sense once the refactoring is done :)

Agreed; a stub Kconfig item could be added, but since the file layout
and naming conventions change after this patch, doing this would
probably just create noise in the series though.

Looking at <linux/kconfig.h> (yikes!), IS_ENABLED() is designed to do
the right thing for non-existing Kconfigs...

If nobody is too concerned about the temporarily dangling IS_ENABLED()s
in this series, I won't propose any change here.


> As Reinette comments, likely we want here:
> 
> Fixes: 6eac36bb9eb0 ("x86/resctrl: Allocate the cleanest CLOSID by searching closid_num_dirty_rmid")

Noted for James' attention.

> 
> > Signed-off-by: James Morse <james.morse@xxxxxxx>
> > ---
> >   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > index 011e17efb1a6..1767c1affa60 100644
> > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > @@ -149,7 +149,8 @@ static int closid_alloc(void)
> >   	lockdep_assert_held(&rdtgroup_mutex);
> > -	if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
> > +	if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID) &&
> > +	    is_llc_occupancy_enabled()) {
> >   		cleanest_closid = resctrl_find_cleanest_closid();
> >   		if (cleanest_closid < 0)
> >   			return cleanest_closid;
> 
> Reviewed-by: David Hildenbrand <david@xxxxxxxxxx>
> 
> -- 
> Cheers,
> 
> David / dhildenb

Thanks; noted for James' attention.

Cheers
---Dave