| Message ID | 20240627133456.11327-1-chrubis@suse.cz |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v2] include/tst_timer: Add TST_NO_LIBLTP | expand |
Hi, this is a lot of unnecessary code to fix a problem that didn't exist in the original sched_football patch. I'd prefer one of two other solutions: 1) go back to using ts_delta() from the original patch 2) fully port the test to new LTP API On 27. 06. 24 15:34, Cyril Hrubis wrote: > Older compilers (gcc-4.8) are not smart enough to eliminate the > impossible branch with tst_brk() early enough and the sched_football > compilation fails due to the unresolved function. > > Add TST_NO_LIBLTP macro that adds tst_brk_() inline implementaiont that > prints the message and calls abort() and make use of it in sched_football. > > Cc: kernel-team@android.com > Cc: Darren Hart <darren@os.amperecomputing.com> > Cc: John Stultz <jstultz@google.com> > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > --- > include/tst_test.h | 23 +++++++++++++++++++ > .../func/sched_football/sched_football.c | 1 + > 2 files changed, 24 insertions(+) > > diff --git a/include/tst_test.h b/include/tst_test.h > index 8dc20d110..9bd1b363d 100644 > --- a/include/tst_test.h > +++ b/include/tst_test.h > @@ -648,4 +648,27 @@ int main(int argc, char *argv[]) > #define TST_TEST_TCONF(message) \ > static struct tst_test test = { .tconf_msg = message } \ > > +/* > + * Fallback for cases where we do not want to link against ltp library. > + * > + * This allows us for instance to use tst_timer.h without LTP library. > + */ > +#ifdef TST_NO_LIBLTP > +__attribute__ ((format (printf, 4, 5))) > +inline void tst_brk_(const char *file, const int lineno, int ttype, > + const char *fmt, ...) > +{ > + va_list va; > + > + (void) ttype; > + > + fprintf(stderr, "%s: %i: ", file, lineno); > + va_start(va, fmt); > + vfprintf(stderr, fmt, va); > + va_end(va); > + > + abort(); > +} > +#endif > + > #endif /* TST_TEST_H__ */ > diff --git a/testcases/realtime/func/sched_football/sched_football.c b/testcases/realtime/func/sched_football/sched_football.c > index b6ae692af..6846978f4 100644 > --- a/testcases/realtime/func/sched_football/sched_football.c > +++ b/testcases/realtime/func/sched_football/sched_football.c > @@ -74,6 +74,7 @@ > #include <librttest.h> > #include <tst_atomic.h> > #define TST_NO_DEFAULT_MAIN > +#define TST_NO_LIBLTP > #include <tst_timer.h> > >
Hi all, > Hi, > this is a lot of unnecessary code to fix a problem that didn't exist in the > original sched_football patch. I'd prefer one of two other solutions: > 1) go back to using ts_delta() from the original patch > 2) fully port the test to new LTP API Sounds reasonable. FYI back at the time I asked Mel Gorman to investigate which tests would be relevant and worth to move from LTP into rt-tests. In his reply, which I put into https://github.com/linux-test-project/ltp/issues/1078, he considered sched_football as interesting, but requiring to reimplement. Therefore I would prefer to minimise changes (e.g. using ts_delta) to not complicate moving test to rt-tests in the future. Kind regards, Petr > On 27. 06. 24 15:34, Cyril Hrubis wrote: > > Older compilers (gcc-4.8) are not smart enough to eliminate the > > impossible branch with tst_brk() early enough and the sched_football > > compilation fails due to the unresolved function. > > Add TST_NO_LIBLTP macro that adds tst_brk_() inline implementaiont that > > prints the message and calls abort() and make use of it in sched_football. > > Cc: kernel-team@android.com > > Cc: Darren Hart <darren@os.amperecomputing.com> > > Cc: John Stultz <jstultz@google.com> > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > > --- > > include/tst_test.h | 23 +++++++++++++++++++ > > .../func/sched_football/sched_football.c | 1 + > > 2 files changed, 24 insertions(+) > > diff --git a/include/tst_test.h b/include/tst_test.h > > index 8dc20d110..9bd1b363d 100644 > > --- a/include/tst_test.h > > +++ b/include/tst_test.h > > @@ -648,4 +648,27 @@ int main(int argc, char *argv[]) > > #define TST_TEST_TCONF(message) \ > > static struct tst_test test = { .tconf_msg = message } \ > > +/* > > + * Fallback for cases where we do not want to link against ltp library. > > + * > > + * This allows us for instance to use tst_timer.h without LTP library. > > + */ > > +#ifdef TST_NO_LIBLTP > > +__attribute__ ((format (printf, 4, 5))) > > +inline void tst_brk_(const char *file, const int lineno, int ttype, > > + const char *fmt, ...) > > +{ > > + va_list va; > > + > > + (void) ttype; > > + > > + fprintf(stderr, "%s: %i: ", file, lineno); > > + va_start(va, fmt); > > + vfprintf(stderr, fmt, va); > > + va_end(va); > > + > > + abort(); > > +} > > +#endif > > + > > #endif /* TST_TEST_H__ */ > > diff --git a/testcases/realtime/func/sched_football/sched_football.c b/testcases/realtime/func/sched_football/sched_football.c > > index b6ae692af..6846978f4 100644 > > --- a/testcases/realtime/func/sched_football/sched_football.c > > +++ b/testcases/realtime/func/sched_football/sched_football.c > > @@ -74,6 +74,7 @@ > > #include <librttest.h> > > #include <tst_atomic.h> > > #define TST_NO_DEFAULT_MAIN > > +#define TST_NO_LIBLTP > > #include <tst_timer.h>
diff --git a/include/tst_test.h b/include/tst_test.h index 8dc20d110..9bd1b363d 100644 --- a/include/tst_test.h +++ b/include/tst_test.h @@ -648,4 +648,27 @@ int main(int argc, char *argv[]) #define TST_TEST_TCONF(message) \ static struct tst_test test = { .tconf_msg = message } \ +/* + * Fallback for cases where we do not want to link against ltp library. + * + * This allows us for instance to use tst_timer.h without LTP library. + */ +#ifdef TST_NO_LIBLTP +__attribute__ ((format (printf, 4, 5))) +inline void tst_brk_(const char *file, const int lineno, int ttype, + const char *fmt, ...) +{ + va_list va; + + (void) ttype; + + fprintf(stderr, "%s: %i: ", file, lineno); + va_start(va, fmt); + vfprintf(stderr, fmt, va); + va_end(va); + + abort(); +} +#endif + #endif /* TST_TEST_H__ */ diff --git a/testcases/realtime/func/sched_football/sched_football.c b/testcases/realtime/func/sched_football/sched_football.c index b6ae692af..6846978f4 100644 --- a/testcases/realtime/func/sched_football/sched_football.c +++ b/testcases/realtime/func/sched_football/sched_football.c @@ -74,6 +74,7 @@ #include <librttest.h> #include <tst_atomic.h> #define TST_NO_DEFAULT_MAIN +#define TST_NO_LIBLTP #include <tst_timer.h>
Older compilers (gcc-4.8) are not smart enough to eliminate the impossible branch with tst_brk() early enough and the sched_football compilation fails due to the unresolved function. Add TST_NO_LIBLTP macro that adds tst_brk_() inline implementaiont that prints the message and calls abort() and make use of it in sched_football. Cc: kernel-team@android.com Cc: Darren Hart <darren@os.amperecomputing.com> Cc: John Stultz <jstultz@google.com> Signed-off-by: Cyril Hrubis <chrubis@suse.cz> --- include/tst_test.h | 23 +++++++++++++++++++ .../func/sched_football/sched_football.c | 1 + 2 files changed, 24 insertions(+)