Messages in this thread | | | Date | Thu, 24 Aug 2023 17:54:28 +0100 | Subject | Re: [PATCH v5 08/24] x86/resctrl: Allocate the cleanest CLOSID by searching closid_num_dirty_rmid | From | James Morse <> |
| |
Hi Fenghua,
On 15/08/2023 03:59, Fenghua Yu wrote: > On 7/28/23 09:42, James Morse wrote: >> 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. >> >> Instead of allocating the first free CLOSID, on architectures where >> CONFIG_RESCTRL_RMID_DEPENDS_ON_COSID is enabled, search >> closid_num_dirty_rmid[] to find the cleanest CLOSID. >> >> The CLOSID found is returned to closid_alloc() for the free list >> to be updated.
>> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c >> index 44addc0126fc..c268aa5925c7 100644 >> --- a/arch/x86/kernel/cpu/resctrl/monitor.c >> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c >> @@ -379,6 +379,48 @@ static struct rmid_entry *resctrl_find_free_rmid(u32 closid) >> return ERR_PTR(-ENOSPC); >> } >> +/** >> + * resctrl_find_cleanest_closid() - Find a CLOSID where all the associated >> + * RMID are clean, or the CLOSID that has >> + * the most clean RMID. >> + * >> + * MPAM's equivalent of RMID are per-CLOSID, meaning a freshly allocated CLOSID >> + * may not be able to allocate clean RMID. To avoid this the allocator will >> + * choose the CLOSID with the most clean RMID. >> + * >> + * When the CLOSID and RMID are independent numbers, the first free CLOSID will >> + * be returned. >> + */ >> +int resctrl_find_cleanest_closid(void) >> +{ >> + u32 cleanest_closid = ~0, iter_num_dirty; >> + int i = 0; >> + >> + lockdep_assert_held(&rdtgroup_mutex); >> + >> + if (!IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) >> + return -EIO; >> + >> + for (i = 0; i < closids_supported(); i++) { >> + if (closid_allocated(i)) >> + continue; >> + >> + iter_num_dirty = closid_num_dirty_rmid[i]; >> + if (iter_num_dirty == 0) >> + return i; >> + >> + if (cleanest_closid == ~0) >> + cleanest_closid = i; >> + >> + if (iter_num_dirty < closid_num_dirty_rmid[cleanest_closid]) >> + cleanest_closid = i; >> + } >> + >> + if (cleanest_closid == ~0) >> + return -ENOSPC; >> + return cleanest_closid; >> +} >> + > > resctrl_find_cleanest_closid() is not empty on x86 after compiled although it's very > short. After all, the function is irrelevant to x86 and could be completely empty on x86. > > If put the function in > #ifdef CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID > resctrl_find_cleanest_closid() > ... > #else > resctrl_find_cleanest_closid() {} > #endif > > It's fully empty on x86.
I think you forgot the return type. You'd still need to return an error in the stub - which is what the existing function will be reduced to by the compilers dead code elimination.
Here is the existing function on x86: | 0000000000000680 <resctrl_find_cleanest_closid>: | 680: f3 0f 1e fa endbr64 | 684: b8 fb ff ff ff mov $0xfffffffb,%eax | 689: e9 00 00 00 00 jmp 68e <resctrl_find_cleanest_closid+0xe> | 68e: 66 90 xchg %ax,%ax | 690: 90 nop | 691: 90 nop [and quite a few more nops]
and here is the stub you propose: | int resctrl_find_cleanest_closid_as_a_stub(void) | { | return -EIO; | }
which builds as: | 00000000000006a0 <resctrl_find_cleanest_closid_as_a_stub>: | 6a0: f3 0f 1e fa endbr64 | 6a4: b8 fb ff ff ff mov $0xfffffffb,%eax | 6a9: e9 00 00 00 00 jmp 6ae <resctrl_find_cleanest_closid_as_a_s> | 6ae: 66 90 xchg %ax,%ax | 6b0: 90 nop | 6b1: 90 nop [and quite a few more nops]
The only difference is the #ifdeffery makes this hard to read, and means CI systems need extra Kconfig files to get good coverage of this code.
It's not the 90s anymore: we no-one wants to see an #ifdef!
James
| |