Message ID | 20250527-selftests-mm-cow-dedupe-v2-2-ff198df8e38e@kernel.org |
---|---|
State | New |
Headers | show |
Series | selftests/mm: cow and gup_longterm cleanups | expand |
On 27.05.25 18:04, Mark Brown wrote: > Several of the MM tests have a pattern of printing a description of the > test to be run then reporting the actual TAP result using a generic string > not connected to the specific test, often in a shared function used by many > tests. The name reported typically varies depending on the specific result > rather than the test too. This causes problems for tooling that works with > test results, the names reported with the results are used to deduplicate > tests and track them between runs so both duplicated names and changing > names cause trouble for things like UIs and automated bisection. > > As a first step towards matching these tests better with the expectations > of kselftest provide helpers which record the test name as part of the > initial print and then use that as part of reporting a result. > > This is not added as a generic kselftest helper partly because the use of > a variable to store the test name doesn't fit well with the header only > implementation of kselftest.h and partly because it's not really an > intended pattern. Ideally at some point the mm tests that use it will be > updated to not need it. > > Signed-off-by: Mark Brown <broonie@kernel.org> > --- > tools/testing/selftests/mm/vm_util.h | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h > index 6effafdc4d8a..4944e4c79051 100644 > --- a/tools/testing/selftests/mm/vm_util.h > +++ b/tools/testing/selftests/mm/vm_util.h > @@ -3,6 +3,7 @@ > #include <stdbool.h> > #include <sys/mman.h> > #include <err.h> > +#include <stdarg.h> > #include <strings.h> /* ffsl() */ > #include <unistd.h> /* _SC_PAGESIZE */ > #include "../kselftest.h" > @@ -74,6 +75,25 @@ int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len, > unsigned long get_free_hugepages(void); > bool check_vmflag_io(void *addr); > > +/* These helpers need to be inline to match the kselftest.h idiom. */ > +static char test_name[1024]; > + > +static inline void log_test_start(const char *name, ...) > +{ > + va_list args; > + va_start(args, name); > + > + vsnprintf(test_name, sizeof(test_name), name, args); > + ksft_print_msg("[RUN] %s\n", test_name); > + > + va_end(args); > +} > + > +static inline void log_test_result(int result) > +{ > + ksft_test_result_report(result, "%s\n", test_name); > +} Won't win a beauty contest, but should get the job done. We could allocate the array in log_test_start() and free it in log_test_result(). Then, we could assert more easily that we always have a log_test_result() follow exactly one log_test_start() etc.
On Tue, Jun 03, 2025 at 02:37:41PM +0200, David Hildenbrand wrote: > On 27.05.25 18:04, Mark Brown wrote: > > +static char test_name[1024]; > > + > > +static inline void log_test_start(const char *name, ...) > > +{ > > + va_list args; > > + va_start(args, name); > > + > > + vsnprintf(test_name, sizeof(test_name), name, args); > > + ksft_print_msg("[RUN] %s\n", test_name); > We could allocate the array in log_test_start() and free it in > log_test_result(). Then, we could assert more easily that we always have a > log_test_result() follow exactly one log_test_start() etc. We could, however we don't have vasprintf() in nolibc and people have been doing work towards making nolibc more generally useful as a libc for the selftests (and/or the selftest interfaces more friendly to nolibc). I don't really know what the end goal with that is but given the fairly small gain and the hope that this won't be a long term framework for anything I'd rather not add something that gets in the way of whatever's going on there. Ideally the test programs would be refactored and these helpers deleted, but as we said previously that's a bigger job that neither of us is likely to get to in the short term :(
diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index 6effafdc4d8a..4944e4c79051 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -3,6 +3,7 @@ #include <stdbool.h> #include <sys/mman.h> #include <err.h> +#include <stdarg.h> #include <strings.h> /* ffsl() */ #include <unistd.h> /* _SC_PAGESIZE */ #include "../kselftest.h" @@ -74,6 +75,25 @@ int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len, unsigned long get_free_hugepages(void); bool check_vmflag_io(void *addr); +/* These helpers need to be inline to match the kselftest.h idiom. */ +static char test_name[1024]; + +static inline void log_test_start(const char *name, ...) +{ + va_list args; + va_start(args, name); + + vsnprintf(test_name, sizeof(test_name), name, args); + ksft_print_msg("[RUN] %s\n", test_name); + + va_end(args); +} + +static inline void log_test_result(int result) +{ + ksft_test_result_report(result, "%s\n", test_name); +} + /* * On ppc64 this will only work with radix 2M hugepage size */
Several of the MM tests have a pattern of printing a description of the test to be run then reporting the actual TAP result using a generic string not connected to the specific test, often in a shared function used by many tests. The name reported typically varies depending on the specific result rather than the test too. This causes problems for tooling that works with test results, the names reported with the results are used to deduplicate tests and track them between runs so both duplicated names and changing names cause trouble for things like UIs and automated bisection. As a first step towards matching these tests better with the expectations of kselftest provide helpers which record the test name as part of the initial print and then use that as part of reporting a result. This is not added as a generic kselftest helper partly because the use of a variable to store the test name doesn't fit well with the header only implementation of kselftest.h and partly because it's not really an intended pattern. Ideally at some point the mm tests that use it will be updated to not need it. Signed-off-by: Mark Brown <broonie@kernel.org> --- tools/testing/selftests/mm/vm_util.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)