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

Shuah Khan
2020-10-29T00:00:00Z

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@intel.com/]
>
> 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@intel.com/]
>
> 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