mbox series

[v3,0/5] selftests/resctrl: Add resctrl_tests into kselftest set

Message ID 20220216022641.2998318-1-tan.shaopeng@jp.fujitsu.com (mailing list archive)
Headers show
Series selftests/resctrl: Add resctrl_tests into kselftest set | expand

Message

Shaopeng Tan Feb. 16, 2022, 2:26 a.m. UTC
Hello,

The aim of this series is to make resctrl_tests run by using
kselftest framework.
- I modify resctrl_test Makefile and kselftest Makefile,
  to enable build/run resctrl_tests by using kselftest framework.
  Of course, users can also build/run resctrl_tests without
  using framework as before.
- I change the default limited time for resctrl_tests to 120 seconds, to
  ensure the resctrl_tests finish in limited time on different environments.
- When resctrl file system is not supported by environment or
  resctrl_tests is not run as root, return skip code of kselftest framework.
- If resctrl_tests does not finish in limited time, terminate it as
  same as executing ctrl+c that kills parent process and child process.

Difference from v2:
- I reworte changelog of this patch series.
- I added how to use framework to run resctrl to README. [PATCH v3 2/5]
- License has no dependencies on this patch series, I separated from it this patch series to another patch.
https://lore.kernel.org/lkml/20211213100154.180599-1-tan.shaopeng@jp.fujitsu.com/

With regard to the limited time, I think 120s is not a problem since some tests have a longer
timeout (e.g. net test is 300s). Please let me know if this is wrong.

Thanks,

Shaopeng Tan (5):
  selftests/resctrl: Kill child process before parent process terminates
    if SIGTERM is received
  selftests/resctrl: Make resctrl_tests run using kselftest framework
  selftests/resctrl: Update README about using kselftest framework to
    build/run resctrl_tests
  selftests/resctrl: Change the default limited time to 120 seconds
  selftests/resctrl: Fix resctrl_tests' return code to work with
    selftest framework

 tools/testing/selftests/Makefile              |  1 +
 tools/testing/selftests/resctrl/Makefile      | 20 ++++-------
 tools/testing/selftests/resctrl/README        | 34 +++++++++++++++++++
 .../testing/selftests/resctrl/resctrl_tests.c |  4 +--
 tools/testing/selftests/resctrl/resctrl_val.c |  1 +
 tools/testing/selftests/resctrl/settings      |  1 +
 6 files changed, 45 insertions(+), 16 deletions(-)
 create mode 100644 tools/testing/selftests/resctrl/settings

Comments

Shuah Khan Feb. 18, 2022, 8:44 p.m. UTC | #1
On 2/15/22 7:26 PM, Shaopeng Tan wrote:
> Hello,
> 
> The aim of this series is to make resctrl_tests run by using
> kselftest framework.
> - I modify resctrl_test Makefile and kselftest Makefile,
>    to enable build/run resctrl_tests by using kselftest framework.
>    Of course, users can also build/run resctrl_tests without
>    using framework as before.
> - I change the default limited time for resctrl_tests to 120 seconds, to
>    ensure the resctrl_tests finish in limited time on different environments.
> - When resctrl file system is not supported by environment or
>    resctrl_tests is not run as root, return skip code of kselftest framework.
> - If resctrl_tests does not finish in limited time, terminate it as
>    same as executing ctrl+c that kills parent process and child process.
> 
> Difference from v2:
> - I reworte changelog of this patch series.
> - I added how to use framework to run resctrl to README. [PATCH v3 2/5]
> - License has no dependencies on this patch series, I separated from it this patch series to another patch.
> https://lore.kernel.org/lkml/20211213100154.180599-1-tan.shaopeng@jp.fujitsu.com/
> 
> With regard to the limited time, I think 120s is not a problem since some tests have a longer
> timeout (e.g. net test is 300s). Please let me know if this is wrong.
> 
> Thanks,
> 
> Shaopeng Tan (5):
>    selftests/resctrl: Kill child process before parent process terminates
>      if SIGTERM is received
>    selftests/resctrl: Make resctrl_tests run using kselftest framework
>    selftests/resctrl: Update README about using kselftest framework to
>      build/run resctrl_tests
>    selftests/resctrl: Change the default limited time to 120 seconds
>    selftests/resctrl: Fix resctrl_tests' return code to work with
>      selftest framework
> 
>   tools/testing/selftests/Makefile              |  1 +
>   tools/testing/selftests/resctrl/Makefile      | 20 ++++-------
>   tools/testing/selftests/resctrl/README        | 34 +++++++++++++++++++
>   .../testing/selftests/resctrl/resctrl_tests.c |  4 +--
>   tools/testing/selftests/resctrl/resctrl_val.c |  1 +
>   tools/testing/selftests/resctrl/settings      |  1 +
>   6 files changed, 45 insertions(+), 16 deletions(-)
>   create mode 100644 tools/testing/selftests/resctrl/settings
> 

