mbox series

[v5,0/3] selftests/resctrl: Simplify test cleanup functions

Message ID cover.1709018050.git.maciej.wieczor-retman@intel.com (mailing list archive)
Headers show
Series selftests/resctrl: Simplify test cleanup functions | expand

Message

Maciej Wieczor-Retman Feb. 27, 2024, 7:21 a.m. UTC
Cleaning up after tests is implemented separately for individual tests
and called at the end of each test execution. Since these functions are
very similar and a more generalized test framework was introduced a
function pointer in the resctrl_test struct can be used to reduce the
amount of function calls.

These functions are also all called in the ctrl-c handler because the
handler isn't aware which test is currently running. Since the handler
is implemented with a sigaction no function parameters can be passed
there but information about what test is currently running can be passed
with a global variable.

Series applies cleanly on top of kselftests/next.

Changelog v5:
- Rebase onto kselftests/next.
- Add Reinette's reviewed-by tag.

Changelog v4:
- Check current_test pointer and reset it in signal unregistering.
- Move cleanup call to test_cleanup function.

Changelog v3:
- Make current_test static.
- Add callback NULL check to the ctrl-c handler.

Changelog v2:
- Make current_test a const pointer limited in scope to resctrl_val
  file.
- Remove tests_cleanup from resctrl.h.
- Cleanup 'goto out' path and labels in individual test functions.

Older versions of this series:
[v1] https://lore.kernel.org/all/cover.1708434017.git.maciej.wieczor-retman@intel.com/
[v2] https://lore.kernel.org/all/cover.1708596015.git.maciej.wieczor-retman@intel.com/
[v3] https://lore.kernel.org/all/cover.1708599491.git.maciej.wieczor-retman@intel.com/
[v4] https://lore.kernel.org/all/cover.1708949785.git.maciej.wieczor-retman@intel.com/

Maciej Wieczor-Retman (3):
  selftests/resctrl: Add cleanup function to test framework
  selftests/resctrl: Simplify cleanup in ctrl-c handler
  selftests/resctrl: Move cleanups out of individual tests

 tools/testing/selftests/resctrl/cat_test.c    |  8 +++-----
 tools/testing/selftests/resctrl/cmt_test.c    |  4 ++--
 tools/testing/selftests/resctrl/mba_test.c    |  8 +++-----
 tools/testing/selftests/resctrl/mbm_test.c    |  8 +++-----
 tools/testing/selftests/resctrl/resctrl.h     |  9 +++------
 .../testing/selftests/resctrl/resctrl_tests.c | 20 +++++++------------
 tools/testing/selftests/resctrl/resctrl_val.c |  8 ++++++--
 7 files changed, 27 insertions(+), 38 deletions(-)

Comments

Ilpo Järvinen Feb. 27, 2024, 12:49 p.m. UTC | #1
On Tue, 27 Feb 2024, Maciej Wieczor-Retman wrote:

> Cleaning up after tests is implemented separately for individual tests
> and called at the end of each test execution. Since these functions are
> very similar and a more generalized test framework was introduced a
> function pointer in the resctrl_test struct can be used to reduce the
> amount of function calls.
> 
> These functions are also all called in the ctrl-c handler because the
> handler isn't aware which test is currently running. Since the handler
> is implemented with a sigaction no function parameters can be passed
> there but information about what test is currently running can be passed
> with a global variable.
> 
> Series applies cleanly on top of kselftests/next.

Thanks. For the entire series:

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reinette Chatre Feb. 27, 2024, 4:36 p.m. UTC | #2
Hi Shuah,

Could you please consider this series for inclusion? I do admit that
there has been a lot of resctrl selftest work recently. This should be
it for a while as new work is still being worked on. 

Thank you very much.

Reinette

