Re: [rt-tests v2 v2 00/20] rt-numa.h cleanups

[John Kacur <jkacur@redhat.com>]

On Fri, 18 Dec 2020, Daniel Wagner wrote:

> This series cleans up the rt-numa.h header.
> 
> As we have a hard dependency on libnuma we can simplify the code in
> cyclictest. This allows remove all the small helpers in rt_numa.h. And
> with this we can remove the header and reduce the confusion with
> rt-numa.h

I'm not entirely convinced. The code has been moving in this direction,
but I don't know what motivation there is for libnuma people to provide 
libnuma on architectures without numa. This is what the code used to do, 
it wrapped these calls in a way that we could call them whether numa was 
available or not. On Fedora there is no libnuma (numactl) for arm 
processors currently afaik as an example. But, there are a lot of good 
changes here and I  want the code to go forward, so there is no point 
holding on to functionality that we largely got rid of, but if in the
future we have arches that won't work on some distributions,
we may be forced to reimpliment the numa wrappers.

> 
> While at it, I simplified the --smp vs --affinity vs --threads
> logic. There is no need for additional variables to keep state. With
> this we also make --affinity to behave as with the rest of
> rt-tests. That is a plan -a will be the same as with -S. There is no
> need for -S anymore but I think we should leave it in place for
> backwards compatibility. I suspect, there must be a lot of muscle
> memory out there :)

This is really good. Note, the first step that got us here, was the code
that just detected whether numa was available and used it automatically
if it was, unless smp was specified. Believe it or not though, some of
the motivation for that was to reduce the alphabet soup of options on
cyclictest. If -S is no longer needed, it should be removed, people can
learn new muscle memory. (I'm typing this on a Canadian (English layout)
keyboard after using a German keyboard for 13+ years.)


> 
> Since signaltest has the same code as cyclictest, cleanup this tool as
> well.
> 
> With all those cleanups in cyclictest and signaltest, rt-numa.h
> contains only a three functions. The final part of the series is to
> remove unnecessary function arguments (max_cpus). This also fixes the
> theoretical problem that a sparse cpumask would not be handled
> correctly.
> 
> changes v2:
>   - added more cleanup patches (7-20)
> 
> Daniel Wagner (20):
>   cyclictest: Always use libnuma
>   cyclictest: Use numa API directly
>   cyclictest: Use affinity_mask for stearing thread placement
>   cyclictest: Mimik --smp behavior with --affinity
>   cyclictest: Simplify --smp vs --affinity vs --threads argument logic
>   cyclictest: Move verbose message into main
>   signaltest: Always use libnuma
>   signaltest: Use affinity_mask for stearing thread placement
>   signaltest:  Simplify --smp vs --affinity vs --threads argument logic
>   rt-numa: Remove unused definitions and numa_initialize()
>   rt-numa: Add generic cpu_for_thread() helper
>   rt-numa: Use mask size for iterator limit
>   rt-numa: Remove max_cpus argument from parse_cpusmask
>   rt-numa: Use error message helpers
>   signaltest: Remove unused max_cpus argument from process_options
>   cyclictest: Remove unused max_cpus argument from process_options
>   rt-numa: Use CPU_SETSIZE as upper loop limit
>   rt-numa: Remove used max_cpus argument from cpu_for_thread()
>   cyclictest: Remove max cpus used verbose information
>   cyclictest: Remove unecessary local variable
> 
>  src/cyclictest/cyclictest.c | 177 +++++++++++++-----------------------
>  src/cyclictest/rt_numa.h    |  98 --------------------
>  src/include/rt-numa.h       |  13 +--
>  src/lib/rt-numa.c           |  44 ++++-----
>  src/oslat/oslat.c           |   3 +-
>  src/signaltest/signaltest.c |  80 ++++------------
>  6 files changed, 101 insertions(+), 314 deletions(-)
>  delete mode 100644 src/cyclictest/rt_numa.h
> 
> -- 
> 2.29.2
> 
>