| Message ID | 1464523432-12605-2-git-send-email-anton@ozlabs.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Sun, 2016-05-29 at 22:03 +1000, Anton Blanchard wrote: > From: Anton Blanchard <anton@samba.org> > > In setup_sigcontext(), we set current->thread.vrsave then use it > straight after. Since current is hidden from the compiler via inline > assembly, it cannot optimise this and we end up with a load hit store. Is setup_sigcontext() really a fast path we need to optimise? Mikey > Fix this by using a temporary. > > Signed-off-by: Anton Blanchard <anton@samba.org> > --- > arch/powerpc/kernel/signal_64.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/kernel/signal_64.c > b/arch/powerpc/kernel/signal_64.c > index 2552079..7e49984 100644 > --- a/arch/powerpc/kernel/signal_64.c > +++ b/arch/powerpc/kernel/signal_64.c > @@ -104,6 +104,7 @@ static long setup_sigcontext(struct sigcontext __user > *sc, struct pt_regs *regs, > */ > #ifdef CONFIG_ALTIVEC > elf_vrreg_t __user *v_regs = sigcontext_vmx_regs(sc); > + unsigned long vrsave; > #endif > unsigned long msr = regs->msr; > long err = 0; > @@ -125,9 +126,13 @@ static long setup_sigcontext(struct sigcontext > __user *sc, struct pt_regs *regs, > /* We always copy to/from vrsave, it's 0 if we don't have or > don't > * use altivec. > */ > - if (cpu_has_feature(CPU_FTR_ALTIVEC)) > - current->thread.vrsave = mfspr(SPRN_VRSAVE); > - err |= __put_user(current->thread.vrsave, (u32 __user > *)&v_regs[33]); > + vrsave = 0; > + if (cpu_has_feature(CPU_FTR_ALTIVEC)) { > + vrsave = mfspr(SPRN_VRSAVE); > + current->thread.vrsave = vrsave; > + } > + > + err |= __put_user(vrsave, (u32 __user *)&v_regs[33]); > #else /* CONFIG_ALTIVEC */ > err |= __put_user(0, &sc->v_regs); > #endif /* CONFIG_ALTIVEC */
Hi, > On Sun, 2016-05-29 at 22:03 +1000, Anton Blanchard wrote: > > From: Anton Blanchard <anton@samba.org> > > > > In setup_sigcontext(), we set current->thread.vrsave then use it > > straight after. Since current is hidden from the compiler via inline > > assembly, it cannot optimise this and we end up with a load hit > > store. > > Is setup_sigcontext() really a fast path we need to optimise? setup_sigcontext() is called for every signal we deliver. I'd like userspace to use less signals, but we still stumble across workloads with quite heavy use of them. Anton
On Sun, 2016-29-05 at 12:03:51 UTC, Anton Blanchard wrote: > From: Anton Blanchard <anton@samba.org> > > In setup_sigcontext(), we set current->thread.vrsave then use it > straight after. Since current is hidden from the compiler via inline > assembly, it cannot optimise this and we end up with a load hit store. > > Fix this by using a temporary. > > Signed-off-by: Anton Blanchard <anton@samba.org> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/d96f234f47aff593538f9e3d67 cheers
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c index 2552079..7e49984 100644 --- a/arch/powerpc/kernel/signal_64.c +++ b/arch/powerpc/kernel/signal_64.c @@ -104,6 +104,7 @@ static long setup_sigcontext(struct sigcontext __user *sc, struct pt_regs *regs, */ #ifdef CONFIG_ALTIVEC elf_vrreg_t __user *v_regs = sigcontext_vmx_regs(sc); + unsigned long vrsave; #endif unsigned long msr = regs->msr; long err = 0; @@ -125,9 +126,13 @@ static long setup_sigcontext(struct sigcontext __user *sc, struct pt_regs *regs, /* We always copy to/from vrsave, it's 0 if we don't have or don't * use altivec. */ - if (cpu_has_feature(CPU_FTR_ALTIVEC)) - current->thread.vrsave = mfspr(SPRN_VRSAVE); - err |= __put_user(current->thread.vrsave, (u32 __user *)&v_regs[33]); + vrsave = 0; + if (cpu_has_feature(CPU_FTR_ALTIVEC)) { + vrsave = mfspr(SPRN_VRSAVE); + current->thread.vrsave = vrsave; + } + + err |= __put_user(vrsave, (u32 __user *)&v_regs[33]); #else /* CONFIG_ALTIVEC */ err |= __put_user(0, &sc->v_regs); #endif /* CONFIG_ALTIVEC */