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

Segher Boessenkool
2019-10-26T23:10:07Z

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