Changes since V3:
- V3: https://lore.kernel.org/all/cover.1729218182.git.reinette.chatre@intel.com/
- Rebased on HEAD 2a027d6bb660 of kselftest/next.
- Fix empty string parsing issues pointed out by Ilpo.
- Add Reviewed-by tags.
- Please see individual patches for detailed changes.

Changes since V2:
- V2: https://lore.kernel.org/all/cover.1726164080.git.reinette.chatre@intel.com/
- Add fix to protect against buffer overflow when parsing text from sysfs files.
- Add cleanup patch to address use of magic constants as pointed out by
  Ilpo.
- Add Reviewed-by tags where received, except for "selftests/resctrl: Use cache
  size to determine "fill_buf" buffer size" that changed too much since
  receiving the Reviewed-by tag.
- Please see individual patches for detailed changes.

Changes since V1:
- V1: https://lore.kernel.org/cover.1724970211.git.reinette.chatre@intel.com/
- V2 contains the same general solutions to stated problem as V1 but these
  are now preceded by more fixes (patches 1 to 5) and improved robustness
  (patches 6 to 9) to existing tests before the series gets back
  to solving the original problem with more confidence in patches 10 to 13.
- The posibility of making "memflush = false" for CMT test was discussed
  during V1. Modifying this setting does not have a significant impact on the
  observed results that are already well within acceptable range and this
  version thus keeps original default. If performance was a goal it may
  be possible to do further experimentation where "memflush = false" could
  eliminate the need for the sleep(1) within the test wrapper, but
  improving the performance is not a goal of this work.
- (New) Support what seems to be unintended ability for user space to provide
  parameters to "fill_buf" by making the parsing robust and only support
  changing parameters that are supported to be changed. Drop support for
  "write" operation since it has never been measured.
- (New) Improve wraparound handling. (Ilpo)
- (New) A couple of new fixes addressing issues discovered during development.
- (Change from V1) To support fill_buf parameters provided by user space as
  well as test specific fill_buf parameters struct fill_buf_param is no longer
  just a member of struct resctrl_val_param, instead there could be at most
  two instances of struct fill_buf_param, the immutable parameters provided
  by user space and the parameters used by individual tests. (Ilpo)
- Please see individual patches for detailed changes.

V1 cover:

The resctrl selftests for Memory Bandwidth Allocation (MBA) and Memory
Bandwidth Monitoring (MBM) are failing on some (for example [1]) Emerald
Rapids systems. The test failures result from the following two
properties of these systems:
1) Emerald Rapids systems can have up to 320MB L3 cache. The resctrl
   MBA and MBM selftests measure memory traffic for which a hardcoded
   250MB buffer has been sufficient so far. On platforms with L3 cache
   larger than the buffer, the buffer fits in the L3 cache and thus
   no/very little memory traffic is generated during the "memory
   bandwidth" tests.
2) Some platform features, for example RAS features or memory
   performance features that generate memory traffic may drive accesses
   that are counted differently by performance counters and MBM
   respectively, for instance generating "overhead" traffic which is not
   counted against any specific RMID. Until now these counting
   differences have always been "in the noise". On Emerald Rapids
   systems the maximum MBA throttling (10% memory bandwidth)
   throttles memory bandwidth to where memory accesses by these other
   platform features push the memory bandwidth difference between
   memory controller performance counters and resctrl (MBM) beyond the
   tests' hardcoded tolerance.

Make the tests more robust against platform variations:
1) Let the buffer used by memory bandwidth tests be guided by the size
   of the L3 cache.
2) Larger buffers require longer initialization time before the buffer can
   be used to measurement. Rework the tests to ensure that buffer
   initialization is complete before measurements start.
3) Do not compare performance counters and MBM measurements at low
   bandwidth. The value of "low" is hardcoded to 750MiB based on
   measurements on Emerald Rapids, Sapphire Rapids, and Ice Lake
   systems. This limit is not applicable to AMD systems since it
   only applies to the MBA and MBM tests that are isolated to Intel.

[1]
https://ark.intel.com/content/www/us/en/ark/products/237261/intel-xeon-platinum-8592-processor-320m-cache-1-9-ghz.html