On 2/26/2024 11:21 PM, Maciej Wieczor-Retman wrote:
> Cleaning up after tests is implemented separately for individual tests
> and called at the end of each test execution. Since these functions are
> very similar and a more generalized test framework was introduced a
> function pointer in the resctrl_test struct can be used to reduce the
> amount of function calls.
> 
> These functions are also all called in the ctrl-c handler because the
> handler isn't aware which test is currently running. Since the handler
> is implemented with a sigaction no function parameters can be passed
> there but information about what test is currently running can be passed
> with a global variable.
> 
> Series applies cleanly on top of kselftests/next.
> 
> Changelog v5:
> - Rebase onto kselftests/next.
> - Add Reinette's reviewed-by tag.
> 
> Changelog v4:
> - Check current_test pointer and reset it in signal unregistering.
> - Move cleanup call to test_cleanup function.
> 
> Changelog v3:
> - Make current_test static.
> - Add callback NULL check to the ctrl-c handler.
> 
> Changelog v2:
> - Make current_test a const pointer limited in scope to resctrl_val
>   file.
> - Remove tests_cleanup from resctrl.h.
> - Cleanup 'goto out' path and labels in individual test functions.
> 
> Older versions of this series:
> [v1] https://lore.kernel.org/all/cover.1708434017.git.maciej.wieczor-retman@intel.com/
> [v2] https://lore.kernel.org/all/cover.1708596015.git.maciej.wieczor-retman@intel.com/
> [v3] https://lore.kernel.org/all/cover.1708599491.git.maciej.wieczor-retman@intel.com/
> [v4] https://lore.kernel.org/all/cover.1708949785.git.maciej.wieczor-retman@intel.com/
> 
> Maciej Wieczor-Retman (3):
>   selftests/resctrl: Add cleanup function to test framework
>   selftests/resctrl: Simplify cleanup in ctrl-c handler
>   selftests/resctrl: Move cleanups out of individual tests
> 
>  tools/testing/selftests/resctrl/cat_test.c    |  8 +++-----
>  tools/testing/selftests/resctrl/cmt_test.c    |  4 ++--
>  tools/testing/selftests/resctrl/mba_test.c    |  8 +++-----
>  tools/testing/selftests/resctrl/mbm_test.c    |  8 +++-----
>  tools/testing/selftests/resctrl/resctrl.h     |  9 +++------
>  .../testing/selftests/resctrl/resctrl_tests.c | 20 +++++++------------
>  tools/testing/selftests/resctrl/resctrl_val.c |  8 ++++++--
>  7 files changed, 27 insertions(+), 38 deletions(-)
>
Reinette Chatre March 27, 2024, 11:08 p.m. UTC | #3
Hi Shuah,

On 2/27/2024 8:36 AM, Reinette Chatre wrote:
> Hi Shuah,
> 
> Could you please consider this series for inclusion? I do admit that
> there has been a lot of resctrl selftest work recently. This should be
> it for a while as new work is still being worked on. 
> 
> Thank you very much.
> 
> Reinette

I think we missed the cutoff for the previous merge window. Could you
please consider this series for inclusion in preparation for
the next merge window?

Thank you very much.

Reinette
Shuah Khan March 28, 2024, 7:25 p.m. UTC | #4
On 3/27/24 17:08, Reinette Chatre wrote:
> Hi Shuah,
> 
> On 2/27/2024 8:36 AM, Reinette Chatre wrote:
>> Hi Shuah,
>>
>> Could you please consider this series for inclusion? I do admit that
>> there has been a lot of resctrl selftest work recently. This should be
>> it for a while as new work is still being worked on.
>>

No worries about the number of patches. Thank you for the cleanups
and fixes.

Looks like there another series in the works :)

  selftests/resctrl: resctrl_val() related cleanups & improvements

>> Thank you very much.
>>
>> Reinette
> 
> I think we missed the cutoff for the previous merge window. Could you
> please consider this series for inclusion in preparation for
> the next merge window?
> 

Applied to linux-kselftest next for 6.10-rc1.

thanks,
-- Shuah
Reinette Chatre March 28, 2024, 8:28 p.m. UTC | #5
Hi Shuah,

On 3/28/2024 12:25 PM, Shuah Khan wrote:
> On 3/27/24 17:08, Reinette Chatre wrote:
>> On 2/27/2024 8:36 AM, Reinette Chatre wrote:

>>> Could you please consider this series for inclusion? I do admit that
>>> there has been a lot of resctrl selftest work recently. This should be
>>> it for a while as new work is still being worked on.
>>>
> 
> No worries about the number of patches. Thank you for the cleanups
> and fixes.
> 
> Looks like there another series in the works :)
> 
>  selftests/resctrl: resctrl_val() related cleanups & improvements

Indeed. There is also a "SNC support for resctrl selftests" [1] being
worked on, but it is waiting on the (not yet upstream) arch/x86 feature
it tests to settle.

> 
> Applied to linux-kselftest next for 6.10-rc1.
> 

Thank you very much.

Reinette


[1] https://lore.kernel.org/lkml/cover.1709721159.git.maciej.wieczor-retman@intel.com/