Messages in this thread | | | Date | Thu, 9 Nov 2023 14:47:55 -0600 | From | "Moger, Babu" <> | Subject | Re: [PATCH v7 16/24] x86/resctrl: Make resctrl_mounted checks explicit |
| |
On 10/25/23 13:03, James Morse wrote: > The rdt_enable_key is switched when resctrl is mounted, and used to > prevent a second mount of the filesystem. It also enables the > architecture's context switch code. > > This requires another architecture to have the same set of static-keys, > as resctrl depends on them too. The existing users of these static-keys > are implicitly also checking if the filesystem is mounted. > > Make the resctrl_mounted checks explicit: resctrl can keep track of > whether it has been mounted once. This doesn't need to be combined with > whether the arch code is context switching the CLOSID. > > rdt_mon_enable_key is never used just to test that resctrl is mounted, > but does also have this implication. Add a resctrl_mounted to all uses > of rdt_mon_enable_key. This will allow rdt_mon_enable_key to be swapped > with a helper in a subsequent patch. > > This will allow the static-key changing to be moved behind resctrl_arch_ > calls. > > Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> > Tested-by: Peter Newman <peternewman@google.com> > Reviewed-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> > Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> > Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Babu Moger <babu.moger@amd.com> > > --- > Changes since v3: > * Removed a newline. > * Rephrased commit message > > Changes since v4: > * Rephrased comment. > > No changes since v6 > --- > arch/x86/kernel/cpu/resctrl/internal.h | 1 + > arch/x86/kernel/cpu/resctrl/monitor.c | 12 ++++++++++-- > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 23 +++++++++++++++++------ > 3 files changed, 28 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h > index bb07b9b01e36..5ce49503fcd8 100644 > --- a/arch/x86/kernel/cpu/resctrl/internal.h > +++ b/arch/x86/kernel/cpu/resctrl/internal.h > @@ -145,6 +145,7 @@ extern bool rdt_alloc_capable; > extern bool rdt_mon_capable; > extern unsigned int rdt_mon_features; > extern struct list_head resctrl_schema_all; > +extern bool resctrl_mounted; > > enum rdt_group_type { > RDTCTRL_GROUP = 0, > diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c > index 1ca8db8c82cc..843df6f64e0e 100644 > --- a/arch/x86/kernel/cpu/resctrl/monitor.c > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c > @@ -837,7 +837,11 @@ void mbm_handle_overflow(struct work_struct *work) > > mutex_lock(&rdtgroup_mutex); > > - if (!static_branch_likely(&rdt_mon_enable_key)) > + /* > + * If the filesystem has been unmounted this work no longer needs to > + * run. > + */ > + if (!resctrl_mounted || !static_branch_likely(&rdt_mon_enable_key)) > goto out_unlock; > > r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl; > @@ -870,7 +874,11 @@ void mbm_setup_overflow_handler(struct rdt_domain *dom, unsigned long delay_ms) > unsigned long delay = msecs_to_jiffies(delay_ms); > int cpu; > > - if (!static_branch_likely(&rdt_mon_enable_key)) > + /* > + * When a domain comes online there is no guarantee the filesystem is > + * mounted. If not, there is no need to catch counter overflow. > + */ > + if (!resctrl_mounted || !static_branch_likely(&rdt_mon_enable_key)) > return; > cpu = cpumask_any_housekeeping(&dom->cpu_mask); > dom->mbm_work_cpu = cpu; > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index fe6dfea471f2..9ce1f981c8e1 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > @@ -42,6 +42,9 @@ LIST_HEAD(rdt_all_groups); > /* list of entries for the schemata file */ > LIST_HEAD(resctrl_schema_all); > > +/* The filesystem can only be mounted once. */ > +bool resctrl_mounted; > + > /* Kernel fs node for "info" directory under root */ > static struct kernfs_node *kn_info; > > @@ -881,7 +884,7 @@ int proc_resctrl_show(struct seq_file *s, struct pid_namespace *ns, > mutex_lock(&rdtgroup_mutex); > > /* Return empty if resctrl has not been mounted. */ > - if (!static_branch_unlikely(&rdt_enable_key)) { > + if (!resctrl_mounted) { > seq_puts(s, "res:\nmon:\n"); > goto unlock; > } > @@ -2613,7 +2616,7 @@ static int rdt_get_tree(struct fs_context *fc) > /* > * resctrl file system can only be mounted once. > */ > - if (static_branch_unlikely(&rdt_enable_key)) { > + if (resctrl_mounted) { > ret = -EBUSY; > goto out; > } > @@ -2674,8 +2677,10 @@ static int rdt_get_tree(struct fs_context *fc) > if (rdt_mon_capable) > static_branch_enable_cpuslocked(&rdt_mon_enable_key); > > - if (rdt_alloc_capable || rdt_mon_capable) > + if (rdt_alloc_capable || rdt_mon_capable) { > static_branch_enable_cpuslocked(&rdt_enable_key); > + resctrl_mounted = true; > + } > > if (is_mbm_enabled()) { > r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl; > @@ -2949,6 +2954,7 @@ static void rdt_kill_sb(struct super_block *sb) > static_branch_disable_cpuslocked(&rdt_alloc_enable_key); > static_branch_disable_cpuslocked(&rdt_mon_enable_key); > static_branch_disable_cpuslocked(&rdt_enable_key); > + resctrl_mounted = false; > kernfs_kill_sb(sb); > mutex_unlock(&rdtgroup_mutex); > cpus_read_unlock(); > @@ -3918,7 +3924,7 @@ void resctrl_offline_domain(struct rdt_resource *r, struct rdt_domain *d) > * If resctrl is mounted, remove all the > * per domain monitor data directories. > */ > - if (static_branch_unlikely(&rdt_mon_enable_key)) > + if (resctrl_mounted && static_branch_unlikely(&rdt_mon_enable_key)) > rmdir_mondata_subdir_allrdtgrp(r, d->id); > > if (is_mbm_enabled()) > @@ -3995,8 +4001,13 @@ int resctrl_online_domain(struct rdt_resource *r, struct rdt_domain *d) > if (is_llc_occupancy_enabled()) > INIT_DELAYED_WORK(&d->cqm_limbo, cqm_handle_limbo); > > - /* If resctrl is mounted, add per domain monitor data directories. */ > - if (static_branch_unlikely(&rdt_mon_enable_key)) > + /* > + * If the filesystem is not mounted then only the default resource group > + * exists. Creation of its directories is deferred until mount time > + * by rdt_get_tree() calling mkdir_mondata_all(). > + * If resctrl is mounted, add per domain monitor data directories. > + */ > + if (resctrl_mounted && static_branch_unlikely(&rdt_mon_enable_key)) > mkdir_mondata_subdir_allrdtgrp(r, d); > > return 0;
-- Thanks Babu Moger
| |