On Wed, Sep 25, 2024 at 8:20 AM Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> wrote: > > On 9/24/24 17:59, John Stultz wrote: > > On Tue, Sep 24, 2024 at 8:57 AM Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> 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
