Messages in this thread | | | Date | Thu, 27 Apr 2023 15:20:07 +0100 | Subject | Re: [PATCH v3 17/19] x86/resctrl: Allow overflow/limbo handlers to be scheduled on any-but cpu | From | James Morse <> |
| |
Hi Ilpo,
On 21/03/2023 15:25, Ilpo Järvinen wrote: > On Tue, 21 Mar 2023, Ilpo Jï¿œrvinen wrote: >> On Mon, 20 Mar 2023, James Morse wrote: >> >>> When a CPU is taken offline resctrl may need to move the overflow or >>> limbo handlers to run on a different CPU. >>> >>> Once the offline callbacks have been split, cqm_setup_limbo_handler() >>> will be called while the CPU that is going offline is still present >>> in the cpu_mask. >>> >>> Pass the CPU to exclude to cqm_setup_limbo_handler() and >>> mbm_setup_overflow_handler(). These functions can use a variant of >>> cpumask_any_but() when selecting the CPU. -1 is used to indicate no CPUs >>> need excluding.
>>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h >>> index 3eb5b307b809..47838ba6876e 100644 >>> --- a/arch/x86/kernel/cpu/resctrl/internal.h >>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h >>> @@ -78,6 +78,37 @@ static inline unsigned int cpumask_any_housekeeping(const struct cpumask *mask) >>> return cpu; >>> } >>> >>> +/** >>> + * cpumask_any_housekeeping_but() - Chose any cpu in @mask, preferring those >>> + * that aren't marked nohz_full, excluding >>> + * the provided CPU >>> + * @mask: The mask to pick a CPU from. >>> + * @exclude_cpu:The CPU to avoid picking. >>> + * >>> + * Returns a CPU from @mask, but not @but. If there are houskeeping CPUs that >>> + * don't use nohz_full, these are preferred. >>> + * Returns >= nr_cpu_ids if no CPUs are available. >>> + */ >>> +static inline unsigned int >>> +cpumask_any_housekeeping_but(const struct cpumask *mask, int exclude_cpu) >>> +{ >>> + int cpu, hk_cpu; >>> + >>> + cpu = cpumask_any_but(mask, exclude_cpu); >>> + if (tick_nohz_full_cpu(cpu)) { >>> + hk_cpu = cpumask_nth_andnot(0, mask, tick_nohz_full_mask); >>> + if (hk_cpu == exclude_cpu) { >>> + hk_cpu = cpumask_nth_andnot(1, mask, >>> + tick_nohz_full_mask);
>> I'm left to wonder if it's okay to alter tick_nohz_full_mask in resctrl >> code??
Why do you think cpumask_nth_andnot() modifies its arguments?
The cpumask arguments to cpumask_nth_andnot() are const.
> I suppose it should do instead: > hk_cpu = cpumask_nth_and(0, mask, tick_nohz_full_mask); > if (hk_cpu == exclude_cpu) > hk_cpu = cpumask_next_and(hk_cpu, mask, tick_nohz_full_mask); >
Removing the 'not' changes the behaviour. hk_cpu is now guaranteed to be a nohz_full CPU. This needs to prefer CPUs that are not in that mask.
Passing 'hk_cpu' the second time doesn't look right, hk_cpu is a CPU-number, not a count of the 'nth CPU to find', which is what the argument expects. For example: If the mask only has CPU 10-12, where CPU 10 should be excluded, its possible the first attempt for the 0th CPU returns 10... in which case I want to pass '1' now I know that the 0th is the excluded CPU. If I pass 10 I expect an error, as there aren't 10 bits set in the mask.
Thanks,
James
| |