Reviewed the patches - patches 1/5, 4/5 & 5/5 don't depend on kselftest
framework improvements. 2/5 and 3/5 are.

Please reorder the patches - move 4/5 and 5/5 up and make 2/5 and 3/5
the last in this series. Also see comments on individual patches.

thanks,
-- Shuah
Shaopeng Tan (Fujitsu) Feb. 25, 2022, 8:03 a.m. UTC | #2
Hi Shuah,

> On 2/15/22 7:26 PM, Shaopeng Tan wrote:
> > Hello,
> >
> > The aim of this series is to make resctrl_tests run by using kselftest
> > framework.
> > - I modify resctrl_test Makefile and kselftest Makefile,
> >    to enable build/run resctrl_tests by using kselftest framework.
> >    Of course, users can also build/run resctrl_tests without
> >    using framework as before.
> > - I change the default limited time for resctrl_tests to 120 seconds, to
> >    ensure the resctrl_tests finish in limited time on different environments.
> > - When resctrl file system is not supported by environment or
> >    resctrl_tests is not run as root, return skip code of kselftest framework.
> > - If resctrl_tests does not finish in limited time, terminate it as
> >    same as executing ctrl+c that kills parent process and child process.
> >
> > Difference from v2:
> > - I reworte changelog of this patch series.
> > - I added how to use framework to run resctrl to README. [PATCH v3
> > 2/5]
> > - License has no dependencies on this patch series, I separated from it this
> patch series to another patch.
> > https://lore.kernel.org/lkml/20211213100154.180599-1-tan.shaopeng@jp.f
> > ujitsu.com/
> >
> > With regard to the limited time, I think 120s is not a problem since
> > some tests have a longer timeout (e.g. net test is 300s). Please let me know if
> this is wrong.
> >
> > Thanks,
> >
> > Shaopeng Tan (5):
> >    selftests/resctrl: Kill child process before parent process terminates
> >      if SIGTERM is received
> >    selftests/resctrl: Make resctrl_tests run using kselftest framework
> >    selftests/resctrl: Update README about using kselftest framework to
> >      build/run resctrl_tests
> >    selftests/resctrl: Change the default limited time to 120 seconds
> >    selftests/resctrl: Fix resctrl_tests' return code to work with
> >      selftest framework
> >
> >   tools/testing/selftests/Makefile              |  1 +
> >   tools/testing/selftests/resctrl/Makefile      | 20 ++++-------
> >   tools/testing/selftests/resctrl/README        | 34
> +++++++++++++++++++
> >   .../testing/selftests/resctrl/resctrl_tests.c |  4 +--
> >   tools/testing/selftests/resctrl/resctrl_val.c |  1 +
> >   tools/testing/selftests/resctrl/settings      |  1 +
> >   6 files changed, 45 insertions(+), 16 deletions(-)
> >   create mode 100644 tools/testing/selftests/resctrl/settings
> >
> 
> Reviewed the patches - patches 1/5, 4/5 & 5/5 don't depend on kselftest
> framework improvements. 2/5 and 3/5 are.
> 
> Please reorder the patches - move 4/5 and 5/5 up and make 2/5 and 3/5 the
> last in this series. Also see comments on individual patches.

