Messages in this thread | | | Date | Wed, 25 Oct 2023 18:56:47 +0100 | Subject | Re: [PATCH v6 13/24] x86/resctrl: Queue mon_event_read() instead of sending an IPI | From | James Morse <> |
| |
Hi Reinette,
On 03/10/2023 22:17, Reinette Chatre wrote: > On 9/14/2023 10:21 AM, James Morse wrote: >> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c >> index b44c487727d4..bd263b9a0abd 100644 >> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c >> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c >> @@ -19,6 +19,7 @@ >> #include <linux/kernfs.h> >> #include <linux/seq_file.h> >> #include <linux/slab.h> >> +#include <linux/tick.h> >> #include "internal.h" >> > > Please keep the empty line between groups of header files.
(in this case, adding one, but sure)
>> @@ -520,12 +521,24 @@ int rdtgroup_schemata_show(struct kernfs_open_file *of, >> return ret; >> } >> >> +static int smp_mon_event_count(void *arg) >> +{ >> + mon_event_count(arg); >> + >> + return 0; >> +} >> + >> void mon_event_read(struct rmid_read *rr, struct rdt_resource *r, >> struct rdt_domain *d, struct rdtgroup *rdtgrp, >> int evtid, int first) >> { >> + int cpu; >> + >> + /* When picking a CPU from cpu_mask, ensure it can't race with cpuhp */ > > This comment is not accurate at this point. It should accompany the code it applies to. > >> + lockdep_assert_held(&rdtgroup_mutex);
This refers to the d->cpu_mask calls further down this function. These are written to by the cpuhp callbacks, rdtgroup_mutex is what prevents the cpuhp callback from running at the same time as mon_event_read(). If that mutex weren't held, you could pick an offline CPU.
Patch 24 changes this to be lockdep_asser_cpus_held(), as the mutex is no longer used for this purpose.
This got added here instead of patch-24 because I've added additional use of d->cpu_mask, these things serve to document how that is safe. If you prefer I'll leave it unsaid here, and add it with all the others in patch24.
Thanks,
James
| |