[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2504daf3-16fa-9474-e9c5-cd6aa99d4776@amd.com>
Date: Tue, 24 Oct 2023 18:20:17 -0500
From: "Moger, Babu" <bmoger@....com>
To: Tony Luck <tony.luck@...el.com>, Fenghua Yu <fenghua.yu@...el.com>,
Reinette Chatre <reinette.chatre@...el.com>,
Peter Newman <peternewman@...gle.com>,
Jonathan Corbet <corbet@....net>,
Shuah Khan <skhan@...uxfoundation.org>,
"x86@...nel.org" <x86@...nel.org>
Cc: Shaopeng Tan <tan.shaopeng@...itsu.com>,
James Morse <james.morse@....com>,
Jamie Iles <quic_jiles@...cinc.com>,
Randy Dunlap <rdunlap@...radead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"patches@...ts.linux.dev" <patches@...ts.linux.dev>
Subject: Re: [PATCH] x86/resctrl: mba_MBps: Fall back to total b/w if local
b/w unavailable
Hi Tony,
> -----Original Message-----
> From: Tony Luck <tony.luck@...el.com>
> Sent: Tuesday, October 24, 2023 1:16 PM
> To: Fenghua Yu <fenghua.yu@...el.com>; Reinette Chatre
> <reinette.chatre@...el.com>; Peter Newman <peternewman@...gle.com>;
> Jonathan Corbet <corbet@....net>; Shuah Khan <skhan@...uxfoundation.org>;
> x86@...nel.org
> Cc: Shaopeng Tan <tan.shaopeng@...itsu.com>; James Morse
> <james.morse@....com>; Jamie Iles <quic_jiles@...cinc.com>; Moger, Babu
> <Babu.Moger@....com>; Randy Dunlap <rdunlap@...radead.org>; linux-
> kernel@...r.kernel.org; linux-doc@...r.kernel.org; patches@...ts.linux.dev;
> Tony Luck <tony.luck@...el.com>
> Subject: [PATCH] x86/resctrl: mba_MBps: Fall back to total b/w if local b/w
> unavailable
>
> On Intel the various resource director technology (RDT) features are all
> orthogonal and independently enumerated. Thus it is possible to have a system
> that provides "total" memory bandwidth measurements without providing
> "local" bandwidth measurements.
>
> If local bandwidth measurement is not available, do not give up on providing
> the "mba_MBps" feedback option completely, make the code fall back to using
> total bandwidth.
Is this customer requirement ?
What do you mean by " If local bandwidth measurement is not available" ?
Is the hardware supports only total bandwidth and not local?
It can get real ugly if we try to handle one special case.
thanks
Babu
>
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
> arch/x86/kernel/cpu/resctrl/monitor.c | 34 ++++++++++++++++----------
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 2 +-
> 2 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c
> b/arch/x86/kernel/cpu/resctrl/monitor.c
> index f136ac046851..3b9531cce807 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -418,6 +418,14 @@ static int __mon_event_count(u32 rmid, struct
> rmid_read *rr)
> return 0;
> }
>
> +static struct mbm_state *get_mbm_data(struct rdt_domain *dom_mbm, int
> +rmid) {
> + if (is_mbm_local_enabled())
> + return &dom_mbm->mbm_local[rmid];
> +
> + return &dom_mbm->mbm_total[rmid];
> +}
> +
> /*
> * mbm_bw_count() - Update bw count from values previously read by
> * __mon_event_count().
> @@ -431,7 +439,7 @@ static int __mon_event_count(u32 rmid, struct
> rmid_read *rr)
> */
> static void mbm_bw_count(u32 rmid, struct rmid_read *rr) {
> - struct mbm_state *m = &rr->d->mbm_local[rmid];
> + struct mbm_state *m = get_mbm_data(rr->d, rmid);
> u64 cur_bw, bytes, cur_bytes;
>
> cur_bytes = rr->val;
> @@ -526,14 +534,14 @@ static void update_mba_bw(struct rdtgroup *rgrp,
> struct rdt_domain *dom_mbm)
> struct list_head *head;
> struct rdtgroup *entry;
>
> - if (!is_mbm_local_enabled())
> + if (!is_mbm_enabled())
> return;
>
> r_mba = &rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl;
>
> closid = rgrp->closid;
> rmid = rgrp->mon.rmid;
> - pmbm_data = &dom_mbm->mbm_local[rmid];
> + pmbm_data = get_mbm_data(dom_mbm, rmid);
>
> dom_mba = get_domain_from_cpu(smp_processor_id(), r_mba);
> if (!dom_mba) {
> @@ -553,7 +561,7 @@ static void update_mba_bw(struct rdtgroup *rgrp,
> struct rdt_domain *dom_mbm)
> */
> head = &rgrp->mon.crdtgrp_list;
> list_for_each_entry(entry, head, mon.crdtgrp_list) {
> - cmbm_data = &dom_mbm->mbm_local[entry->mon.rmid];
> + cmbm_data = get_mbm_data(dom_mbm, entry->mon.rmid);
> cur_bw += cmbm_data->prev_bw;
> delta_bw += cmbm_data->delta_bw;
> }
> @@ -595,7 +603,7 @@ static void update_mba_bw(struct rdtgroup *rgrp,
> struct rdt_domain *dom_mbm)
> */
> pmbm_data->delta_comp = true;
> list_for_each_entry(entry, head, mon.crdtgrp_list) {
> - cmbm_data = &dom_mbm->mbm_local[entry->mon.rmid];
> + cmbm_data = get_mbm_data(dom_mbm, entry->mon.rmid);
> cmbm_data->delta_comp = true;
> }
> }
> @@ -621,15 +629,15 @@ static void mbm_update(struct rdt_resource *r, struct
> rdt_domain *d, int rmid)
> rr.evtid = QOS_L3_MBM_LOCAL_EVENT_ID;
> rr.val = 0;
> __mon_event_count(rmid, &rr);
> -
> - /*
> - * Call the MBA software controller only for the
> - * control groups and when user has enabled
> - * the software controller explicitly.
> - */
> - if (is_mba_sc(NULL))
> - mbm_bw_count(rmid, &rr);
> }
> +
> + /*
> + * Call the MBA software controller only for the
> + * control groups and when user has enabled
> + * the software controller explicitly.
> + */
> + if (is_mba_sc(NULL))
> + mbm_bw_count(rmid, &rr);
> }
>
> /*
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 69a1de92384a..0c4f8a1b8df0 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2294,7 +2294,7 @@ static bool supports_mba_mbps(void) {
> struct rdt_resource *r =
> &rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl;
>
> - return (is_mbm_local_enabled() &&
> + return (is_mbm_enabled() &&
> r->alloc_capable && is_mba_linear()); }
>
> --
> 2.41.0
Powered by blists - more mailing lists