Re: [PATCH v3 00/21] Miscellaneous fixes for resctrl selftests

[Shuah Khan]

On 10/20/20 5:51 PM, Fenghua Yu wrote:
> This patch set has several miscellaneous fixes to resctrl selftest tool
> that are easily visible to user. V1 had fixes to CAT test and CMT test
> but they were dropped in V2 because having them here made the patchset
> humongous. So, changes to CAT test and CMT test will be posted in another
> patchset.

This is still a very long patch series. Several of the patches can be
combined and can be rearranged. 21 patches don't seem to any specific
order.

> Change Log:
> v3:
> Address various comments (commit messages, return value on test failure,
> print failure info on test failure etc) from Reinette and Tony.
> [v2: https://lore.kernel.org/linux-kselftest/cover.1589835155.git.sai.praneeth.prakhya@xxxxxxxxx/]
>
> v2:
> 1. Dropped changes to CAT test and CMT test as they will be posted in a later
>     series.
> 2. Added several other fixes
> [v1: https://lore.kernel.org/linux-kselftest/cover.1583657204.git.sai.praneeth.prakhya@xxxxxxxxx/]
>
> Fenghua Yu (18):
>    selftests/resctrl: Rename CQM test as CMT test
>    selftests/resctrl: Declare global variables as extern
>    selftests/resctrl: Return if resctrl file system is not supported
>    selftests/resctrl: Check for resctrl mount point only if resctrl FS is
>      supported
>    selftests/resctrl: Use resctrl/info for feature detection
>    selftests/resctrl: Fix missing options "-n" and "-p"
>    selftests/resctrl: Fix MBA/MBM results reporting format
>    selftests/resctrl: Abort running tests if not root user
>    selftests/resctrl: Enable gcc checks to detect buffer overflows
>    selftests/resctrl: Don't hard code value of "no_of_bits" variable

>    selftests/resctrl: Modularize resctrl test suite main() function

Yes. This is a needed change. I didn't make it to this patch yet.

>    selftests/resctrl: Skip the test if requested resctrl feature is not
>      supported

Commented on this patch already. Look into using config file like other
tests.

>    selftests/resctrl: Umount resctrl FS only if mounted
>    selftests/resctrl: Unmount resctrl FS after running all tests
>    selftests/resctrl: Fix incorrect parsing of iMC counters
>    selftests/resctrl: Fix checking for < 0 for unsigned values
>    selftests/resctrl: Fix unnecessary usage of global variables
>    selftests/resctrl: Don't use global variable for capacity bitmask
>      (CBM)
>
> Reinette Chatre (3):
>    selftests/resctrl: Fix typo
>    selftests/resctrl: Fix typo in help text

Why not combine the above two patches. The commit summary doesn't
make sense.

>    selftests/resctrl: Ensure sibling CPU is not same as original CPU
>
>   tools/testing/selftests/resctrl/Makefile      |   2 +-
>   tools/testing/selftests/resctrl/README        |   4 +-
>   tools/testing/selftests/resctrl/cache.c       |   4 +-
>   tools/testing/selftests/resctrl/cat_test.c    |  20 +--
>   .../resctrl/{cqm_test.c => cmt_test.c}        |  34 ++--
>   tools/testing/selftests/resctrl/mba_test.c    |  23 ++-
>   tools/testing/selftests/resctrl/mbm_test.c    |  16 +-
>   tools/testing/selftests/resctrl/resctrl.h     |  20 ++-
>   .../testing/selftests/resctrl/resctrl_tests.c | 156 ++++++++++++------
>   tools/testing/selftests/resctrl/resctrl_val.c |  75 ++++++---
>   tools/testing/selftests/resctrl/resctrlfs.c   |  79 ++++++---
>   11 files changed, 272 insertions(+), 161 deletions(-)
>   rename tools/testing/selftests/resctrl/{cqm_test.c => cmt_test.c} (85%)

I will review rest of the patches. Try to combine a few patches and
collapse fixes. I would like to see all the fixes first and then
renaming from CQM to CMT.

thanks,
-- Shuah