Reinette Chatre (15):
  selftests/resctrl: Make functions only used in same file static
  selftests/resctrl: Print accurate buffer size as part of MBM results
  selftests/resctrl: Fix memory overflow due to unhandled wraparound
  selftests/resctrl: Protect against array overrun during iMC config
    parsing
  selftests/resctrl: Protect against array overflow when reading strings
  selftests/resctrl: Make wraparound handling obvious
  selftests/resctrl: Remove "once" parameter required to be false
  selftests/resctrl: Only support measured read operation
  selftests/resctrl: Remove unused measurement code
  selftests/resctrl: Make benchmark parameter passing robust
  selftests/resctrl: Ensure measurements skip initialization of default
    benchmark
  selftests/resctrl: Use cache size to determine "fill_buf" buffer size
  selftests/resctrl: Do not compare performance counters and resctrl at
    low bandwidth
  selftests/resctrl: Keep results from first test run
  selftests/resctrl: Replace magic constants used as array size

 tools/testing/selftests/resctrl/cmt_test.c    |  37 +-
 tools/testing/selftests/resctrl/fill_buf.c    |  45 +-
 tools/testing/selftests/resctrl/mba_test.c    |  54 ++-
 tools/testing/selftests/resctrl/mbm_test.c    |  37 +-
 tools/testing/selftests/resctrl/resctrl.h     |  79 +++-
 .../testing/selftests/resctrl/resctrl_tests.c |  95 +++-
 tools/testing/selftests/resctrl/resctrl_val.c | 447 +++++-------------
 tools/testing/selftests/resctrl/resctrlfs.c   |  19 +-
 8 files changed, 354 insertions(+), 459 deletions(-)


base-commit: 2a027d6bb66002c8e50e974676f932b33c5fce10
-- 
2.47.0

