When the mba_MBps mount option is used Linux initializes the
MBA control values in the schemata file to MBA_MAX_MBPS (which
is defined as U32_MAX). So the schemata file contains this line:

 MB:0=4294967295;1=4294967295

If a user edits the schemata file with vi(1) (or other editor) and
simply writes that line back, it gets changed to:

  MB:0=   4;1=   4

which sets maximum bandwidth to a very low value.

This happens because bw_validate() unconditionally rounds user supplied
values up to next multiple of r->membw.bw_gran. That results in a value
that will not fit into d->mbps_val[closid].

Rounding up only makes sense when mba_MBps is not enabled and control
values are expressed as percentage values.

Skip the roundup() when mba_MBps is enabled.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 50fa1fe9a073..ac636366c0cb 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -55,7 +55,7 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
 		return false;
 	}
 
-	*data = roundup(bw, (unsigned long)r->membw.bw_gran);
+	*data = is_mba_sc(r) ? bw : roundup(bw, (unsigned long)r->membw.bw_gran);
 	return true;
 }
 
-- 
2.46.1

+Martin

Hi Tony,

On 9/27/24 3:30 PM, Tony Luck wrote:
> When the mba_MBps mount option is used Linux initializes the
> MBA control values in the schemata file to MBA_MAX_MBPS (which
> is defined as U32_MAX). So the schemata file contains this line:
> 
>  MB:0=4294967295;1=4294967295
> 
> If a user edits the schemata file with vi(1) (or other editor) and
> simply writes that line back, it gets changed to:
> 
>   MB:0=   4;1=   4
> 
> which sets maximum bandwidth to a very low value.
> 
> This happens because bw_validate() unconditionally rounds user supplied
> values up to next multiple of r->membw.bw_gran. That results in a value
> that will not fit into d->mbps_val[closid].
> 
> Rounding up only makes sense when mba_MBps is not enabled and control
> values are expressed as percentage values.
> 
> Skip the roundup() when mba_MBps is enabled.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---

A fix for this issue is already being discussed. Please see latest fix
at [1] that additionally addresses the issue if a user attempts to 
write a value larger than 4294967295. Could you please check if that
fix works for you?

Thank you.

Reinette

[1] https://lore.kernel.org/all/a7c80676-0761-4618-ac07-0b53434b1a9b@intel.com/

> A fix for this issue is already being discussed. Please see latest fix
> at [1] that additionally addresses the issue if a user attempts to 
> write a value larger than 4294967295. Could you please check if that
> fix works for you?

That one looks more complete. My patch only covered the case for overflow
during roundup. That one uses kstrtou32() instead of kstrtoul() so will also
catch the case where a user enters a value bigger that U32_MAX.

-Tony