Ok, I will reorder all patches as follows, so that independent patches come first
and Makefile related patches come last:
[PATCH 1/5] selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received
[PATCH 4/5] selftests/resctrl: Change the default limited time to 120 seconds
[PATCH 5/5] selftests/resctrl: Fix resctrl_tests' return code to work with selftest framework
[PATCH 2/5] selftests/resctrl: Make resctrl_tests run using kselftest framework
[PATCH 3/5] selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests
[PATCH] selftests/resctrl: Add missing SPDX license to Makefile

Please let me know if I'm wrong.

Best regards,
Tan Shaopeng
Shuah Khan Feb. 25, 2022, 6:22 p.m. UTC | #3
On 2/25/22 1:03 AM, tan.shaopeng@fujitsu.com wrote:
> Hi Shuah,
> 
>> On 2/15/22 7:26 PM, Shaopeng Tan wrote:
>>> Hello,
>>>
>>> The aim of this series is to make resctrl_tests run by using kselftest
>>> framework.
>>> - I modify resctrl_test Makefile and kselftest Makefile,
>>>     to enable build/run resctrl_tests by using kselftest framework.
>>>     Of course, users can also build/run resctrl_tests without
>>>     using framework as before.
>>> - I change the default limited time for resctrl_tests to 120 seconds, to
>>>     ensure the resctrl_tests finish in limited time on different environments.
>>> - When resctrl file system is not supported by environment or
>>>     resctrl_tests is not run as root, return skip code of kselftest framework.
>>> - If resctrl_tests does not finish in limited time, terminate it as
>>>     same as executing ctrl+c that kills parent process and child process.
>>>
>>> Difference from v2:
>>> - I reworte changelog of this patch series.
>>> - I added how to use framework to run resctrl to README. [PATCH v3
>>> 2/5]
>>> - License has no dependencies on this patch series, I separated from it this
>> patch series to another patch.
>>> https://lore.kernel.org/lkml/20211213100154.180599-1-tan.shaopeng@jp.f
>>> ujitsu.com/
>>>
>>> With regard to the limited time, I think 120s is not a problem since
>>> some tests have a longer timeout (e.g. net test is 300s). Please let me know if
>> this is wrong.
>>>
>>> Thanks,
>>>
>>> Shaopeng Tan (5):
>>>     selftests/resctrl: Kill child process before parent process terminates
>>>       if SIGTERM is received
>>>     selftests/resctrl: Make resctrl_tests run using kselftest framework
>>>     selftests/resctrl: Update README about using kselftest framework to
>>>       build/run resctrl_tests
>>>     selftests/resctrl: Change the default limited time to 120 seconds
>>>     selftests/resctrl: Fix resctrl_tests' return code to work with
>>>       selftest framework
>>>
>>>    tools/testing/selftests/Makefile              |  1 +
>>>    tools/testing/selftests/resctrl/Makefile      | 20 ++++-------
>>>    tools/testing/selftests/resctrl/README        | 34
>> +++++++++++++++++++
>>>    .../testing/selftests/resctrl/resctrl_tests.c |  4 +--
>>>    tools/testing/selftests/resctrl/resctrl_val.c |  1 +
>>>    tools/testing/selftests/resctrl/settings      |  1 +
>>>    6 files changed, 45 insertions(+), 16 deletions(-)
>>>    create mode 100644 tools/testing/selftests/resctrl/settings
>>>
>>
>> Reviewed the patches - patches 1/5, 4/5 & 5/5 don't depend on kselftest
>> framework improvements. 2/5 and 3/5 are.
>>
>> Please reorder the patches - move 4/5 and 5/5 up and make 2/5 and 3/5 the
>> last in this series. Also see comments on individual patches.
> 
> Ok, I will reorder all patches as follows, so that independent patches come first
> and Makefile related patches come last:
> [PATCH 1/5] selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received
> [PATCH 4/5] selftests/resctrl: Change the default limited time to 120 seconds
> [PATCH 5/5] selftests/resctrl: Fix resctrl_tests' return code to work with selftest framework
> [PATCH 2/5] selftests/resctrl: Make resctrl_tests run using kselftest framework
> [PATCH 3/5] selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests
> [PATCH] selftests/resctrl: Add missing SPDX license to Makefile
> 
> Please let me know if I'm wrong.
> 

This split looks good to me.

thanks,
-- Shuah