On 10/24/24 15:18, Reinette Chatre wrote:
> Changes since V3:
> - V3: https://lore.kernel.org/all/cover.1729218182.git.reinette.chatre@intel.com/
> - Rebased on HEAD 2a027d6bb660 of kselftest/next.
> - Fix empty string parsing issues pointed out by Ilpo.
> - Add Reviewed-by tags.
> - Please see individual patches for detailed changes.
> 
> Changes since V2:
> - V2: https://lore.kernel.org/all/cover.1726164080.git.reinette.chatre@intel.com/
> - Add fix to protect against buffer overflow when parsing text from sysfs files.
> - Add cleanup patch to address use of magic constants as pointed out by
>    Ilpo.
> - Add Reviewed-by tags where received, except for "selftests/resctrl: Use cache
>    size to determine "fill_buf" buffer size" that changed too much since
>    receiving the Reviewed-by tag.
> - Please see individual patches for detailed changes.
> 
> Changes since V1:
> - V1: https://lore.kernel.org/cover.1724970211.git.reinette.chatre@intel.com/
> - V2 contains the same general solutions to stated problem as V1 but these
>    are now preceded by more fixes (patches 1 to 5) and improved robustness
>    (patches 6 to 9) to existing tests before the series gets back
>    to solving the original problem with more confidence in patches 10 to 13.
> - The posibility of making "memflush = false" for CMT test was discussed
>    during V1. Modifying this setting does not have a significant impact on the
>    observed results that are already well within acceptable range and this
>    version thus keeps original default. If performance was a goal it may
>    be possible to do further experimentation where "memflush = false" could
>    eliminate the need for the sleep(1) within the test wrapper, but
>    improving the performance is not a goal of this work.
> - (New) Support what seems to be unintended ability for user space to provide
>    parameters to "fill_buf" by making the parsing robust and only support
>    changing parameters that are supported to be changed. Drop support for
>    "write" operation since it has never been measured.
> - (New) Improve wraparound handling. (Ilpo)
> - (New) A couple of new fixes addressing issues discovered during development.
> - (Change from V1) To support fill_buf parameters provided by user space as
>    well as test specific fill_buf parameters struct fill_buf_param is no longer
>    just a member of struct resctrl_val_param, instead there could be at most
>    two instances of struct fill_buf_param, the immutable parameters provided
>    by user space and the parameters used by individual tests. (Ilpo)
> - Please see individual patches for detailed changes.
> 
> V1 cover:
> 
> The resctrl selftests for Memory Bandwidth Allocation (MBA) and Memory
> Bandwidth Monitoring (MBM) are failing on some (for example [1]) Emerald
> Rapids systems. The test failures result from the following two
> properties of these systems:
> 1) Emerald Rapids systems can have up to 320MB L3 cache. The resctrl
>     MBA and MBM selftests measure memory traffic for which a hardcoded
>     250MB buffer has been sufficient so far. On platforms with L3 cache
>     larger than the buffer, the buffer fits in the L3 cache and thus
>     no/very little memory traffic is generated during the "memory
>     bandwidth" tests.
> 2) Some platform features, for example RAS features or memory
>     performance features that generate memory traffic may drive accesses
>     that are counted differently by performance counters and MBM
>     respectively, for instance generating "overhead" traffic which is not
>     counted against any specific RMID. Until now these counting
>     differences have always been "in the noise". On Emerald Rapids
>     systems the maximum MBA throttling (10% memory bandwidth)
>     throttles memory bandwidth to where memory accesses by these other
>     platform features push the memory bandwidth difference between
>     memory controller performance counters and resctrl (MBM) beyond the
>     tests' hardcoded tolerance.
> 
> Make the tests more robust against platform variations:
> 1) Let the buffer used by memory bandwidth tests be guided by the size
>     of the L3 cache.
> 2) Larger buffers require longer initialization time before the buffer can
>     be used to measurement. Rework the tests to ensure that buffer
>     initialization is complete before measurements start.
> 3) Do not compare performance counters and MBM measurements at low
>     bandwidth. The value of "low" is hardcoded to 750MiB based on
>     measurements on Emerald Rapids, Sapphire Rapids, and Ice Lake
>     systems. This limit is not applicable to AMD systems since it
>     only applies to the MBA and MBM tests that are isolated to Intel.
> 
> [1]
> https://ark.intel.com/content/www/us/en/ark/products/237261/intel-xeon-platinum-8592-processor-320m-cache-1-9-ghz.html
> 
> Reinette Chatre (15):
>    selftests/resctrl: Make functions only used in same file static
>    selftests/resctrl: Print accurate buffer size as part of MBM results
>    selftests/resctrl: Fix memory overflow due to unhandled wraparound
>    selftests/resctrl: Protect against array overrun during iMC config
>      parsing
>    selftests/resctrl: Protect against array overflow when reading strings
>    selftests/resctrl: Make wraparound handling obvious
>    selftests/resctrl: Remove "once" parameter required to be false
>    selftests/resctrl: Only support measured read operation
>    selftests/resctrl: Remove unused measurement code
>    selftests/resctrl: Make benchmark parameter passing robust
>    selftests/resctrl: Ensure measurements skip initialization of default
>      benchmark
>    selftests/resctrl: Use cache size to determine "fill_buf" buffer size
>    selftests/resctrl: Do not compare performance counters and resctrl at
>      low bandwidth
>    selftests/resctrl: Keep results from first test run
>    selftests/resctrl: Replace magic constants used as array size
> 
>   tools/testing/selftests/resctrl/cmt_test.c    |  37 +-
>   tools/testing/selftests/resctrl/fill_buf.c    |  45 +-
>   tools/testing/selftests/resctrl/mba_test.c    |  54 ++-
>   tools/testing/selftests/resctrl/mbm_test.c    |  37 +-
>   tools/testing/selftests/resctrl/resctrl.h     |  79 +++-
>   .../testing/selftests/resctrl/resctrl_tests.c |  95 +++-
>   tools/testing/selftests/resctrl/resctrl_val.c | 447 +++++-------------
>   tools/testing/selftests/resctrl/resctrlfs.c   |  19 +-
>   8 files changed, 354 insertions(+), 459 deletions(-)
> 
> 
> base-commit: 2a027d6bb66002c8e50e974676f932b33c5fce10

Is this patch series ready to be applied?

thanks,
-- Shuah

Hi Shuah,

On 10/24/24 3:36 PM, Shuah Khan wrote:
> On 10/24/24 15:18, Reinette Chatre wrote:
> 
> Is this patch series ready to be applied?
> 

It is now ready after receiving anticipated tags. Could you please consider it for inclusion?

Thank you very much.

Reinette

On 11/4/24 15:16, Reinette Chatre wrote:
> Hi Shuah,
> 
> On 10/24/24 3:36 PM, Shuah Khan wrote:
>> On 10/24/24 15:18, Reinette Chatre wrote:
>>
>> Is this patch series ready to be applied?
>>
> 
> It is now ready after receiving anticipated tags. Could you please consider it for inclusion?
> 

