Patchwork [RFC] powerpc: Add VDSO version of time

login
register
mail settings
Submitter Adhemerval Zanella
Date March 19, 2013, 7:55 p.m.
Message ID <5148C2B3.6010408@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/229217/
State Superseded, archived
Delegated to: Michael Ellerman
Headers show

Comments

Adhemerval Zanella - March 19, 2013, 7:55 p.m.
Hi all,

This patch implement the time syscall as vDSO. I have a glibc patch
to use it as IFUNC (as latest gettimeofday patch). Below the perf
numbers:

Baseline PPC32: 380 nsec
Baseline PPC64: 352 nsec
vdso PPC32:      20 nsec
vdso PPC64:      20 nsec

I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
as well?

Any tips, advices, comments?


--
Benjamin Herrenschmidt - March 20, 2013, 5 a.m.
On Tue, 2013-03-19 at 16:55 -0300, Adhemerval Zanella wrote:
> 
> I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
> as well?

You did provide both 32 and 64-bit VDSO implementations so 32-bit
kernels should be covered.

Cheers,
Ben.
Adhemerval Zanella - March 21, 2013, 1:40 p.m.
On 20-03-2013 02:00, Benjamin Herrenschmidt wrote:
> On Tue, 2013-03-19 at 16:55 -0300, Adhemerval Zanella wrote:
>> I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
>> as well?
> You did provide both 32 and 64-bit VDSO implementations so 32-bit
> kernels should be covered.

Indeed and thanks for the reply. Any objection or request about including it?

Thanks.
Adhemerval Zanella - March 26, 2013, 5:32 p.m.
Hi all,

Just sending a ping about this patch.

On 21-03-2013 10:40, Adhemerval Zanella wrote:
> On 20-03-2013 02:00, Benjamin Herrenschmidt wrote:
>> On Tue, 2013-03-19 at 16:55 -0300, Adhemerval Zanella wrote:
>>> I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
>>> as well?
>> You did provide both 32 and 64-bit VDSO implementations so 32-bit
>> kernels should be covered.
> Indeed and thanks for the reply. Any objection or request about including it?
>
> Thanks.
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Adhemerval Zanella - April 4, 2013, 7:22 p.m.
Hi Benjamin,

Any objection or request about this patch?


On 20-03-2013 02:00, Benjamin Herrenschmidt wrote:
> On Tue, 2013-03-19 at 16:55 -0300, Adhemerval Zanella wrote:
>> I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
>> as well?
> You did provide both 32 and 64-bit VDSO implementations so 32-bit
> kernels should be covered.
>
> Cheers,
> Ben.
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Michael Ellerman - April 5, 2013, 6:21 a.m.
On Tue, Mar 19, 2013 at 04:55:31PM -0300, Adhemerval Zanella wrote:
> Hi all,
> 
> This patch implement the time syscall as vDSO. I have a glibc patch
> to use it as IFUNC (as latest gettimeofday patch). Below the perf
> numbers:
> 
> Baseline PPC32: 380 nsec
> Baseline PPC64: 352 nsec
> vdso PPC32:      20 nsec
> vdso PPC64:      20 nsec
> 
> I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
> as well?

You did provide a 32-bit implementation. I take it you haven't tested
that though? Can you test it?

What happens if I don't have the glibc patch?

cheers
Adhemerval Zanella - April 5, 2013, 6:49 p.m.
On 04/05/2013 03:21 AM, Michael Ellerman wrote:
> On Tue, Mar 19, 2013 at 04:55:31PM -0300, Adhemerval Zanella wrote:
>> Hi all,
>>
>> This patch implement the time syscall as vDSO. I have a glibc patch
>> to use it as IFUNC (as latest gettimeofday patch). Below the perf
>> numbers:
>>
>> Baseline PPC32: 380 nsec
>> Baseline PPC64: 352 nsec
>> vdso PPC32:      20 nsec
>> vdso PPC64:      20 nsec
>>
>> I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
>> as well?
> You did provide a 32-bit implementation. I take it you haven't tested
> that though? Can you test it?
I haven't test it yet and I believe it won't be troublesome to do so.
>
> What happens if I don't have the glibc patch?
GLIBC current behavior is to use the syscall.

>
> cheers
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Adhemerval Zanella - April 8, 2013, 2:05 p.m.
On 04/05/2013 03:21 AM, Michael Ellerman wrote:
> On Tue, Mar 19, 2013 at 04:55:31PM -0300, Adhemerval Zanella wrote:
>> Hi all,
>>
>> This patch implement the time syscall as vDSO. I have a glibc patch
>> to use it as IFUNC (as latest gettimeofday patch). Below the perf
>> numbers:
>>
>> Baseline PPC32: 380 nsec
>> Baseline PPC64: 352 nsec
>> vdso PPC32:      20 nsec
>> vdso PPC64:      20 nsec
>>
>> I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
>> as well?
> You did provide a 32-bit implementation. I take it you haven't tested
> that though? Can you test it?

Hi,

I didn't build a 32 bit kernel, but I tested 32 bits binaries (that uses
the VDSO32 implantation) without any issue. Performance gains are similar.
Michael Ellerman - April 18, 2013, 5:58 a.m.
On Mon, Apr 08, 2013 at 11:05:50AM -0300, Adhemerval Zanella wrote:
> On 04/05/2013 03:21 AM, Michael Ellerman wrote:
> > On Tue, Mar 19, 2013 at 04:55:31PM -0300, Adhemerval Zanella wrote:
> >> Hi all,
> >>
> >> This patch implement the time syscall as vDSO. I have a glibc patch
> >> to use it as IFUNC (as latest gettimeofday patch). Below the perf
> >> numbers:
> >>
> >> Baseline PPC32: 380 nsec
> >> Baseline PPC64: 352 nsec
> >> vdso PPC32:      20 nsec
> >> vdso PPC64:      20 nsec
> >>
> >> I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
> >> as well?
> > You did provide a 32-bit implementation. I take it you haven't tested
> > that though? Can you test it?
> 
> Hi,
> 
> I didn't build a 32 bit kernel, but I tested 32 bits binaries (that uses
> the VDSO32 implantation) without any issue. Performance gains are similar.

