2023-10-18 07:56:17

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH v2 1/1] selftests/resctrl: Fix MBM test failure when MBA unavailable

Commit 20d96b25cc4c ("selftests/resctrl: Fix schemata write error
check") exposed a problem in feature detection logic in MBM selftest.
If schemata does not support MB:x=x entries, the schemata write to
initialize 100% memory bandwidth allocation in mbm_setup() will now
fail with -EINVAL due to the error handling corrected by the commit
20d96b25cc4c ("selftests/resctrl: Fix schemata write error check").
That commit just uncovers the failed write, it is not wrong itself.

If MB:x=x is not supported by schemata, it is safe to assume 100%
memory bandwidth is always set. Therefore, the previously ignored error
does not make the MBM test itself wrong.

Restore the previous behavior of MBM test by checking MB support before
attempting to write it into schemata which results in behavior
equivalent to ignoring the write error.

Fixes: 20d96b25cc4c ("selftests/resctrl: Fix schemata write error check")
Signed-off-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
---
tools/testing/selftests/resctrl/mbm_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index d3c0d30c676a..741533f2b075 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -95,7 +95,7 @@ static int mbm_setup(struct resctrl_val_param *p)
return END_OF_TESTS;

/* Set up shemata with 100% allocation on the first run. */
- if (p->num_of_runs == 0)
+ if (p->num_of_runs == 0 && validate_resctrl_feature_request("MB", NULL))
ret = write_schemata(p->ctrlgrp, "100", p->cpu_no,
p->resctrl_val);

--
2.30.2


2023-10-18 08:10:35

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] selftests/resctrl: Fix MBM test failure when MBA unavailable

On Wed, 18 Oct 2023, Ilpo J?rvinen wrote:

> Commit 20d96b25cc4c ("selftests/resctrl: Fix schemata write error
> check") exposed a problem in feature detection logic in MBM selftest.
> If schemata does not support MB:x=x entries, the schemata write to
> initialize 100% memory bandwidth allocation in mbm_setup() will now
> fail with -EINVAL due to the error handling corrected by the commit
> 20d96b25cc4c ("selftests/resctrl: Fix schemata write error check").
> That commit just uncovers the failed write, it is not wrong itself.
>
> If MB:x=x is not supported by schemata, it is safe to assume 100%
> memory bandwidth is always set. Therefore, the previously ignored error
> does not make the MBM test itself wrong.
>
> Restore the previous behavior of MBM test by checking MB support before
> attempting to write it into schemata which results in behavior
> equivalent to ignoring the write error.
>
> Fixes: 20d96b25cc4c ("selftests/resctrl: Fix schemata write error check")
> Signed-off-by: Ilpo J?rvinen <[email protected]>
> Reviewed-by: Reinette Chatre <[email protected]>
> ---

It seems it's too early morning for me so I entirely forgot to describe
the changes I made... doh. So here it is (nothing earth-shattering here
really):

v2:
- Make checkpatch --strict happy.
- Remove () that --strict called "extra".
- Duplicate the referenced commit description in the changelog
to follow the commit formatting guidelines.
- Alter the wording at one of the references into the commit
20d96b25cc4c in order to avoid having to copy the description
there too.
- Added Reinette's rev-by.

--
i.


> tools/testing/selftests/resctrl/mbm_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
> index d3c0d30c676a..741533f2b075 100644
> --- a/tools/testing/selftests/resctrl/mbm_test.c
> +++ b/tools/testing/selftests/resctrl/mbm_test.c
> @@ -95,7 +95,7 @@ static int mbm_setup(struct resctrl_val_param *p)
> return END_OF_TESTS;
>
> /* Set up shemata with 100% allocation on the first run. */
> - if (p->num_of_runs == 0)
> + if (p->num_of_runs == 0 && validate_resctrl_feature_request("MB", NULL))
> ret = write_schemata(p->ctrlgrp, "100", p->cpu_no,
> p->resctrl_val);
>
>

2023-10-18 19:07:53

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] selftests/resctrl: Fix MBM test failure when MBA unavailable

On 10/18/23 02:10, Ilpo Järvinen wrote:
> On Wed, 18 Oct 2023, Ilpo Järvinen wrote:
>
>> Commit 20d96b25cc4c ("selftests/resctrl: Fix schemata write error
>> check") exposed a problem in feature detection logic in MBM selftest.
>> If schemata does not support MB:x=x entries, the schemata write to
>> initialize 100% memory bandwidth allocation in mbm_setup() will now
>> fail with -EINVAL due to the error handling corrected by the commit
>> 20d96b25cc4c ("selftests/resctrl: Fix schemata write error check").
>> That commit just uncovers the failed write, it is not wrong itself.
>>
>> If MB:x=x is not supported by schemata, it is safe to assume 100%
>> memory bandwidth is always set. Therefore, the previously ignored error
>> does not make the MBM test itself wrong.
>>
>> Restore the previous behavior of MBM test by checking MB support before
>> attempting to write it into schemata which results in behavior
>> equivalent to ignoring the write error.
>>
>> Fixes: 20d96b25cc4c ("selftests/resctrl: Fix schemata write error check")
>> Signed-off-by: Ilpo Järvinen <[email protected]>
>> Reviewed-by: Reinette Chatre <[email protected]>
>> ---
>
> It seems it's too early morning for me so I entirely forgot to describe
> the changes I made... doh. So here it is (nothing earth-shattering here
> really):
>
> v2:
> - Make checkpatch --strict happy.
> - Remove () that --strict called "extra".
> - Duplicate the referenced commit description in the changelog
> to follow the commit formatting guidelines.
> - Alter the wording at one of the references into the commit
> 20d96b25cc4c in order to avoid having to copy the description
> there too.
> - Added Reinette's rev-by.
>


Thank you. Applied to linux-kselftest next for Linux 6.7-rc1

thanks,
-- Shuah