linux-kernel - Re: [PATCH 1/4] Add clock_gettime

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: <CALCETrW+9OoEQnP1+9MJX+JO5Ch42QhUeo8i=k9YdaCwWhg=Xw@mail.gmail.com>
Date:	Wed, 28 Dec 2011 15:42:20 -0800
From:	Andy Lutomirski <luto@...capital.net>
To:	Arun Sharma <asharma@...com>
Cc:	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <peterz@...radead.org>,
	Kumar Sundararajan <kumar@...com>,
	linux-kernel@...r.kernel.org, john stultz <johnstul@...ibm.com>,
	Richard Cochran <richardcochran@...il.com>
Subject: Re: [PATCH 1/4] Add clock_gettime_ns syscall

On Wed, Dec 28, 2011 at 2:45 PM, Arun Sharma <asharma@...com> wrote:
> On Wed, Dec 28, 2011 at 12:13:37PM -0800, Andy Lutomirski wrote:
>> > How about returning a (signed) long as the time in ns? This way, we save
>> a store and a load and the value can be passed in registers.
>> >
>> > This shouldn't preclude future expansion via extra args.
>>
>> With an unconditional store to a pointer?  If a null pointer is allowed,
>> the branch will probably kill any performance gain.
>
> No - please see the patch below.
>
>>
>> The downside is that this is probably a non-starter for a syscall on 32-bit
>> architectures.  I'll see what the i386 ABI says.  I wonder if returning a
>> struct will use registers for future expansion.
>>
>
> I was thinking of doing something similar to lseek() on 32 bit archs
> (i.e. by using a type similar to off_t that maps to the right thing for
> both 32 and 64 bit).

Huh?  That nanosecond count really needs to be 64 bits (or more).
2^32 nanoseconds is a rather short time.

>
> I used the code below to benchmark the performance of clock_gettime()
> vs clock_gettime_ns() when the client is interested in a nanosec based
> interface.
>
> gettimespec: 3.19 secs
> getns: 2.54 secs (21% faster)
>
>  -Arun
>
> PS: I didn't have to delete struct timens. I meant to drop timens.ns
> (since its the return value now).

How are you keeping the ability to add extra fields?

FWIW, returning a long long on i386 generates nice code (two registers
used), but returning a struct bigger than 64 bits uses a pointer
passed in via the stack (i.e. worse than passing a pointer as a
parameter).  Returning a struct with two 64-bit ints on x86-64 uses
registers, which is rather unfortunate.

Something like:

u64 __vdso_clock_gettime_ns(clock_t clockid, struct timens_extra *extra)
{
  extra->padding = 0;
  return <the time>;
}


might be okay.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/