Re: ptrace && fpu_lazy_restore

[Linus Torvalds]

On Sat, Apr 14, 2012 at 4:52 PM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> But I am not sure about the fix, and in any case I need more time
> to read this new code.

Oh, the fix is obviously correct. Yes, when we write to the FPU state
area, we need to invalidate the cached copy in the FPU itself.

It used to be that you could never have cached FPU contents unless you
owned the CPU (well, not strictly true - we used to do *really* lazy
FPU save/restone long long long ago in the pre-SMP days too), so this
bug is new with the lazy restore.

The only part I don't like about it is how hacky the location is. It
turns out that if the process *itself* does a "init_fpu()" call, then
the call to "unlazy_fpu()" will do the right thing and invalidate the
FPU state by setting fpu_owner_task to zero. But when we're ptracing
and doing an "init_fpu()" for somebody else, we'll skip that all,
because you can only really unlazy_fpu() yourself.

But that means that "init_fpu()" acts subtly differently depending on
whether you call it on yourself, or on another process.

So I actually think that I would prefer the patch that invalidates the
FPU caches more aggressively. Sure, we don't really *need* to
invalidate if we're just reading, but I'd almost prefer to just have
it done once in "init_fpu()".

The only case where we care about the FPU caches remaining is actually
the nice normal "we just switched tasks through normal scheduling".
Any path that calls "init_fpu()" is special enough that I think we
should just make sure the FPU state is all in memory. Hmm?

So I think I'd prefer the attached (UNTESTED!) oneliner instead.

If that works fine for people, how about sending it back to me with a
commit log and sign-off? I don't want to get credit for this patch
that is just a "let's do it in a different place" version of yours.

                             Linus

Attachment: patch.diff
Description: Binary data