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