OK. Please send an updated version of the patch which a changelog
describing the testing you've done, and add your Signed-off-by to the
patch.

cheers
Anton Blanchard - April 18, 2013, 10:38 p.m.
Hi Adhemerval,

> This patch implement the time syscall as vDSO. I have a glibc patch
> to use it as IFUNC (as latest gettimeofday patch). Below the perf
> numbers:
> 
> Baseline PPC32: 380 nsec
> Baseline PPC64: 352 nsec
> vdso PPC32:      20 nsec
> vdso PPC64:      20 nsec

Very nice speedup. One small performance improvement:

+1:	ld	r8,CFG_TB_UPDATE_COUNT(r3)
+	ld	r4,STAMP_XTIME+TSPC64_TV_SEC(r3)
+	andi.	r0,r8,1			/* pending update ? loop */
+	bne-	1b

Since you are only reading one long you shouldn't need to check the
update count and loop, you will always see a consistent value. The
system call version of time() just does an unprotected load for example.

> I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
> as well?
> 
> Any tips, advices, comments?

With the above change and with Michael's comments covered (decent
changelog entry and Signed-off-by):

Acked-by: Anton Blanchard <anton@samba.org>

Anton

Patch

diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index 1b2076f..d4f463a 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -113,6 +113,10 @@  static struct vdso_patch_def vdso_patches[] = {
 		CPU_FTR_USE_TB, 0,
 		"__kernel_get_tbfreq", NULL
 	},
+	{
+		CPU_FTR_USE_TB, 0,
+		"__kernel_time", NULL
+	},
 };
 
 /*
diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index 4ee09ee..9a60a87 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -181,6 +181,35 @@  V_FUNCTION_END(__kernel_clock_getres)
 
 
 /*
+ * Exact prototype of time()
+ *
+ * time_t time(time *t);
+ *
+ */
+V_FUNCTION_BEGIN(__kernel_time)
+  .cfi_startproc
+	mflr	r12
+  .cfi_register lr,r12
+
+	mr	r11,r3			/* r11 holds t */
+	bl	__get_datapage@local
+	mr	r9, r3			/* datapage ptr in r9 */
+
+1:	lwz	r8,(CFG_TB_UPDATE_COUNT+LOPART)(r9)
+	lwz	r3,STAMP_XTIME+TSPEC_TV_SEC(r9)
+	andi.	r0,r8,1			/* pending update ? loop */
+	bne-	1b
+
+	cmplwi	r11,0			/* check if t is NULL */
+	beq	2f
+	stw	r3,0(r11)		/* store result at *t */
+2:	mtlr	r12
+	crclr	cr0*4+so
+	blr
+  .cfi_endproc
+V_FUNCTION_END(__kernel_time)
+
+/*
  * This is the core of clock_gettime() and gettimeofday(),
  * it returns the current time in r3 (seconds) and r4.
  * On entry, r7 gives the resolution of r4, either USEC_PER_SEC
diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index 43200ba..f223409 100644
--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
@@ -150,6 +150,7 @@  VERSION
 #ifdef CONFIG_PPC64
 		__kernel_getcpu;
 #endif
+		__kernel_time;
 
 	local: *;
 	};
diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S b/arch/powerpc/kernel/vdso64/gettimeofday.S
index e97a9a0..f05aa68 100644
--- a/arch/powerpc/kernel/vdso64/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso64/gettimeofday.S
@@ -164,6 +164,35 @@  V_FUNCTION_BEGIN(__kernel_clock_getres)
   .cfi_endproc
 V_FUNCTION_END(__kernel_clock_getres)
 
+/*
+ * Exact prototype of time()
+ *
+ * time_t time(time *t);
+ *
+ */
+V_FUNCTION_BEGIN(__kernel_time)
+  .cfi_startproc
+	mflr	r12
+  .cfi_register lr,r12
+
+	mr	r11,r3			/* r11 holds t */
+	bl	V_LOCAL_FUNC(__get_datapage)
+
+1:	ld	r8,CFG_TB_UPDATE_COUNT(r3)
+	ld	r4,STAMP_XTIME+TSPC64_TV_SEC(r3)
+	andi.	r0,r8,1			/* pending update ? loop */
+	bne-	1b
+
+	cmpldi	r11,0			/* check if t is NULL */
+	beq	2f
+	std	r4,0(r11)		/* store result at *t */
+2:	mtlr	r12
+	crclr	cr0*4+so
+	mr	r3,r4
+	blr
+  .cfi_endproc
+V_FUNCTION_END(__kernel_time)
+
 
 /*
  * This is the core of clock_gettime() and gettimeofday(),
diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S b/arch/powerpc/kernel/vdso64/vdso64.lds.S
index e6c1758..e486381 100644
--- a/arch/powerpc/kernel/vdso64/vdso64.lds.S
+++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S
@@ -147,6 +147,7 @@  VERSION
 		__kernel_sync_dicache_p5;
 		__kernel_sigtramp_rt64;
 		__kernel_getcpu;
+		__kernel_time;
 
 	local: *;
 	};