From: Michael Neuling <mikey@neuling•org>
To: Michael Ellerman <michael@ellerman•id.au>,
benh <benh@kernel•crashing.org>
Cc: linuxppc-dev <linuxppc-dev@lists•ozlabs.org>,
Anton Blanchard <anton@samba•org>,
Aaron Sawdey <sawdey@us•ibm.com>,
Steve Munroe <sjmunroe@us•ibm.com>
Subject: [PATCH] powerpc/vdso: Avoid link stack corruption in __get_datapage()
Date: Wed, 23 Sep 2015 16:05:02 +1000 [thread overview]
Message-ID: <1442988302.31273.81.camel@neuling.org> (raw)
The 32 and 64 bit variants of __get_datapage() use a "bcl; mflr" to
determine the loaded address of the VDSO. The current version of these
attempt to use the special bcl variant which avoids pushing to the
link stack.
Unfortunately it uses bcl+8 rather than the required bcl+4. Hence the
current code results in link stack corruption and the resulting
performance degradation (due to branch mis-prediction).
This patch moves us to bcl+4 by moving __kernel_datapage_offset
out of __get_datapage().
With this patch, running the below benchmark we get a bump in
performance on POWER8 for gettimeofday() (which uses
__get_datapage()).
64bit gets ~4% improvement:
Without patch:
# ./tb
time =3D 0.180321
With patch:
# ./tb
time =3D 0.187408
32bit gets ~9% improvement:
Without patch:
# ./tb
time =3D 0.276551
With patch:
# ./tb
time =3D 0.252767
Testcase tb.c (stolen from Anton)
/* gcc -O2 tb.c -o tb */
#include <sys/time.h>
#include <stdio.h>
int main()
{
int i;
struct timeval tv_start, tv_end;
gettimeofday(&tv_start, NULL);
for(i =3D 0; i < 10000000; i++) {
gettimeofday(&tv_end, NULL);
}
printf("time =3D %.6f\n", tv_end.tv_sec - tv_start.tv_sec + (tv_end.tv_u=
sec - tv_start.tv_usec) * 1e-6);
return 0;
}
Signed-off-by: Michael Neuling <mikey@neuling•org>
Reported-by: Aaron Sawdey <sawdey@us•ibm.com>
diff --git a/arch/powerpc/kernel/vdso32/datapage.S b/arch/powerpc/kernel/vd=
so32/datapage.S
index dc21e89..59cf5f4 100644
--- a/arch/powerpc/kernel/vdso32/datapage.S
+++ b/arch/powerpc/kernel/vdso32/datapage.S
@@ -16,6 +16,10 @@
#include <asm/vdso.h>
=20
.text
+ .global __kernel_datapage_offset;
+__kernel_datapage_offset:
+ .long 0
+
V_FUNCTION_BEGIN(__get_datapage)
.cfi_startproc
/* We don't want that exposed or overridable as we want other objects
@@ -27,13 +31,11 @@ V_FUNCTION_BEGIN(__get_datapage)
mflr r0
.cfi_register lr,r0
=20
- bcl 20,31,1f
- .global __kernel_datapage_offset;
-__kernel_datapage_offset:
- .long 0
-1:
+ bcl 20,31,data_page_branch
+data_page_branch:
mflr r3
mtlr r0
+ addi r3, r3, __kernel_datapage_offset-data_page_branch
lwz r0,0(r3)
add r3,r0,r3
blr
diff --git a/arch/powerpc/kernel/vdso64/datapage.S b/arch/powerpc/kernel/vd=
so64/datapage.S
index 79796de..2f01c4a 100644
--- a/arch/powerpc/kernel/vdso64/datapage.S
+++ b/arch/powerpc/kernel/vdso64/datapage.S
@@ -16,6 +16,10 @@
#include <asm/vdso.h>
=20
.text
+.global __kernel_datapage_offset;
+__kernel_datapage_offset:
+ .long 0
+
V_FUNCTION_BEGIN(__get_datapage)
.cfi_startproc
/* We don't want that exposed or overridable as we want other objects
@@ -27,13 +31,11 @@ V_FUNCTION_BEGIN(__get_datapage)
mflr r0
.cfi_register lr,r0
=20
- bcl 20,31,1f
- .global __kernel_datapage_offset;
-__kernel_datapage_offset:
- .long 0
-1:
+ bcl 20,31,data_page_branch
+data_page_branch:
mflr r3
mtlr r0
+ addi r3, r3, __kernel_datapage_offset-data_page_branch
lwz r0,0(r3)
add r3,r0,r3
blr
next reply other threads:[~2015-09-23 6:05 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-23 6:05 Michael Neuling [this message]
2015-09-23 18:21 ` [PATCH] powerpc/vdso: Avoid link stack corruption in __get_datapage() Denis Kirjanov
2015-09-23 18:35 ` Aaron Sawdey
[not found] ` <201509231835.t8NIZr0Q029029@d01av04.pok.ibm.com>
2015-09-23 23:28 ` Michael Neuling
2015-09-23 23:33 ` Michael Neuling
2015-09-23 22:23 ` Michael Ellerman
2015-09-24 10:10 ` Denis Kirjanov
2015-09-25 0:05 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1442988302.31273.81.camel@neuling.org \
--to=mikey@neuling$(echo .)org \
--cc=anton@samba$(echo .)org \
--cc=benh@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=michael@ellerman$(echo .)id.au \
--cc=sawdey@us$(echo .)ibm.com \
--cc=sjmunroe@us$(echo .)ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox