linux-kernel - Re: [PATCH 2/2] selftests: timers: Remove local NSEC_PER_SEC and USEC_PER

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b0647b67-5cba-4e42-9d42-d12ba2aa2174@linuxfoundation.org>
Date: Wed, 25 Sep 2024 09:20:41 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: John Stultz <jstultz@...gle.com>
Cc: tglx@...utronix.de, sboyd@...nel.org, shuah@...nel.org,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
 Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH 2/2] selftests: timers: Remove local NSEC_PER_SEC and
 USEC_PER_SEC defines

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