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

From: Christophe Leroy
Date: Sun Oct 27 2019 - 05:21:49 EST



Le 27/10/2019 Ã 01:06, Segher Boessenkool a ÃcritÂ:
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.

Well, all this started with COARSE clocks not being supported by PPC32 VDSO. I first submitted a series with a set of optimisations including the implementation of COARSE clocks (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=126779)

Then after a comment received on patch 4 of the series from Santosh Sivaraj asking for a common implementation of it for PPC32 and PPC64, I started looking into making the whole VDSO source code common to PPC32 and PPC64. Most functions are similar. Time functions are also rather similar but unfortunately don't use the same registers. They also don't cover all possible clocks. And getres() is also buggy, see series https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=110321

So instead of reworking the existing time functions, I started investigating whether we could plug powerpc to the generic implementation. One drawback of PPC is that we need to setup an ASM trampoline to handle the SO bit as it can't be handled from C directly, can it ?

How critical are these functions ? Although we have a slight degration with the C implementation, they are still way faster than the corresponding syscall.

Another thing I was wondering, is it worth using the 64 bit timebase on PPC32 ? As far as I understand, the timebase is there to calculate a linear date update since last VDSO datapage update. How often is the VDSO datapage updated ? On the 885 clocked at 132Mhz, the timebase is at 8.25 Mhz, which means it needs more than 8 minutes to loop over 32 bits.

Christophe