| Message ID | 20240711104400.63355-2-pvorel@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [RFC,1/2] realtime: Use offsetof() macro from <stddef.h> | expand |
Hi! > Combining LTP librealtime (librttest.c) and LTP library is somehow > experimental. -lltp was needed to be added to CFLAGS but yet on musl > it fails to find the function on runtime: > tst_test.c:985: TBROK: No test function specified That should not happen, you get this message something went horribly wrong and the test_all function pointer ended up NULL in the test library. Which gcc version was it, I would expect that this would be more compiler specific than libc specific. Otherwise the conversion looks pretty straightforward, mostly about option parsing and printf vs tst_res().
Hi Cyril, First, thanks for having a look. > Hi! > > Combining LTP librealtime (librttest.c) and LTP library is somehow > > experimental. -lltp was needed to be added to CFLAGS but yet on musl > > it fails to find the function on runtime: > > tst_test.c:985: TBROK: No test function specified > That should not happen, you get this message something went horribly > wrong and the test_all function pointer ended up NULL in the test > library. Which gcc version was it, I would expect that this would be > more compiler specific than libc specific. $ gcc -v Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-alpine-linux-musl/13.2.1/lto-wrapper Target: x86_64-alpine-linux-musl Configured with: /home/buildozer/aports/main/gcc/src/gcc-13-20231014/configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --build=x86_64-alpine-linux-musl --host=x86_64-alpine-linux-musl --target=x86_64-alpine-linux-musl --enable-checking=release --disable-cet --disable-fixed-point --disable-libstdcxx-pch --disable-multilib --disable-nls --disable-werror --disable-symvers --enable-__cxa_atexit --enable-default-pie --enable-default-ssp --enable-languages=c,c++,d,objc,go,fortran,ada --enable-link-serialization=2 --enable-linker-build-id --disable-libssp --disable-libsanitizer --enable-shared --enable-threads --enable-tls --with-bugurl=https://gitlab.alpinelinux.org/alpine/aports/-/issues --with-system-zlib --with-linker-hash-style=gnu --with-pkgversion='Alpine 13.2.1_git20231014' Thread model: posix Supported LTO compression algorithms: zlib gcc version 13.2.1 20231014 (Alpine 13.2.1_git20231014) Hm, after doing make clean of the library everything magically worked. I'm sorry for the noise. $ make V=1 gcc -I/home/foo/ltp/testcases/realtime/include -I/home/foo/ltp/testcases/realtime/include -I../../../../include -I../../../../include -I../../../../include/old/ -g -O2 -g -O2 -fno-strict-aliasing -pipe -Wall -W -Wold-style-definition -std=gnu99 -D_GNU_SOURCE -L/home/foo/ltp/testcases/realtime/lib -L../../../../lib sched_football.c -lrealtime -lpthread -lrt -lm -lltp -o sched_football # ./sched_football tst_test.c:1806: TINFO: LTP version: 20240524-81-g562d1b39b tst_test.c:1650: TINFO: Timeout per run is 0h 00m 30s sched_football.c:159: TINFO: players_per_team: 2 game_length: 5 sched_football.c:171: TINFO: Starting 2 offense threads at priority 15 sched_football.c:182: TINFO: Starting 2 defense threads at priority 30 sched_football.c:193: TINFO: Starting 2 fan threads at priority 50 sched_football.c:120: TINFO: Starting referee thread sched_football.c:123: TINFO: Starting the game (5 sec) sched_football.c:143: TINFO: Final ball position: 0 sched_football.c:145: TPASS: Expect: final_ball == 0 => main problem solved, but there are 2 others. 1) realtime suite does not trigger libltp dependency (build system fix required): /usr/lib/gcc/x86_64-alpine-linux-musl/13.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: cannot find -lltp: No such file or directory I wonder if we should get rid of LTP realtime library for this file. Fixing it would be obviously better. 2) Running with -i is broken (everywhere, also on glibc) ./sched_football -i2 tst_test.c:1806: TINFO: LTP version: 20240524-93-g17c91c528 tst_test.c:1650: TINFO: Timeout per run is 0h 00m 30s sched_football.c:159: TINFO: players_per_team: 2 game_length: 5 sched_football.c:171: TINFO: Starting 2 offense threads at priority 15 sched_football.c:182: TINFO: Starting 2 defense threads at priority 30 sched_football.c:193: TINFO: Starting 2 fan threads at priority 50 sched_football.c:120: TINFO: Starting referee thread sched_football.c:123: TINFO: Starting the game (5 sec) sched_football.c:143: TINFO: Final ball position: 0 sched_football.c:145: TPASS: Expect: final_ball == 0 sched_football.c:159: TINFO: players_per_team: 2 game_length: 5 sched_football.c:171: TINFO: Starting 2 offense threads at priority 15 Test timeouted, sending SIGKILL! Something needs to be reset for each run and it's not? ball is reset: tst_atomic_store(0, &ball); Kind regards, Petr > Otherwise the conversion looks pretty straightforward, mostly about > option parsing and printf vs tst_res().
Hi! > Hm, after doing make clean of the library everything magically worked. I'm sorry > for the noise. I guess that the reason for most problems we have is that LTP does not have a proper dependencies in the build system. I guess that I can try to add support for autogenerating dep files and including them, but I'm afraid that with the amount of complexity we have there because of the out-of-tree build support things will get very complicated quite fast.
> Hi! > > Hm, after doing make clean of the library everything magically worked. I'm sorry > > for the noise. > I guess that the reason for most problems we have is that LTP does not > have a proper dependencies in the build system. I guess that I can try > to add support for autogenerating dep files and including them, but I'm > afraid that with the amount of complexity we have there because of the > out-of-tree build support things will get very complicated quite fast. I hoped there is something I just overlooked. Even I'm not a big fan of meson, instead of implementing something own I would consider migrating to it. Looking into Andrea's POV, where he did at least some work [1]. But back to the reality, would it be possible to merge this even with broken dependency? I'm not sure myself. Kind regards, Petr [1] https://github.com/acerv/ltp-core/
Hi! > I hoped there is something I just overlooked. Even I'm not a big fan of meson, > instead of implementing something own I would consider migrating to it. > Looking into Andrea's POV, where he did at least some work [1]. I'm afraid that meson is not good choice for us. LTP has to be able to be compiled everywhere even on ten years old distribution and very restricted embedded devices, hence I would like us to stay on the most common and stable build system tooling out there. Which is plain old make. And the main problem with our build system is not the tooling we choose, but the complexity imposed by the out-of-tree build implemented in the complex makefiles. As far as I can tell 99% of the problems would be solved by ripping out out-of-tree support, which would remove most of the code we have in there. > But back to the reality, would it be possible to merge this even with broken > dependency? I'm not sure myself. I will double check the code, before adding my final reviewed-by. Also this is a step number one. I think that we should enable the realtime compilation by default and the sched_football should be added to the scheduller runtest file as well, so that it's executed by default as well.
> Hi! > > I hoped there is something I just overlooked. Even I'm not a big fan of meson, > > instead of implementing something own I would consider migrating to it. > > Looking into Andrea's POV, where he did at least some work [1]. > I'm afraid that meson is not good choice for us. LTP has to be able to > be compiled everywhere even on ten years old distribution and very > restricted embedded devices, hence I would like us to stay on the most > common and stable build system tooling out there. Which is plain old > make. Correct + not pushing for it now. FYI iputils compilation worked even on old CentOS 7 (well, meson and ninja got over epel). 15-SP2 (IMHO the oldest distro LTP upstream cares) has meson 0.54.2 and ninja 1.10, these would be enough for initial support. Therefore once we bump the support higher (next year), we *could* consider moving away from autotools (obviously starting to ask on automated-testing@yoctoproject.org if somebody has problems with the dependencies. > And the main problem with our build system is not the tooling we choose, > but the complexity imposed by the out-of-tree build implemented in the > complex makefiles. As far as I can tell 99% of the problems would be > solved by ripping out out-of-tree support, which would remove most of > the code we have in there. +1 for removal. I wonder if anybody finds a time to do that. > > But back to the reality, would it be possible to merge this even with broken > > dependency? I'm not sure myself. > I will double check the code, before adding my final reviewed-by. Thank you! It still bothers me that building sched_football does not trigger building a library, but at least it will work for a full build (building whole LTP). > Also this is a step number one. I think that we should enable the > realtime compilation by default and the sched_football should be added > to the scheduller runtest file as well, so that it's executed by default > as well. +1, I'll have look into it. Kind regards, Petr
Hi! > > And the main problem with our build system is not the tooling we choose, > > but the complexity imposed by the out-of-tree build implemented in the > > complex makefiles. As far as I can tell 99% of the problems would be > > solved by ripping out out-of-tree support, which would remove most of > > the code we have in there. > > +1 for removal. I wonder if anybody finds a time to do that. That is unfortunatelly a major task... > > > But back to the reality, would it be possible to merge this even with broken > > > dependency? I'm not sure myself. > > > I will double check the code, before adding my final reviewed-by. > > Thank you! It still bothers me that building sched_football does not trigger > building a library, but at least it will work for a full build (building whole > LTP). It's even more subtle, there are more things to consider. The test should be rebuild even if any of the headers it includes change, which is not detected properly at the moment. For that to work first thing the build system has to do is to generate a dependencies with $(CC) -MM and then include them in the build system.
> Hi! > > > And the main problem with our build system is not the tooling we choose, > > > but the complexity imposed by the out-of-tree build implemented in the > > > complex makefiles. As far as I can tell 99% of the problems would be > > > solved by ripping out out-of-tree support, which would remove most of > > > the code we have in there. > > +1 for removal. I wonder if anybody finds a time to do that. > That is unfortunatelly a major task... > > > > But back to the reality, would it be possible to merge this even with broken > > > > dependency? I'm not sure myself. > > > I will double check the code, before adding my final reviewed-by. > > Thank you! It still bothers me that building sched_football does not trigger > > building a library, but at least it will work for a full build (building whole > > LTP). > It's even more subtle, there are more things to consider. The test > should be rebuild even if any of the headers it includes change, which > is not detected properly at the moment. For that to work first thing the > build system has to do is to generate a dependencies with $(CC) -MM and > then include them in the build system. Cool, it really detects required headers. $ gcc -MM -I../../include tst_needs_cmds04.c tst_needs_cmds04.o: tst_needs_cmds04.c ../../include/tst_test.h \ ../../include/tst_common.h ../../include/tst_res_flags.h \ ../../include/tst_parse.h ../../include/tst_test_macros.h \ ../../include/tst_checkpoint.h ../../include/tst_checkpoint_fn.h \ ../../include/tst_device.h ../../include/tst_mkfs.h \ ../../include/tst_fs.h ../../include/tst_pid.h ../../include/tst_cmd.h \ ../../include/tst_cpu.h ../../include/tst_process_state.h \ ... ../../include/tst_cgroup.h Kind regards, Petr
Hi! > -/* Here's the position of the ball */ > -static int the_ball; > - > +static int ball; Honestly the only thing that I do not like about this is that you dropped the the_ from the ball, because *the* ball is *the* thing that we observe for the whole time. Otherwise I do not think that the test library can interfere with the test in any way so we can go ahead and push this. Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
> Hi! > > -/* Here's the position of the ball */ > > -static int the_ball; > > - > > +static int ball; > Honestly the only thing that I do not like about this is that you > dropped the the_ from the ball, because *the* ball is *the* thing that > we observe for the whole time. OK, I put 'the_ball' back. > Otherwise I do not think that the test library can interfere with the > test in any way so we can go ahead and push this. Thanks! I also add TODO to makefile for proper integration, removed unneeded includes (mostly included by tst_test.h or unused) and merged! Kind regards, Petr
diff --git a/testcases/realtime/func/sched_football/Makefile b/testcases/realtime/func/sched_football/Makefile index 61753f0309..f194c2fbfc 100644 --- a/testcases/realtime/func/sched_football/Makefile +++ b/testcases/realtime/func/sched_football/Makefile @@ -25,4 +25,5 @@ top_srcdir ?= ../../../.. INSTALL_TARGETS := run_auto.sh include $(top_srcdir)/include/mk/env_pre.mk include $(abs_srcdir)/../../config.mk +LDLIBS += -lltp include $(top_srcdir)/include/mk/generic_leaf_target.mk diff --git a/testcases/realtime/func/sched_football/sched_football.c b/testcases/realtime/func/sched_football/sched_football.c index b6ae692af7..a3e4cbb344 100644 --- a/testcases/realtime/func/sched_football/sched_football.c +++ b/testcases/realtime/func/sched_football/sched_football.c @@ -1,62 +1,37 @@ -/****************************************************************************** +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright © International Business Machines Corp., 2007, 2008 + * Copyright (c) 2024 Petr Vorel <pvorel@suse.cz> + * Author: John Stultz <jstultz@google.com> + */ + +/*\ + * [Description] * - * Copyright © International Business Machines Corp., 2007, 2008 + * Scheduler test that uses a football analogy. * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. + * The premise is that we want to make sure that lower priority threads + * don't run while we have runnable higher priority threads. + * The offense is trying to increment the balls position, while the + * defense is trying to block that from happening. + * And the ref (highest priority thread) will blow the wistle if the + * ball moves. Finally, we have crazy fans (higer prority) that try to + * distract the defense by occasionally running onto the field. * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See - * the GNU General Public License for more details. + * [Algorithm] * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - * - * NAME - * sched_football.c - * - * DESCRIPTION - * This is a scheduler test that uses a football analogy. - * The premise is that we want to make sure that lower priority threads - * don't run while we have runnable higher priority threads. - * The offense is trying to increment the balls position, while the - * defense is trying to block that from happening. - * And the ref (highest priority thread) will blow the wistle if the - * ball moves. Finally, we have crazy fans (higer prority) that try to - * distract the defense by occasionally running onto the field. - * - * Steps: - * - Create NR_CPU offense threads (lower priority) - * - Create NR_CPU defense threads (mid priority) - * - Create 2*NR_CPU fan threads (high priority) - * - Create a referee thread (highest priority) - * - Once everyone is on the field, the offense thread spins incrementing - * the value of 'the_ball'. The defense thread tries to block the ball - * by never letting the offense players get the CPU (it just spins). - * The crazy fans sleep a bit, then jump the rail and run across the - * field, disrupting the players on the field. - * - The refree threads wakes up regularly to check if the game is over :) - * - In the end, if the value of 'the_ball' is >0, the test is considered - * to have failed. - * - * USAGE: - * Use run_auto.sh script in current directory to build and run test. - * - * AUTHOR - * John Stultz <johnstul@xxxxxxxxx > - * - * HISTORY - * 2006-03-16 Reduced verbosity, non binary failure reporting, removal of - * crazy_fans thread, added game_length argument by Darren Hart. - * 2007-08-01 Remove all thread cleanup in favor of simply exiting.Various - * bugfixes and cleanups. -- Josh Triplett - * 2009-06-23 Simplified atomic startup mechanism, avoiding thundering herd - * scheduling at the beginning of the game. -- Darren Hart - *****************************************************************************/ + * - Create NR_CPU offense threads (lower priority). + * - Create NR_CPU defense threads (mid priority). + * - Create 2*NR_CPU fan threads (high priority). + * - Create a referee thread (highest priority). + * - Once everyone is on the field, the offense thread spins incrementing + * the value of ball. The defense thread tries to block the ball + * by never letting the offense players get the CPU (it just spins). + * The crazy fans sleep a bit, then jump the rail and run across the + * field, disrupting the players on the field. + * - The refree threads wakes up regularly to check if the game is over :). + * - If the value of ball is > 0, the test is considered to have failed. + */ #include <stdio.h> #include <stdlib.h> @@ -71,55 +46,25 @@ #include <unistd.h> #include <sys/prctl.h> #include <sys/time.h> -#include <librttest.h> #include <tst_atomic.h> -#define TST_NO_DEFAULT_MAIN #include <tst_timer.h> - +#include "librttest.h" +#include "tst_test.h" #define DEF_GAME_LENGTH 5 +#define SPIN_TIME_NS 200000000ULL +#define SLEEP_TIME_NS 50000000ULL -/* Here's the position of the ball */ -static int the_ball; - +static int ball; static int players_per_team = 0; static int game_length = DEF_GAME_LENGTH; static int players_ready; -void usage(void) -{ - rt_help(); - printf("sched_football specific options:\n"); - printf(" -nPLAYERS players per team (defaults to num_cpus)\n"); - printf(" -lGAME_LENGTH game length in seconds (defaults to %d s)\n", - DEF_GAME_LENGTH); -} - -int parse_args(int c, char *v) -{ +static char *str_game_length; +static char *str_players_per_team; - int handled = 1; - switch (c) { - case 'h': - usage(); - exit(0); - case 'n': - players_per_team = atoi(v); - break; - case 'l': - game_length = atoi(v); - break; - default: - handled = 0; - break; - } - return handled; -} - -#define SPIN_TIME_NS 200000000ULL -#define SLEEP_TIME_NS 50000000ULL /* These are fans running across the field. They're trying to interrupt/distract everyone */ -void *thread_fan(void *arg) +void *thread_fan(void *arg LTP_ATTRIBUTE_UNUSED) { prctl(PR_SET_NAME, "crazy_fan", 0, 0, 0); tst_atomic_add_return(1, &players_ready); @@ -139,38 +84,43 @@ void *thread_fan(void *arg) nsec = tst_timespec_diff_ns(stop, start); } } + return NULL; } /* This is the defensive team. They're trying to block the offense */ -void *thread_defense(void *arg) +void *thread_defense(void *arg LTP_ATTRIBUTE_UNUSED) { prctl(PR_SET_NAME, "defense", 0, 0, 0); tst_atomic_add_return(1, &players_ready); /*keep the ball from being moved */ while (1) { } + return NULL; } /* This is the offensive team. They're trying to move the ball */ -void *thread_offense(void *arg) +void *thread_offense(void *arg LTP_ATTRIBUTE_UNUSED) { prctl(PR_SET_NAME, "offense", 0, 0, 0); tst_atomic_add_return(1, &players_ready); while (1) { - tst_atomic_add_return(1, &the_ball); /* move the ball ahead one yard */ + tst_atomic_add_return(1, &ball); /* move the ball ahead one yard */ } + return NULL; } -int referee(int game_length) +void referee(int game_length) { struct timeval start, now; int final_ball; + tst_res(TINFO, "Starting referee thread"); + prctl(PR_SET_NAME, "referee", 0, 0, 0); - printf("Game On (%d seconds)!\n", game_length); + tst_res(TINFO, "Starting the game (%d sec)", game_length); /* open trace marker early to avoid latency with the first message */ trace_marker_prep(); @@ -178,7 +128,7 @@ int referee(int game_length) now = start; /* Start the game! */ - tst_atomic_store(0, &the_ball); + tst_atomic_store(0, &ball); atrace_marker_write("sched_football", "Game_started!"); /* Watch the game */ @@ -187,29 +137,26 @@ int referee(int game_length) gettimeofday(&now, NULL); } atrace_marker_write("sched_football", "Game_Over!"); - final_ball = tst_atomic_load(&the_ball); + final_ball = tst_atomic_load(&ball); + /* Blow the whistle */ - printf("Game Over!\n"); - printf("Final ball position: %d\n", final_ball); - return final_ball != 0; + tst_res(TINFO, "Final ball position: %d", final_ball); + + TST_EXP_EXPR(final_ball == 0); } -int main(int argc, char *argv[]) +static void do_test(void) { struct sched_param param; int priority; int i; - int result; - setup(); - - rt_init("n:l:h", parse_args, argc, argv); if (players_per_team == 0) players_per_team = sysconf(_SC_NPROCESSORS_ONLN); tst_atomic_store(0, &players_ready); - printf("Running with: players_per_team=%d game_length=%d\n", + tst_res(TINFO, "players_per_team: %d game_length: %d", players_per_team, game_length); /* We're the ref, so set our priority right */ @@ -221,7 +168,7 @@ int main(int argc, char *argv[]) * They are lower priority than defense, so they must be started first. */ priority = 15; - printf("Starting %d offense threads at priority %d\n", + tst_res(TINFO, "Starting %d offense threads at priority %d", players_per_team, priority); for (i = 0; i < players_per_team; i++) create_fifo_thread(thread_offense, NULL, priority); @@ -232,7 +179,7 @@ int main(int argc, char *argv[]) /* Start the defense */ priority = 30; - printf("Starting %d defense threads at priority %d\n", + tst_res(TINFO, "Starting %d defense threads at priority %d", players_per_team, priority); for (i = 0; i < players_per_team; i++) create_fifo_thread(thread_defense, NULL, priority); @@ -243,7 +190,7 @@ int main(int argc, char *argv[]) /* Start the crazy fans*/ priority = 50; - printf("Starting %d fan threads at priority %d\n", + tst_res(TINFO, "Starting %d fan threads at priority %d", players_per_team, priority); for (i = 0; i < players_per_team*2; i++) create_fifo_thread(thread_fan, NULL, priority); @@ -255,9 +202,26 @@ int main(int argc, char *argv[]) /* let things get into steady state */ sleep(2); /* Ok, everyone is on the field, bring out the ref */ - printf("Starting referee thread\n"); - result = referee(game_length); - printf("Result: %s\n", result ? "FAIL" : "PASS"); - return result; + referee(game_length); } + +static void do_setup(void) +{ + if (tst_parse_int(str_game_length, &game_length, 1, INT_MAX)) + tst_brk(TBROK, "Invalid game length '%s'", str_game_length); + + if (tst_parse_int(str_players_per_team, &players_per_team, 1, INT_MAX)) + tst_brk(TBROK, "Invalid number of players '%s'", str_players_per_team); +} + +static struct tst_test test = { + .test_all = do_test, + .setup = do_setup, + .options = (struct tst_option[]) { + {"l:", &str_game_length, "Game length in sec (default: " + TST_TO_STR(DEF_GAME_LENGTH) " sec)"}, + {"n:", &str_players_per_team, "Number of players (default: number of CPU)"}, + {} + }, +};
This is due test compilation broken on old gcc 4.8 we still support since 8fc3cf4ad6. Combining LTP librealtime (librttest.c) and LTP library is somehow experimental. -lltp was needed to be added to CFLAGS but yet on musl it fails to find the function on runtime: tst_test.c:985: TBROK: No test function specified librttest.c getopts were ignored, port just test specific -l and -n. Fixes: 8fc3cf4ad6 ("sched_football: Re-add the crazy fans to interrupt everyone") Signed-off-by: Petr Vorel <pvorel@suse.cz> --- .../realtime/func/sched_football/Makefile | 1 + .../func/sched_football/sched_football.c | 196 +++++++----------- 2 files changed, 81 insertions(+), 116 deletions(-)