[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <IA1PR11MB609729461B9FF9F9BFD1DEC99BC69@IA1PR11MB6097.namprd11.prod.outlook.com>
Date: Tue, 17 Jan 2023 18:29:01 +0000
From: "Yu, Fenghua" <fenghua.yu@...el.com>
To: James Morse <james.morse@....com>,
"x86@...nel.org" <x86@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "Chatre, Reinette" <reinette.chatre@...el.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
H Peter Anvin <hpa@...or.com>,
Babu Moger <Babu.Moger@....com>,
"shameerali.kolothum.thodi@...wei.com"
<shameerali.kolothum.thodi@...wei.com>,
D Scott Phillips OS <scott@...amperecomputing.com>,
"carl@...amperecomputing.com" <carl@...amperecomputing.com>,
"lcherian@...vell.com" <lcherian@...vell.com>,
"bobo.shaobowang@...wei.com" <bobo.shaobowang@...wei.com>,
"tan.shaopeng@...itsu.com" <tan.shaopeng@...itsu.com>,
"xingxin.hx@...nanolis.org" <xingxin.hx@...nanolis.org>,
"baolin.wang@...ux.alibaba.com" <baolin.wang@...ux.alibaba.com>,
Jamie Iles <quic_jiles@...cinc.com>,
Xin Hao <xhao@...ux.alibaba.com>,
"peternewman@...gle.com" <peternewman@...gle.com>
Subject: RE: [PATCH v2 06/18] x86/resctrl: Allow the allocator to check if a
CLOSID can allocate clean RMID
Hi, James,
> MPAM's PMG bits extend its PARTID space, meaning the same PMG value can be
> used for different control groups.
>
> This means once a CLOSID is allocated, all its monitoring ids may still be dirty,
> and held in limbo.
>
> Add a helper to allow the CLOSID allocator to check if a CLOSID has dirty RMID
> values. This behaviour is enabled by a kconfig option selected by the
> architecture, which avoids a pointless search for x86.
>
> Tested-by: Shaopeng Tan <tan.shaopeng@...itsu.com>
> Signed-off-by: James Morse <james.morse@....com>
>
> ---
> Changes since v1:
> * Removed superflous IS_ENABLED().
> ---
> arch/x86/kernel/cpu/resctrl/internal.h | 1 +
> arch/x86/kernel/cpu/resctrl/monitor.c | 31 ++++++++++++++++++++++++++
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 17 ++++++++------
> 3 files changed, 42 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h
> b/arch/x86/kernel/cpu/resctrl/internal.h
> index 013c8fc9fd28..adae6231324f 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -509,6 +509,7 @@ int rdtgroup_pseudo_lock_create(struct rdtgroup
> *rdtgrp); void rdtgroup_pseudo_lock_remove(struct rdtgroup *rdtgrp); struct
> rdt_domain *get_domain_from_cpu(int cpu, struct rdt_resource *r); int
> closids_supported(void);
> +bool resctrl_closid_is_dirty(u32 closid);
> void closid_free(int closid);
> int alloc_rmid(u32 closid);
> void free_rmid(u32 closid, u32 rmid);
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c
> b/arch/x86/kernel/cpu/resctrl/monitor.c
> index 347be3767241..190ac183139e 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -327,6 +327,37 @@ static struct rmid_entry *resctrl_find_free_rmid(u32
> closid)
> return ERR_PTR(-ENOSPC);
> }
>
> +/**
> + * resctrl_closid_is_dirty - Determine if clean RMID can be allocate for this
s/allocate/allocated/
> + * CLOSID.
> + * @closid: The CLOSID that is being queried.
> + *
> + * MPAM's equivalent of RMID are per-CLOSID, meaning a freshly allocate
s/allocate/allocated/
> +CLOSID
> + * may not be able to allocate clean RMID. To avoid this the allocator
> +will
> + * only return clean CLOSID.
> + */
> +bool resctrl_closid_is_dirty(u32 closid) {
> + struct rmid_entry *entry;
> + int i;
> +
> + lockdep_assert_held(&rdtgroup_mutex);
It's better to move lockdep_asser_held() after if (!IS_ENABLE()).
Then compiler might optimize this function to empty on X86.
> +
> + if (!IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
> + return false;
> +
> + for (i = 0; i < resctrl_arch_system_num_rmid_idx(); i++) {
> + entry = &rmid_ptrs[i];
> + if (entry->closid != closid)
> + continue;
> +
> + if (entry->busy)
> + return true;
> + }
> +
> + return false;
> +}
> +
> /*
> * As of now the RMIDs allocation is the same in each domain.
> * However we keep track of which packages the RMIDs diff --git
> a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index ac88610a6946..e1f879e13823 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -93,7 +93,7 @@ void rdt_last_cmd_printf(const char *fmt, ...)
> * - Our choices on how to configure each resource become progressively more
> * limited as the number of resources grows.
> */
> -static int closid_free_map;
> +static unsigned long closid_free_map;
> static int closid_free_map_len;
>
> int closids_supported(void)
> @@ -119,14 +119,17 @@ static void closid_init(void)
>
> static int closid_alloc(void)
> {
> - u32 closid = ffs(closid_free_map);
> + u32 closid;
>
> - if (closid == 0)
> - return -ENOSPC;
> - closid--;
> - closid_free_map &= ~(1 << closid);
> + for_each_set_bit(closid, &closid_free_map, closid_free_map_len) {
> + if (resctrl_closid_is_dirty(closid))
> + continue;
>
> - return closid;
> + clear_bit(closid, &closid_free_map);
> + return closid;
> + }
> +
> + return -ENOSPC;
> }
>
> void closid_free(int closid)
> --
> 2.30.2
Thanks.
-Fenghua
Powered by blists - more mailing lists