Messages in this thread | | | Date | Tue, 23 Aug 2022 09:42:37 -0700 | Subject | Re: [PATCH v5 07/21] x86/resctrl: Abstract and use supports_mba_mbps() | From | Reinette Chatre <> |
| |
Hi James,
On 6/22/2022 9:46 AM, James Morse wrote:
...
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index b32ceff8325a..49d9e2c3c743 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > @@ -1890,17 +1890,26 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r) > } > > /* > - * Enable or disable the MBA software controller > - * which helps user specify bandwidth in MBps. > * MBA software controller is supported only if > * MBM is supported and MBA is in linear scale. > */ > +static bool supports_mba_mbps(void) > +{ > + struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl; > + > + return (is_mbm_enabled() && > + r->alloc_capable && is_mba_linear()); > +} > + > +/* > + * Enable or disable the MBA software controller > + * which helps user specify bandwidth in MBps. > + */ > static int set_mba_sc(bool mba_sc) > { > struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl; > > - if (!is_mbm_enabled() || !is_mba_linear() || > - mba_sc == is_mba_sc(r)) > + if (!supports_mba_mbps() || mba_sc == is_mba_sc(r)) > return -EINVAL; > > r->membw.mba_sc = mba_sc;
It seems that this change is propagating an issue that exists in the current implementation (not introduced by this work).
The supports_mba_mbps() test uses is_mbm_enabled() which is: (is_mbm_total_enabled() || is_mbm_local_enabled())
That is not accurate since the software controller is only called when MBM local is enabled: mbm_update() { ... if (is_mbm_local_enabled()) { ... if (is_mba_sc(NULL)) mbm_bw_count(rmid, &rr); } }
Could the "is_mbm_enabled()" in the new supports_mba_mbps() introduced here be changed to use "is_mbm_local_enabled()" instead? It does not seem an issue for existing hardware (that supports total and local) but the kernel parameters do enable users to boot with "rdt=!mbmlocal" to trigger problematic scenarios.
Reinette
| |