Thread (16 messages) 16 messages, 3 authors, 2016-02-10

Re: [PATCH v3 4/9] powerpc: Explicitly disable math features when copying thread

From: Balbir Singh <bsingharora@gmail.com>
Date: 2016-01-27 12:02:03

On Wed, Jan 27, 2016 at 10:50 AM, Cyril Bur [off-list ref] wrote:
On Mon, 25 Jan 2016 11:04:23 +1100
Balbir Singh [off-list ref] wrote:
quoted
On Thu, 21 Jan 2016 11:55:44 +1100
Cyril Bur [off-list ref] wrote:
quoted
Currently when threads get scheduled off they always giveup the FPU,
Altivec (VMX) and Vector (VSX) units if they were using them. When they are
scheduled back on a fault is then taken to enable each facility and load
registers. As a result explicitly disabling FPU/VMX/VSX has not been
necessary.

Future changes and optimisations remove this mandatory giveup and fault
which could cause calls such as clone() and fork() to copy threads and run
them later with FPU/VMX/VSX enabled but no registers loaded.

This patch starts the process of having MSR_{FP,VEC,VSX} mean that a
threads registers are hot while not having MSR_{FP,VEC,VSX} means that the
registers must be loaded. This allows for a smarter return to userspace.

Signed-off-by: Cyril Bur <redacted>
---
 arch/powerpc/kernel/process.c | 1 +
 1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index dccc87e..e0c3d2d 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1307,6 +1307,7 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,

            f = ret_from_fork;
    }
+   childregs->msr &= ~(MSR_FP|MSR_VEC|MSR_VSX);
Hi Balbir,

Perhaps I'm missing something, are you saying
quoted
Ideally you want to use __msr_check_and_clear()
instead of childregs->msr &= ~(MSR_FP|MSR_VEC|MSR_VSX); ? I don't see how that
can work...

__msr_check_and_clear() operates on the currently active MSR, that is, the msr
for the current kernel context. childregs->msr is the value that will be used
for that userspace context when the kernel returns. Here we must ensure that
that children are created with the bit disabled.
Yes, my bad! I thought the routine took generic bits, hoping to reuse
the CONFIG_VSX bits. I don't think it helps much, what you have is
correct.
quoted
Basically we start with these bits off and then take an exception on use?
Currently yes, this is what I'm trying to change. This patch hasn't been
necessary until now as any thread which saves its FPU/VMX/VSX data ALSO
disables those bits in regs->msr and so theres no way a clone() or fork() can
create a child with MSR_FP or MSR_VEC or MSR_VSX set. I add a meaning to
'having a regs->msr FP,VEC,VSX bit set' to mean that 'the regs are hot' in a
subsequent patch which means this assumption no longer holds so now we must
explicitly disable (so as to signal that the FPU/VMX/VSX regs are not hot) for
children thread.

Sounds like I still haven't got that commit message quite right yet.
I think the older series had more data to help understand the patch.
It would help to move some of them to the current series

Balbir
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help