| Message ID | 5148C2B3.6010408@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Michael Ellerman |
| Headers | show |
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.
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.
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 >
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 >
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
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 >
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.
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
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
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: *; };