Messages in this thread |  | | | From | Linus Torvalds <> | | Date | Sat, 14 Apr 2012 19:03:44 -0700 | | Subject | Re: ptrace && fpu_lazy_restore |
| |
On Sat, Apr 14, 2012 at 4:52 PM, Oleg Nesterov <oleg@redhat.com> 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 [unhandled content-type:application/octet-stream] |  |