| 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: <c735cf97-dc26-42be-82c9-cb37f96e896b@intel.com>
Date: Wed, 19 Feb 2025 20:08:41 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: James Morse <james.morse@....com>, <x86@...nel.org>,
<linux-kernel@...r.kernel.org>
CC: 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>, "D Scott
Phillips OS" <scott@...amperecomputing.com>, <carl@...amperecomputing.com>,
<lcherian@...vell.com>, <bobo.shaobowang@...wei.com>,
<tan.shaopeng@...itsu.com>, <baolin.wang@...ux.alibaba.com>, Jamie Iles
<quic_jiles@...cinc.com>, Xin Hao <xhao@...ux.alibaba.com>,
<peternewman@...gle.com>, <dfustini@...libre.com>, <amitsinght@...vell.com>,
David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
"Dave Martin" <dave.martin@....com>, Koba Ko <kobak@...dia.com>, Shanker
Donthineni <sdonthineni@...dia.com>, Shaopeng Tan
<tan.shaopeng@...fujitsu.com>, "Tony Luck" <tony.luck@...el.com>
Subject: Re: [PATCH v6 30/42] x86/resctrl: Claim
get_{mon,ctrl}_domain_from_cpu() helpers for resctrl
Hi James,
On 2/7/25 10:18 AM, James Morse wrote:
> get_{mon,ctrl}_domain_from_cpu() are handy helpers that both the arch
> code and resctrl need to use. Rename them to have a resctrl_ prefix
> and move them to a header file.
>
> Signed-off-by: James Morse <james.morse@....com>
> Tested-by: Carl Worth <carl@...amperecomputing.com> # arm64
> Tested-by: Shaopeng Tan <tan.shaopeng@...fujitsu.com>
> Reviewed-by: Shaopeng Tan <tan.shaopeng@...fujitsu.com>
> Reviewed-by: Tony Luck <tony.luck@...el.com>
> ---
> Changes from v5:
> * Added the word from to a comment.
> ---
> arch/x86/kernel/cpu/resctrl/core.c | 30 ---------------------
> arch/x86/kernel/cpu/resctrl/internal.h | 2 --
> arch/x86/kernel/cpu/resctrl/monitor.c | 2 +-
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 2 +-
> include/linux/resctrl.h | 37 ++++++++++++++++++++++++++
> 5 files changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index e4b676879227..921c351d57ae 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -348,36 +348,6 @@ static void cat_wrmsr(struct msr_param *m)
> wrmsrl(hw_res->msr_base + i, hw_dom->ctrl_val[i]);
> }
>
> -struct rdt_ctrl_domain *get_ctrl_domain_from_cpu(int cpu, struct rdt_resource *r)
> -{
> - struct rdt_ctrl_domain *d;
> -
> - lockdep_assert_cpus_held();
> -
> - list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
> - /* Find the domain that contains this CPU */
> - if (cpumask_test_cpu(cpu, &d->hdr.cpu_mask))
> - return d;
> - }
> -
> - return NULL;
> -}
> -
> -struct rdt_mon_domain *get_mon_domain_from_cpu(int cpu, struct rdt_resource *r)
> -{
> - struct rdt_mon_domain *d;
> -
> - lockdep_assert_cpus_held();
> -
> - list_for_each_entry(d, &r->mon_domains, hdr.list) {
> - /* Find the domain that contains this CPU */
> - if (cpumask_test_cpu(cpu, &d->hdr.cpu_mask))
> - return d;
> - }
> -
> - return NULL;
> -}
> -
> u32 resctrl_arch_get_num_closid(struct rdt_resource *r)
> {
> return resctrl_to_arch_res(r)->num_closid;
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 0d13006e920b..c44c5b496355 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -475,8 +475,6 @@ unsigned int rdtgroup_cbm_to_size(struct rdt_resource *r, struct rdt_ctrl_domain
> unsigned long cbm);
> enum rdtgrp_mode rdtgroup_mode_by_closid(int closid);
> int rdtgroup_tasks_assigned(struct rdtgroup *r);
> -struct rdt_ctrl_domain *get_ctrl_domain_from_cpu(int cpu, struct rdt_resource *r);
> -struct rdt_mon_domain *get_mon_domain_from_cpu(int cpu, struct rdt_resource *r);
> int closids_supported(void);
> void closid_free(int closid);
> int alloc_rmid(u32 closid);
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index d99a05fc1b44..470cf16f506e 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -773,7 +773,7 @@ static void update_mba_bw(struct rdtgroup *rgrp, struct rdt_mon_domain *dom_mbm)
> if (WARN_ON_ONCE(!pmbm_data))
> return;
>
> - dom_mba = get_ctrl_domain_from_cpu(smp_processor_id(), r_mba);
> + dom_mba = resctrl_get_ctrl_domain_from_cpu(smp_processor_id(), r_mba);
> if (!dom_mba) {
> pr_warn_once("Failure to get domain for MBA update\n");
> return;
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 5fc60c9ce28f..08fec23a38bf 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -4274,7 +4274,7 @@ void resctrl_offline_cpu(unsigned int cpu)
> if (!l3->mon_capable)
> goto out_unlock;
>
> - d = get_mon_domain_from_cpu(cpu, l3);
> + d = resctrl_get_mon_domain_from_cpu(cpu, l3);
> if (d) {
> if (resctrl_is_mbm_enabled() && cpu == d->mbm_work_cpu) {
> cancel_delayed_work(&d->mbm_over);
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 29415d023aab..511dab4ffcdc 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -3,6 +3,7 @@
> #define _RESCTRL_H
>
> #include <linux/cacheinfo.h>
> +#include <linux/cpu.h>
> #include <linux/kernel.h>
> #include <linux/list.h>
> #include <linux/pid.h>
> @@ -399,6 +400,42 @@ static inline u32 resctrl_get_config_index(u32 closid,
> }
> }
>
> +/*
> + * Caller must hold the cpuhp read lock to prevent the struct rdt_domain from
struct rdt_domain has since been split into struct rdt_ctrl_domain and struct rdt_mon_domain.
I assume this comment covers both helpers so perhaps this can be "to prevent the domain
from ..."?
> + * being freed.
> + */
> +static inline struct rdt_ctrl_domain *
> +resctrl_get_ctrl_domain_from_cpu(int cpu, struct rdt_resource *r)
> +{
> + struct rdt_ctrl_domain *d;
> +
> + lockdep_assert_cpus_held();
> +
> + list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
> + /* Find the domain that contains this CPU */
> + if (cpumask_test_cpu(cpu, &d->hdr.cpu_mask))
> + return d;
> + }
> +
> + return NULL;
> +}
> +
> +static inline struct rdt_mon_domain *
> +resctrl_get_mon_domain_from_cpu(int cpu, struct rdt_resource *r)
> +{
> + struct rdt_mon_domain *d;
> +
> + lockdep_assert_cpus_held();
> +
> + list_for_each_entry(d, &r->mon_domains, hdr.list) {
> + /* Find the domain that contains this CPU */
> + if (cpumask_test_cpu(cpu, &d->hdr.cpu_mask))
> + return d;
> + }
> +
> + return NULL;
> +}
> +
Similar to previous requests, could you please provide a motivation for
the switch to inline?
> /*
> * Update the ctrl_val and apply this config right now.
> * Must be called on one of the domain's CPUs.
Reinette
Powered by blists - more mailing lists