arch/x86/kernel/cpu/resctrl/internal.h | 5 ++- arch/x86/kernel/cpu/resctrl/core.c | 55 +++++++++-------------- arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 40 ++++------------- arch/x86/kernel/cpu/resctrl/rdtgroup.c | 12 +---- 4 files changed, 34 insertions(+), 78 deletions(-)
reset_all_ctrls() and resctrl_arch_update_domains() use on_each_cpu_mask()
to call rdt_ctrl_update() on potentially one CPU from each domain.
But this means rdt_ctrl_update() needs to figure out which domain to
apply changes to. Doing so requires a search of all domains in a resource,
which can only be done safely if cpus_lock is held. Both callers do hold
this lock, but there isn't a way for a function called on another CPU
via IPI to verify this.
Commit c0d848fcb09d ("x86/resctrl: Remove lockdep annotation that triggers
false positive") removed the incorrect assertions.
Add the target domain to the msr_param structure and
call rdt_ctrl_update() for each domain separately using
smp_call_function_single(). This means that rdt_ctrl_update() doesn't
need to search for the domain and get_domain_from_cpu() can safely assert
that the cpus_lock is held since the remaining callers do not use IPI.
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
Changes since V4: Link: https://lore.kernel.org/all/20240228193717.8170-1-tony.luck@intel.com/
Reinette: Only assign "cpu" once in resctrl_arch_update_domains() [but
see change from James below]
James: Use smp_call_function_any() instead of cpumask_any() +
smp_call_function_single() to avoid unnecessary IPI in both
resctrl_arch_update_domains() and reset_all_ctrls(). This
eliminates a need for the "cpu" local variable.
Tony Luck (2):
x86/resctrl: Pass domain to target CPU
x86/resctrl: Simplify call convention for MSR update functions
arch/x86/kernel/cpu/resctrl/internal.h | 5 ++-
arch/x86/kernel/cpu/resctrl/core.c | 55 +++++++++--------------
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 40 ++++-------------
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 12 +----
4 files changed, 34 insertions(+), 78 deletions(-)
base-commit: c0d848fcb09d80a5f48b99f85e448185125ef59f
--
2.43.0
Hi Boris,
Could you please consider this series for inclusion?
Thank you very much.
Reinette
On 3/8/2024 1:38 PM, Tony Luck wrote:
> reset_all_ctrls() and resctrl_arch_update_domains() use on_each_cpu_mask()
> to call rdt_ctrl_update() on potentially one CPU from each domain.
>
> But this means rdt_ctrl_update() needs to figure out which domain to
> apply changes to. Doing so requires a search of all domains in a resource,
> which can only be done safely if cpus_lock is held. Both callers do hold
> this lock, but there isn't a way for a function called on another CPU
> via IPI to verify this.
>
> Commit c0d848fcb09d ("x86/resctrl: Remove lockdep annotation that triggers
> false positive") removed the incorrect assertions.
>
> Add the target domain to the msr_param structure and
> call rdt_ctrl_update() for each domain separately using
> smp_call_function_single(). This means that rdt_ctrl_update() doesn't
> need to search for the domain and get_domain_from_cpu() can safely assert
> that the cpus_lock is held since the remaining callers do not use IPI.
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
>
> ---
> Changes since V4: Link: https://lore.kernel.org/all/20240228193717.8170-1-tony.luck@intel.com/
>
> Reinette: Only assign "cpu" once in resctrl_arch_update_domains() [but
> see change from James below]
>
> James: Use smp_call_function_any() instead of cpumask_any() +
> smp_call_function_single() to avoid unnecessary IPI in both
> resctrl_arch_update_domains() and reset_all_ctrls(). This
> eliminates a need for the "cpu" local variable.
>
> Tony Luck (2):
> x86/resctrl: Pass domain to target CPU
> x86/resctrl: Simplify call convention for MSR update functions
>
> arch/x86/kernel/cpu/resctrl/internal.h | 5 ++-
> arch/x86/kernel/cpu/resctrl/core.c | 55 +++++++++--------------
> arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 40 ++++-------------
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 12 +----
> 4 files changed, 34 insertions(+), 78 deletions(-)
>
>
> base-commit: c0d848fcb09d80a5f48b99f85e448185125ef59f
Hi Tony,
On 3/8/2024 1:38 PM, Tony Luck wrote:
> reset_all_ctrls() and resctrl_arch_update_domains() use on_each_cpu_mask()
> to call rdt_ctrl_update() on potentially one CPU from each domain.
>
> But this means rdt_ctrl_update() needs to figure out which domain to
> apply changes to. Doing so requires a search of all domains in a resource,
> which can only be done safely if cpus_lock is held. Both callers do hold
> this lock, but there isn't a way for a function called on another CPU
> via IPI to verify this.
>
> Commit c0d848fcb09d ("x86/resctrl: Remove lockdep annotation that triggers
> false positive") removed the incorrect assertions.
>
> Add the target domain to the msr_param structure and
> call rdt_ctrl_update() for each domain separately using
> smp_call_function_single(). This means that rdt_ctrl_update() doesn't
> need to search for the domain and get_domain_from_cpu() can safely assert
> that the cpus_lock is held since the remaining callers do not use IPI.
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
>
> ---
> Changes since V4: Link: https://lore.kernel.org/all/20240228193717.8170-1-tony.luck@intel.com/
>
> Reinette: Only assign "cpu" once in resctrl_arch_update_domains() [but
> see change from James below]
>
> James: Use smp_call_function_any() instead of cpumask_any() +
> smp_call_function_single() to avoid unnecessary IPI in both
> resctrl_arch_update_domains() and reset_all_ctrls(). This
> eliminates a need for the "cpu" local variable.
Great catch, thank you for doing this.
Also, could you please stop referring to previous versions (in this
case it even follows v3) using "In-Reply-To:"? Although v5 here
follows v3, not v4. You previously [1] agreed and did so for v4
but not here.
Reinette
[1] https://lore.kernel.org/lkml/SJ1PR11MB60839AD3BC4DB15BF5B0BBE0FC562@SJ1PR11MB6083.namprd11.prod.outlook.com/
> Also, could you please stop referring to previous versions (in this > case it even follows v3) using "In-Reply-To:"? Although v5 here > follows v3, not v4. You previously [1] agreed and did so for v4 > but not here. Reinette sorry. When sending a patch, or series I create a SEND shell script with the "git send-emal" command line with all the "--to" and "--cc" arguments to send to the right people and mailing lists. I copied an old SEND script and forgot to delete the "--in-reply-to" argument from the script :-( Will try to do better. -Tony
© 2016 - 2026 Red Hat, Inc.