Subject: Re: [RFC PATCH] powerpc/32: Switch VDSO to C implementation.

From: Segher Boessenkool <[email-masked]>
Date: Sat, 26 Oct 2019 18:06:09 -0500

On Sat, Oct 26, 2019 at 08:48:27PM +0200, Thomas Gleixner wrote:
> On Sat, 26 Oct 2019, Christophe Leroy wrote:
> Let's look at the code:
> 
> __cvdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz)
> {
>         const struct vdso_data *vd = __arch_get_vdso_data();
> 
>         if (likely(tv != NULL)) {
> 		struct __kernel_timespec ts;
> 
>                 if (do_hres(&vd[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
>                         return gettimeofday_fallback(tv, tz);
> 
>                 tv->tv_sec = ts.tv_sec;
>                 tv->tv_usec = (u32)ts.tv_nsec / NSEC_PER_USEC;
> 
> IIRC PPC did some magic math tricks to avoid that. Could you just for the
> fun of it replace this division with
> 
>        (u32)ts.tv_nsec >> 10;

On this particular CPU (the 885, right?) a division by 1000 is just 9
cycles.  On other CPUs it can be more, say 19 cycles like on the 750; not
cheap at all, but not hugely expensive either, comparatively.

(A 64/32->32 division is expensive on all 32-bit PowerPC: there is no
hardware help for it at all, so it's all done in software.)

Of course the compiler won't do a division by a constant with a division
instruction at all, so it's somewhat cheaper even, 5 or 6 cycles or so.

> One thing which might be worth to try as well is to mark all functions in
> that file as inline. The speedup by the do_hres() inlining was impressive
> on PPC.

The hand-optimised asm code will pretty likely win handsomely, whatever
you do.  Especially on cores like the 885 (no branch prediction, single
issue, small caches, etc.: every instruction counts).

Is there any reason to replace this hand-optimised code?  It was written
for exacty this reason?  These functions are critical and should be as
fast as possible.

Segher