yes. I will apply the series for the next release.

thanks,
-- Shuah

On 11/4/24 2:28 PM, Shuah Khan wrote:
> On 11/4/24 15:16, Reinette Chatre wrote:
>> Hi Shuah,
>>
>> On 10/24/24 3:36 PM, Shuah Khan wrote:
>>> On 10/24/24 15:18, Reinette Chatre wrote:
>>>
>>> Is this patch series ready to be applied?
>>>
>>
>> It is now ready after receiving anticipated tags. Could you please consider it for inclusion?
>>
> 
> yes. I will apply the series for the next release.
> 

Thank you very much Shuah.

Reinette

On 11/4/24 16:14, Reinette Chatre wrote:
> 
> 
> On 11/4/24 2:28 PM, Shuah Khan wrote:
>> On 11/4/24 15:16, Reinette Chatre wrote:
>>> Hi Shuah,
>>>
>>> On 10/24/24 3:36 PM, Shuah Khan wrote:
>>>> On 10/24/24 15:18, Reinette Chatre wrote:
>>>>
>>>> Is this patch series ready to be applied?
>>>>
>>>
>>> It is now ready after receiving anticipated tags. Could you please consider it for inclusion?
>>>
>>
>> yes. I will apply the series for the next release.
>>
> 
> Thank you very much Shuah.
> 
> Reinette

Applied to linux-kselftest next for Linux 6.13-rc1.

Tested on my system and worked fine.

thanks,
-- Shuah

On 11/4/24 4:07 PM, Shuah Khan wrote:
> On 11/4/24 16:14, Reinette Chatre wrote:
>>
>>
>> On 11/4/24 2:28 PM, Shuah Khan wrote:
>>> On 11/4/24 15:16, Reinette Chatre wrote:
>>>> Hi Shuah,
>>>>
>>>> On 10/24/24 3:36 PM, Shuah Khan wrote:
>>>>> On 10/24/24 15:18, Reinette Chatre wrote:
>>>>>
>>>>> Is this patch series ready to be applied?
>>>>>
>>>>
>>>> It is now ready after receiving anticipated tags. Could you please consider it for inclusion?
>>>>
>>>
>>> yes. I will apply the series for the next release.
>>>
>>
>> Thank you very much Shuah.
>>
>> Reinette
> 
> Applied to linux-kselftest next for Linux 6.13-rc1.
> 
> Tested on my system and worked fine.

Thank you very much Shuah. I received automated emails from patchwork-bot+linux-kselftest@kernel.org
indicating that versions 1, 2, and 3 were merged (no automated email about v4), but when looking
at the kselftest next branch it is clear that latest version, V4, was indeed merged and all looks good.

Thank you.

Reinette

Hi Shuah,

On 10/24/24 3:36 PM, Shuah Khan wrote:
> 
> Is this patch series ready to be applied?
> 

I believe it is close ... I would like to give Ilpo some time to peek
at patches 2 and 10 to confirm if I got their fixes right this time. The
rest of the series is ready.

Thank you

Reinette

On Thu, 24 Oct 2024, Reinette Chatre wrote:

> Hi Shuah,
> 
> On 10/24/24 3:36 PM, Shuah Khan wrote:
> > 
> > Is this patch series ready to be applied?
> > 
> 
> I believe it is close ... I would like to give Ilpo some time to peek
> at patches 2 and 10 to confirm if I got their fixes right this time. The
> rest of the series is ready.

Hi,

I took a look at those two patches now and they seemed fine to me so this 
series should be ready to go now.

-- 
 i.

On 10/25/24 6:54 AM, Ilpo Järvinen wrote:
> On Thu, 24 Oct 2024, Reinette Chatre wrote:
> 
>> Hi Shuah,
>>
>> On 10/24/24 3:36 PM, Shuah Khan wrote:
>>>
>>> Is this patch series ready to be applied?
>>>
>>
>> I believe it is close ... I would like to give Ilpo some time to peek
>> at patches 2 and 10 to confirm if I got their fixes right this time. The
>> rest of the series is ready.
> 
> Hi,
> 
> I took a look at those two patches now and they seemed fine to me so this 
> series should be ready to go now.
> 

Thank you very much Ilpo.

Reinette