Re: [PATCH 06/18] x86/resctrl: Allow the allocator to check if a CLOSID can allocate clean RMID
From: James Morse
Date: Thu Nov 24 2022 - 09:22:02 EST
Hello,
On 10/11/2022 10:50, Shaopeng Tan (Fujitsu) wrote:
> Hi James
>
>> MPAM's PMG bits extend its PARTID space, meaning the same PMG value can
>> be used for different control groups.
>>
>> This means once a CLOSID is allocated, all its monitoring ids may still be dirty,
>> and held in limbo.
>>
>> Add a helper to allow the CLOSID allocator to check if a CLOSID has dirty RMID
>> values. This behaviour is enabled by a kconfig option selected by the
>> architecture, which avoids a pointless search for x86.
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c
>> b/arch/x86/kernel/cpu/resctrl/monitor.c
>> index 59da256a77fe..99854ef4dee4 100644
>> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
>> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
>> @@ -320,6 +320,37 @@ static struct rmid_entry *resctrl_find_free_rmid(u32
>> +bool resctrl_closid_is_dirty(u32 closid) {
>> + struct rmid_entry *entry;
>> + int i;
>> +
>> + lockdep_assert_held(&rdtgroup_mutex);
>> +
>> + if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
>> + return false;
>> a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index ac88610a6946..59f33adcf6f8 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -119,14 +119,18 @@ static void closid_init(void)
>>
>> static int closid_alloc(void)
>> {
>> - u32 closid = ffs(closid_free_map);
>> + u32 closid;
>>
>> - if (closid == 0)
>> - return -ENOSPC;
>> - closid--;
>> - closid_free_map &= ~(1 << closid);
>> + for_each_set_bit(closid, &closid_free_map, closid_free_map_len) {
>> + if
>> (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID) &&
>> + resctrl_closid_is_dirty(closid))
> IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID) is redundant here,
> since it is also at the beginning of function resctrl_closid_is_dirty(closid).
This is true. I included it because resctrl_closid_is_dirty() is in a different
compilation unit, so the compiler can't know it does nothing if that config option isn't
enabled. This avoided a pointless call to a function that does nothing. But you're right
it would be more readable without it, and creating a control group is hardly a performance
critical path. I'll remove it.
Thanks,
James