| Message ID | f797bc704d8eb03234a3447c77fa7b704339f89a.1727191485.git.skhan@linuxfoundation.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | timers test fix and duplicate defines cleanup | expand |
On Tue, Sep 24, 2024 at 8:57 AM Shuah Khan <skhan@linuxfoundation.org> wrote: > > Remove local NSEC_PER_SEC and USEC_PER_SEC defines. Pick them up from > include/vdso/time64.h. This requires -I $(top_srcdir) to the timers > Makefile to include the include/vdso/time64.h. > > posix_timers test names the defines NSECS_PER_SEC and USECS_PER_SEC. > Include the include/vdso/time64.h and change NSECS_PER_SEC and > USECS_PER_SEC references to NSEC_PER_SEC and USEC_PER_SEC respectively. Nit: You got the last bit switched there. This patch changes local NSEC_PER_SEC to the upstream defined NSECS_PER_SEC. Overall no objection from me. I've always pushed to have the tests be mostly self-contained so they can be built outside of the kernel source, but at this point the current kselftest.h dependencies means it already takes some work, so this isn't introducing an undue hardship. Other then the nit, Acked-by: John Stultz <jstultz@google.com> > diff --git a/tools/testing/selftests/timers/adjtick.c b/tools/testing/selftests/timers/adjtick.c > index 205b76a4abb4..cb9a30f54662 100644 > --- a/tools/testing/selftests/timers/adjtick.c > +++ b/tools/testing/selftests/timers/adjtick.c > @@ -22,14 +22,12 @@ > #include <sys/time.h> > #include <sys/timex.h> > #include <time.h> > +#include <include/vdso/time64.h> > > #include "../kselftest.h" > > #define CLOCK_MONOTONIC_RAW 4 I suspect CLOCK_MONOTONIC_RAW (and the other clockid definitions) could be similarly removed here as well in a future patch? thanks -john
On 9/24/24 17:59, John Stultz wrote: > On Tue, Sep 24, 2024 at 8:57 AM Shuah Khan <skhan@linuxfoundation.org> wrote: >> >> Remove local NSEC_PER_SEC and USEC_PER_SEC defines. Pick them up from >> include/vdso/time64.h. This requires -I $(top_srcdir) to the timers >> Makefile to include the include/vdso/time64.h. >> >> posix_timers test names the defines NSECS_PER_SEC and USECS_PER_SEC. >> Include the include/vdso/time64.h and change NSECS_PER_SEC and >> USECS_PER_SEC references to NSEC_PER_SEC and USEC_PER_SEC respectively. > > Nit: You got the last bit switched there. This patch changes local > NSEC_PER_SEC to the upstream defined NSECS_PER_SEC. I think what IO have is correct. posix_timers defines them as NSECS_PER_SEC and USECS_PER_SEC and the header file doesn't have the extra S. It could use rephrasing thought to make it clear. Is it okay to fix this when I apply the patch or would you like me to send v2? > > Overall no objection from me. I've always pushed to have the tests be > mostly self-contained so they can be built outside of the kernel > source, but at this point the current kselftest.h dependencies means > it already takes some work, so this isn't introducing an undue > hardship. > Yes. At this point it would be hard to build it outside. DO you think these defines can be part of uapi? > Other then the nit, > Acked-by: John Stultz <jstultz@google.com> > >> diff --git a/tools/testing/selftests/timers/adjtick.c b/tools/testing/selftests/timers/adjtick.c >> index 205b76a4abb4..cb9a30f54662 100644 >> --- a/tools/testing/selftests/timers/adjtick.c >> +++ b/tools/testing/selftests/timers/adjtick.c >> @@ -22,14 +22,12 @@ >> #include <sys/time.h> >> #include <sys/timex.h> >> #include <time.h> >> +#include <include/vdso/time64.h> >> >> #include "../kselftest.h" >> >> #define CLOCK_MONOTONIC_RAW 4 > > I suspect CLOCK_MONOTONIC_RAW (and the other clockid definitions) > could be similarly removed here as well in a future patch? Yes. It could be cleaned up. I will send a patch in for this. thanks, -- Shuah
On Wed, Sep 25, 2024 at 8:20 AM Shuah Khan <skhan@linuxfoundation.org> wrote: > > On 9/24/24 17:59, John Stultz wrote: > > On Tue, Sep 24, 2024 at 8:57 AM Shuah Khan <skhan@linuxfoundation.org> wrote: > >> > >> Remove local NSEC_PER_SEC and USEC_PER_SEC defines. Pick them up from > >> include/vdso/time64.h. This requires -I $(top_srcdir) to the timers > >> Makefile to include the include/vdso/time64.h. > >> > >> posix_timers test names the defines NSECS_PER_SEC and USECS_PER_SEC. > >> Include the include/vdso/time64.h and change NSECS_PER_SEC and > >> USECS_PER_SEC references to NSEC_PER_SEC and USEC_PER_SEC respectively. > > > > Nit: You got the last bit switched there. This patch changes local > > NSEC_PER_SEC to the upstream defined NSECS_PER_SEC. > > I think what IO have is correct. posix_timers defines them as NSECS_PER_SEC > and USECS_PER_SEC and the header file doesn't have the extra S. It could > use rephrasing thought to make it clear. ? But the patch is removing the local non-plural NSEC_PER_SEC usage in posix_timers? -#define NSEC_PER_SEC 1000000000LL -#define USEC_PER_SEC 1000000 - As the headers do have the values with the extra S. So I'm confused (sort of my natural state :), but this is a minor nit, so apologies and just ignore me if I'm really getting it backwards here. > Is it okay to fix this when I apply the patch or would you like me to send v2? > I don't need a v2 > > > > Overall no objection from me. I've always pushed to have the tests be > > mostly self-contained so they can be built outside of the kernel > > source, but at this point the current kselftest.h dependencies means > > it already takes some work, so this isn't introducing an undue > > hardship. > > > > Yes. At this point it would be hard to build it outside. DO you think > these defines can be part of uapi? Maybe, though they are so common I fret it would cause folks trouble with redefinition collisions. Thanks for doing this cleanup! -john
diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile index 0e73a16874c4..32203593c62e 100644 --- a/tools/testing/selftests/timers/Makefile +++ b/tools/testing/selftests/timers/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -CFLAGS += -O3 -Wl,-no-as-needed -Wall +CFLAGS += -O3 -Wl,-no-as-needed -Wall -I $(top_srcdir) LDLIBS += -lrt -lpthread -lm # these are all "safe" tests that don't modify diff --git a/tools/testing/selftests/timers/adjtick.c b/tools/testing/selftests/timers/adjtick.c index 205b76a4abb4..cb9a30f54662 100644 --- a/tools/testing/selftests/timers/adjtick.c +++ b/tools/testing/selftests/timers/adjtick.c @@ -22,14 +22,12 @@ #include <sys/time.h> #include <sys/timex.h> #include <time.h> +#include <include/vdso/time64.h> #include "../kselftest.h" #define CLOCK_MONOTONIC_RAW 4 -#define NSEC_PER_SEC 1000000000LL -#define USEC_PER_SEC 1000000 - #define MILLION 1000000 long systick; diff --git a/tools/testing/selftests/timers/alarmtimer-suspend.c b/tools/testing/selftests/timers/alarmtimer-suspend.c index ad52e608b88e..62da2a3f949e 100644 --- a/tools/testing/selftests/timers/alarmtimer-suspend.c +++ b/tools/testing/selftests/timers/alarmtimer-suspend.c @@ -28,6 +28,7 @@ #include <signal.h> #include <stdlib.h> #include <pthread.h> +#include <include/vdso/time64.h> #include "../kselftest.h" #define CLOCK_REALTIME 0 @@ -45,7 +46,6 @@ #define NR_CLOCKIDS 12 -#define NSEC_PER_SEC 1000000000ULL #define UNREASONABLE_LAT (NSEC_PER_SEC * 5) /* hopefully we resume in 5 secs */ #define SUSPEND_SECS 15 diff --git a/tools/testing/selftests/timers/inconsistency-check.c b/tools/testing/selftests/timers/inconsistency-check.c index 36a49fba6c9b..75650cf0503f 100644 --- a/tools/testing/selftests/timers/inconsistency-check.c +++ b/tools/testing/selftests/timers/inconsistency-check.c @@ -28,10 +28,10 @@ #include <sys/timex.h> #include <string.h> #include <signal.h> +#include <include/vdso/time64.h> #include "../kselftest.h" #define CALLS_PER_LOOP 64 -#define NSEC_PER_SEC 1000000000ULL #define CLOCK_REALTIME 0 #define CLOCK_MONOTONIC 1 diff --git a/tools/testing/selftests/timers/leap-a-day.c b/tools/testing/selftests/timers/leap-a-day.c index 986abbdb1521..04004a7c0934 100644 --- a/tools/testing/selftests/timers/leap-a-day.c +++ b/tools/testing/selftests/timers/leap-a-day.c @@ -48,9 +48,9 @@ #include <string.h> #include <signal.h> #include <unistd.h> +#include <include/vdso/time64.h> #include "../kselftest.h" -#define NSEC_PER_SEC 1000000000ULL #define CLOCK_TAI 11 time_t next_leap; diff --git a/tools/testing/selftests/timers/mqueue-lat.c b/tools/testing/selftests/timers/mqueue-lat.c index f3179a605bba..63de2334a291 100644 --- a/tools/testing/selftests/timers/mqueue-lat.c +++ b/tools/testing/selftests/timers/mqueue-lat.c @@ -29,9 +29,9 @@ #include <signal.h> #include <errno.h> #include <mqueue.h> +#include <include/vdso/time64.h> #include "../kselftest.h" -#define NSEC_PER_SEC 1000000000ULL #define TARGET_TIMEOUT 100000000 /* 100ms in nanoseconds */ #define UNRESONABLE_LATENCY 40000000 /* 40ms in nanosecs */ diff --git a/tools/testing/selftests/timers/nanosleep.c b/tools/testing/selftests/timers/nanosleep.c index df1d03516e7b..9a354e38a569 100644 --- a/tools/testing/selftests/timers/nanosleep.c +++ b/tools/testing/selftests/timers/nanosleep.c @@ -27,10 +27,9 @@ #include <sys/timex.h> #include <string.h> #include <signal.h> +#include <include/vdso/time64.h> #include "../kselftest.h" -#define NSEC_PER_SEC 1000000000ULL - #define CLOCK_REALTIME 0 #define CLOCK_MONOTONIC 1 #define CLOCK_PROCESS_CPUTIME_ID 2 diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c index ddb1cebc844e..9814b3a1c77d 100644 --- a/tools/testing/selftests/timers/posix_timers.c +++ b/tools/testing/selftests/timers/posix_timers.c @@ -15,13 +15,12 @@ #include <string.h> #include <unistd.h> #include <time.h> +#include <include/vdso/time64.h> #include <pthread.h> #include "../kselftest.h" #define DELAY 2 -#define USECS_PER_SEC 1000000 -#define NSECS_PER_SEC 1000000000 static void __fatal_error(const char *test, const char *name, const char *what) { @@ -86,9 +85,9 @@ static int check_diff(struct timeval start, struct timeval end) long long diff; diff = end.tv_usec - start.tv_usec; - diff += (end.tv_sec - start.tv_sec) * USECS_PER_SEC; + diff += (end.tv_sec - start.tv_sec) * USEC_PER_SEC; - if (llabs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) { + if (llabs(diff - DELAY * USEC_PER_SEC) > USEC_PER_SEC / 2) { printf("Diff too high: %lld..", diff); return -1; } @@ -448,7 +447,7 @@ static inline int64_t calcdiff_ns(struct timespec t1, struct timespec t2) { int64_t diff; - diff = NSECS_PER_SEC * (int64_t)((int) t1.tv_sec - (int) t2.tv_sec); + diff = NSEC_PER_SEC * (int64_t)((int) t1.tv_sec - (int) t2.tv_sec); diff += ((int) t1.tv_nsec - (int) t2.tv_nsec); return diff; } @@ -479,7 +478,7 @@ static void check_sigev_none(int which, const char *name) do { if (clock_gettime(which, &now)) fatal_error(name, "clock_gettime()"); - } while (calcdiff_ns(now, start) < NSECS_PER_SEC); + } while (calcdiff_ns(now, start) < NSEC_PER_SEC); if (timer_gettime(timerid, &its)) fatal_error(name, "timer_gettime()"); @@ -536,7 +535,7 @@ static void check_gettime(int which, const char *name) wraps++; prev = its; - } while (calcdiff_ns(now, start) < NSECS_PER_SEC); + } while (calcdiff_ns(now, start) < NSEC_PER_SEC); if (timer_delete(timerid)) fatal_error(name, "timer_delete()"); @@ -587,7 +586,7 @@ static void check_overrun(int which, const char *name) do { if (clock_gettime(which, &now)) fatal_error(name, "clock_gettime()"); - } while (calcdiff_ns(now, start) < NSECS_PER_SEC); + } while (calcdiff_ns(now, start) < NSEC_PER_SEC); /* Unblock it, which should deliver a signal */ if (sigprocmask(SIG_UNBLOCK, &set, NULL)) diff --git a/tools/testing/selftests/timers/raw_skew.c b/tools/testing/selftests/timers/raw_skew.c index 030143eb09b4..ea50e4efc422 100644 --- a/tools/testing/selftests/timers/raw_skew.c +++ b/tools/testing/selftests/timers/raw_skew.c @@ -25,10 +25,10 @@ #include <sys/time.h> #include <sys/timex.h> #include <time.h> +#include <include/vdso/time64.h> #include "../kselftest.h" #define CLOCK_MONOTONIC_RAW 4 -#define NSEC_PER_SEC 1000000000LL #define shift_right(x, s) ({ \ __typeof__(x) __x = (x); \ diff --git a/tools/testing/selftests/timers/set-2038.c b/tools/testing/selftests/timers/set-2038.c index f7d978721b9e..ed244315e11c 100644 --- a/tools/testing/selftests/timers/set-2038.c +++ b/tools/testing/selftests/timers/set-2038.c @@ -27,10 +27,9 @@ #include <unistd.h> #include <time.h> #include <sys/time.h> +#include <include/vdso/time64.h> #include "../kselftest.h" -#define NSEC_PER_SEC 1000000000LL - #define KTIME_MAX ((long long)~((unsigned long long)1 << 63)) #define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) diff --git a/tools/testing/selftests/timers/set-timer-lat.c b/tools/testing/selftests/timers/set-timer-lat.c index 7ce240c89b21..5365e9ae61c3 100644 --- a/tools/testing/selftests/timers/set-timer-lat.c +++ b/tools/testing/selftests/timers/set-timer-lat.c @@ -28,6 +28,7 @@ #include <signal.h> #include <stdlib.h> #include <pthread.h> +#include <include/vdso/time64.h> #include "../kselftest.h" #define CLOCK_REALTIME 0 @@ -44,8 +45,6 @@ #define CLOCK_TAI 11 #define NR_CLOCKIDS 12 - -#define NSEC_PER_SEC 1000000000ULL #define UNRESONABLE_LATENCY 40000000 /* 40ms in nanosecs */ #define TIMER_SECS 1 diff --git a/tools/testing/selftests/timers/valid-adjtimex.c b/tools/testing/selftests/timers/valid-adjtimex.c index d500884801d8..6b7801055ad1 100644 --- a/tools/testing/selftests/timers/valid-adjtimex.c +++ b/tools/testing/selftests/timers/valid-adjtimex.c @@ -29,11 +29,9 @@ #include <string.h> #include <signal.h> #include <unistd.h> +#include <include/vdso/time64.h> #include "../kselftest.h" -#define NSEC_PER_SEC 1000000000LL -#define USEC_PER_SEC 1000000LL - #define ADJ_SETOFFSET 0x0100 #include <sys/syscall.h>
Remove local NSEC_PER_SEC and USEC_PER_SEC defines. Pick them up from include/vdso/time64.h. This requires -I $(top_srcdir) to the timers Makefile to include the include/vdso/time64.h. posix_timers test names the defines NSECS_PER_SEC and USECS_PER_SEC. Include the include/vdso/time64.h and change NSECS_PER_SEC and USECS_PER_SEC references to NSEC_PER_SEC and USEC_PER_SEC respectively. Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> --- tools/testing/selftests/timers/Makefile | 2 +- tools/testing/selftests/timers/adjtick.c | 4 +--- .../testing/selftests/timers/alarmtimer-suspend.c | 2 +- .../selftests/timers/inconsistency-check.c | 2 +- tools/testing/selftests/timers/leap-a-day.c | 2 +- tools/testing/selftests/timers/mqueue-lat.c | 2 +- tools/testing/selftests/timers/nanosleep.c | 3 +-- tools/testing/selftests/timers/posix_timers.c | 15 +++++++-------- tools/testing/selftests/timers/raw_skew.c | 2 +- tools/testing/selftests/timers/set-2038.c | 3 +-- tools/testing/selftests/timers/set-timer-lat.c | 3 +-- tools/testing/selftests/timers/valid-adjtimex.c | 4 +--- 12 files changed, 18 insertions(+), 26 deletions(-)