lkml.org 
[lkml]   [2012]   [Apr]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: 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]
    \
     
     \ /
      Last update: 2012-04-15 04:07    [W:0.047 / U:0.124 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site