| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <IA1PR11MB609709D870E9B379840592FD9BC69@IA1PR11MB6097.namprd11.prod.outlook.com>
Date: Tue, 17 Jan 2023 18:53:12 +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 05/18] x86/resctrl: Allow RMID allocation to be scoped
by CLOSID
Hi, James,
> MPAMs RMID values are not unique unless the CLOSID is considered as well.
>
> alloc_rmid() expects the RMID to be an independent number.
>
> Pass the CLOSID in to alloc_rmid(). Use this to compare indexes when allocating.
> If the CLOSID is not relevant to the index, this ends up comparing the free RMID
> with itself, and the first free entry will be used. With MPAM the CLOSID is
> included in the index, so this becomes a walk of the free RMID entries, until one
> that matches the supplied CLOSID is found.
>
> Tested-by: Shaopeng Tan <tan.shaopeng@...itsu.com>
> Signed-off-by: James Morse <james.morse@....com>
> ---
> arch/x86/kernel/cpu/resctrl/internal.h | 2 +-
> arch/x86/kernel/cpu/resctrl/monitor.c | 44 ++++++++++++++++++-----
> arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 2 +-
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 2 +-
> 4 files changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h
> b/arch/x86/kernel/cpu/resctrl/internal.h
> index af71401c57e2..013c8fc9fd28 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -510,7 +510,7 @@ 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); void closid_free(int closid); -int
> alloc_rmid(void);
> +int alloc_rmid(u32 closid);
> void free_rmid(u32 closid, u32 rmid);
> int rdt_get_mon_l3_config(struct rdt_resource *r); void mon_event_count(void
> *info); diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c
> b/arch/x86/kernel/cpu/resctrl/monitor.c
> index dbae380e3d1c..347be3767241 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -301,25 +301,51 @@ bool has_busy_rmid(struct rdt_resource *r, struct
> rdt_domain *d)
> return find_first_bit(d->rmid_busy_llc, idx_limit) != idx_limit; }
>
> +static struct rmid_entry *resctrl_find_free_rmid(u32 closid) {
> + struct rmid_entry *itr;
> + u32 itr_idx, cmp_idx;
> +
> + if (list_empty(&rmid_free_lru))
> + return rmid_limbo_count ? ERR_PTR(-EBUSY) : ERR_PTR(-
> ENOSPC);
> +
> + list_for_each_entry(itr, &rmid_free_lru, list) {
> + /*
> + * get the index of this free RMID, and the index it would need
> + * to be if it were used with this CLOSID.
> + * If the CLOSID is irrelevant on this architecture, these will
> + * always be the same. Otherwise they will only match if this
> + * RMID can be used with this CLOSID.
> + */
> + itr_idx = resctrl_arch_rmid_idx_encode(itr->closid, itr->rmid);
> + cmp_idx = resctrl_arch_rmid_idx_encode(closid, itr->rmid);
> +
> + if (itr_idx == cmp_idx)
> + return itr;
Finding free rmid may be called frequently depending on usage.
It would be better to have a simpler and faster arch helper that finds the itr on x86.
Something like:
struct rmid_entry *resctrl_arch_rmid_matchd(u32 ignored, u32 ignored)
{
return list_entry_first(resctrl_free_lru, itr, list);
}
Arm64 implements the complex case going through the rmid_free_lru list in the patch.
> + }
> +
> + return ERR_PTR(-ENOSPC);
> +}
> +
> /*
> - * As of now the RMIDs allocation is global.
> + * As of now the RMIDs allocation is the same in each domain.
> * However we keep track of which packages the RMIDs
> * are used to optimize the limbo list management.
> + * The closid is ignored on x86.
> */
> -int alloc_rmid(void)
> +int alloc_rmid(u32 closid)
> {
> struct rmid_entry *entry;
>
> lockdep_assert_held(&rdtgroup_mutex);
>
> - if (list_empty(&rmid_free_lru))
> - return rmid_limbo_count ? -EBUSY : -ENOSPC;
> + entry = resctrl_find_free_rmid(closid);
> + if (!IS_ERR(entry)) {
> + list_del(&entry->list);
> + return entry->rmid;
> + }
>
> - entry = list_first_entry(&rmid_free_lru,
> - struct rmid_entry, list);
> - list_del(&entry->list);
> -
> - return entry->rmid;
> + return PTR_ERR(entry);
> }
>
> static void add_rmid_to_limbo(struct rmid_entry *entry) diff --git
> a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> index c51932516965..3b724a40d3a2 100644
> --- a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> +++ b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> @@ -763,7 +763,7 @@ int rdtgroup_locksetup_exit(struct rdtgroup *rdtgrp)
> int ret;
>
> if (rdt_mon_capable) {
> - ret = alloc_rmid();
> + ret = alloc_rmid(rdtgrp->closid);
> if (ret < 0) {
> rdt_last_cmd_puts("Out of RMIDs\n");
> return ret;
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index c67083a8a5f5..ac88610a6946 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2875,7 +2875,7 @@ static int mkdir_rdt_prepare_rmid_alloc(struct
> rdtgroup *rdtgrp)
> if (!rdt_mon_capable)
> return 0;
>
> - ret = alloc_rmid();
> + ret = alloc_rmid(rdtgrp->closid);
> if (ret < 0) {
> rdt_last_cmd_puts("Out of RMIDs\n");
> return ret;
> --
> 2.30.2
Thanks.
-Fenghua
Powered by blists - more mailing lists