lkml.org 
[lkml]   [2023]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5 19/24] x86/resctrl: Add helpers for system wide mon/alloc capable
From
Hi, James,

On 7/28/23 09:42, James Morse wrote:
> resctrl reads rdt_alloc_capable or rdt_mon_capable to determine
> whether any of the resources support the corresponding features.
> resctrl also uses the static-keys that affect the architecture's
> context-switch code to determine the same thing.
>
> This forces another architecture to have the same static-keys.
>
> As the static-key is enabled based on the capable flag, and none of
> the filesystem uses of these are in the scheduler path, move the
> capable flags behind helpers, and use these in the filesystem
> code instead of the static-key.
>
> After this change, only the architecture code manages and uses
> the static-keys to ensure __resctrl_sched_in() does not need
> runtime checks.
>
> This avoids multiple architectures having to define the same
> static-keys.
>
> Cases where the static-key implicitly tested if the resctrl
> filesystem was mounted all have an explicit check added by a
> previous patch.
>
> Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com>
> Reviewed-by: Shaopeng Tan <tan.shaopeng@fujitsu.com>
> Signed-off-by: James Morse <james.morse@arm.com>
>
> ---
> Changes since v1:
> * Added missing conversion in mkdir_rdt_prepare_rmid_free()
>
> Changes since v3:
> * Expanded the commit message.
> ---
> arch/x86/include/asm/resctrl.h | 13 +++++++++
> arch/x86/kernel/cpu/resctrl/internal.h | 2 --
> arch/x86/kernel/cpu/resctrl/monitor.c | 4 +--
> arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 6 ++--
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 34 +++++++++++------------
> 5 files changed, 35 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> index 3876d4bb4bed..63a4a2332d61 100644
> --- a/arch/x86/include/asm/resctrl.h
> +++ b/arch/x86/include/asm/resctrl.h
> @@ -38,10 +38,18 @@ struct resctrl_pqr_state {
>
> DECLARE_PER_CPU(struct resctrl_pqr_state, pqr_state);
>
> +extern bool rdt_alloc_capable;
> +extern bool rdt_mon_capable;
> +
> DECLARE_STATIC_KEY_FALSE(rdt_enable_key);
> DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
> DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
>
> +static inline bool resctrl_arch_alloc_capable(void)
> +{
> + return rdt_alloc_capable;
> +}
> +
> static inline void resctrl_arch_enable_alloc(void)
> {
> static_branch_enable_cpuslocked(&rdt_alloc_enable_key);
> @@ -54,6 +62,11 @@ static inline void resctrl_arch_disable_alloc(void)
> static_branch_dec_cpuslocked(&rdt_enable_key);
> }
>
> +static inline bool resctrl_arch_mon_capable(void)
> +{
> + return rdt_mon_capable;
> +}
> +
> static inline void resctrl_arch_enable_mon(void)
> {
> static_branch_enable_cpuslocked(&rdt_mon_enable_key);
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index ac39fecba4ca..f99e0a1f39c8 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -136,8 +136,6 @@ struct rmid_read {
> void *arch_mon_ctx;
> };
>
> -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;
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index 5350d44b16b6..c0b1ad8d8f6d 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -842,7 +842,7 @@ void mbm_handle_overflow(struct work_struct *work)
> * If the filesystem has been unmounted this work no longer needs to
> * run.
> */
> - if (!resctrl_mounted || !static_branch_likely(&rdt_mon_enable_key))
> + if (!resctrl_mounted || !resctrl_arch_mon_capable())
> goto out_unlock;
>
> r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> @@ -879,7 +879,7 @@ void mbm_setup_overflow_handler(struct rdt_domain *dom, unsigned long delay_ms)
> * 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))
> + if (!resctrl_mounted || !resctrl_arch_mon_capable())
> return;
> cpu = cpumask_any_housekeeping(&dom->cpu_mask);
> dom->mbm_work_cpu = cpu;
> diff --git a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> index 5ebd6e54c7f2..460421051abf 100644
> --- a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> +++ b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> @@ -567,7 +567,7 @@ static int rdtgroup_locksetup_user_restrict(struct rdtgroup *rdtgrp)
> if (ret)
> goto err_cpus;
>
> - if (rdt_mon_capable) {
> + if (resctrl_arch_mon_capable()) {
> ret = rdtgroup_kn_mode_restrict(rdtgrp, "mon_groups");
> if (ret)
> goto err_cpus_list;
> @@ -614,7 +614,7 @@ static int rdtgroup_locksetup_user_restore(struct rdtgroup *rdtgrp)
> if (ret)
> goto err_cpus;
>
> - if (rdt_mon_capable) {
> + if (resctrl_arch_mon_capable()) {
> ret = rdtgroup_kn_mode_restore(rdtgrp, "mon_groups", 0777);
> if (ret)
> goto err_cpus_list;
> @@ -762,7 +762,7 @@ int rdtgroup_locksetup_exit(struct rdtgroup *rdtgrp)
> {
> int ret;
>
> - if (rdt_mon_capable) {
> + if (resctrl_arch_mon_capable()) {
> ret = alloc_rmid(rdtgrp->closid);
> if (ret < 0) {
> rdt_last_cmd_puts("Out of RMIDs\n");
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index ce1ed485e4f7..fef78a3dc632 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -630,13 +630,13 @@ static int __rdtgroup_move_task(struct task_struct *tsk,
>
> static bool is_closid_match(struct task_struct *t, struct rdtgroup *r)
> {
> - return (rdt_alloc_capable && (r->type == RDTCTRL_GROUP) &&
> + return (resctrl_arch_alloc_capable() && (r->type == RDTCTRL_GROUP) &&
> resctrl_arch_match_closid(t, r->closid));
> }
>
> static bool is_rmid_match(struct task_struct *t, struct rdtgroup *r)
> {
> - return (rdt_mon_capable && (r->type == RDTMON_GROUP) &&
> + return (resctrl_arch_mon_capable() && (r->type == RDTMON_GROUP) &&
> resctrl_arch_match_rmid(t, r->mon.parent->closid,
> r->mon.rmid));
> }
> @@ -2519,7 +2519,7 @@ static int rdt_get_tree(struct fs_context *fc)
> if (ret < 0)
> goto out_schemata_free;
>
> - if (rdt_mon_capable) {
> + if (resctrl_arch_mon_capable()) {
> ret = mongroup_create_dir(rdtgroup_default.kn,
> &rdtgroup_default, "mon_groups",
> &kn_mongrp);
> @@ -2541,12 +2541,12 @@ static int rdt_get_tree(struct fs_context *fc)
> if (ret < 0)
> goto out_psl;
>
> - if (rdt_alloc_capable)
> + if (resctrl_arch_alloc_capable())
> resctrl_arch_enable_alloc();
> - if (rdt_mon_capable)
> + if (resctrl_arch_mon_capable())
> resctrl_arch_enable_mon();
>
> - if (rdt_alloc_capable || rdt_mon_capable)
> + if (resctrl_arch_alloc_capable() || resctrl_arch_mon_capable())
> resctrl_mounted = true;
>
> if (is_mbm_enabled()) {
> @@ -2560,10 +2560,10 @@ static int rdt_get_tree(struct fs_context *fc)
> out_psl:
> rdt_pseudo_lock_release();
> out_mondata:
> - if (rdt_mon_capable)
> + if (resctrl_arch_mon_capable())
> kernfs_remove(kn_mondata);
> out_mongrp:
> - if (rdt_mon_capable)
> + if (resctrl_arch_mon_capable())
> kernfs_remove(kn_mongrp);
> out_info:
> kernfs_remove(kn_info);
> @@ -2815,9 +2815,9 @@ static void rdt_kill_sb(struct super_block *sb)
> rdt_pseudo_lock_release();
> rdtgroup_default.mode = RDT_MODE_SHAREABLE;
> schemata_list_destroy();
> - if (rdt_alloc_capable)
> + if (resctrl_arch_alloc_capable())
> resctrl_arch_disable_alloc();
> - if (rdt_mon_capable)
> + if (resctrl_arch_mon_capable())
> resctrl_arch_disable_mon();
> resctrl_mounted = false;
> kernfs_kill_sb(sb);
> @@ -3197,7 +3197,7 @@ static int mkdir_rdt_prepare_rmid_alloc(struct rdtgroup *rdtgrp)
> {
> int ret;
>
> - if (!rdt_mon_capable)
> + if (!resctrl_arch_mon_capable())
> return 0;
>
> ret = alloc_rmid(rdtgrp->closid);
> @@ -3219,7 +3219,7 @@ static int mkdir_rdt_prepare_rmid_alloc(struct rdtgroup *rdtgrp)
>
> static void mkdir_rdt_prepare_rmid_free(struct rdtgroup *rgrp)
> {
> - if (rdt_mon_capable)
> + if (resctrl_arch_mon_capable())
> free_rmid(rgrp->closid, rgrp->mon.rmid);
> }
>
> @@ -3385,7 +3385,7 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn,
>
> list_add(&rdtgrp->rdtgroup_list, &rdt_all_groups);
>
> - if (rdt_mon_capable) {
> + if (resctrl_arch_mon_capable()) {
> /*
> * Create an empty mon_groups directory to hold the subset
> * of tasks and cpus to monitor.
> @@ -3440,14 +3440,14 @@ static int rdtgroup_mkdir(struct kernfs_node *parent_kn, const char *name,
> * allocation is supported, add a control and monitoring
> * subdirectory
> */
> - if (rdt_alloc_capable && parent_kn == rdtgroup_default.kn)
> + if (resctrl_arch_alloc_capable() && parent_kn == rdtgroup_default.kn)
> return rdtgroup_mkdir_ctrl_mon(parent_kn, name, mode);
>
> /*
> * If RDT monitoring is supported and the parent directory is a valid
> * "mon_groups" directory, add a monitoring subdirectory.
> */
> - if (rdt_mon_capable && is_mon_groups(parent_kn, name))
> + if (resctrl_arch_mon_capable() && is_mon_groups(parent_kn, name))
> return rdtgroup_mkdir_mon(parent_kn, name, mode);
>
> return -EPERM;
> @@ -3779,7 +3779,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 (resctrl_mounted && static_branch_unlikely(&rdt_mon_enable_key))
> + if (resctrl_mounted && resctrl_arch_mon_capable())
> rmdir_mondata_subdir_allrdtgrp(r, d->id);
>
> if (is_mbm_enabled())
> @@ -3862,7 +3862,7 @@ int resctrl_online_domain(struct rdt_resource *r, struct rdt_domain *d)
> * 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))
> + if (resctrl_mounted && resctrl_arch_mon_capable())
> mkdir_mondata_subdir_allrdtgrp(r, d);
>
> return 0;

Why isn't rdt_alloc_capable in get_rdt_alloc_resources() replaced by the
helper?

static __init bool get_rdt_alloc_resources(void)
{
...
if (rdt_alloc_capable)
...

Thanks.

-Fenghua

\
 
 \ /
  Last update: 2023-08-17 20:36    [W:0.302 / U:0.324 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site