lkml.org 
[lkml]   [2010]   [Aug]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH 08/20] powerpc: Preemptible mmu_gather
    From
    Date
    On Tue, 2010-08-31 at 16:26 +1000, Benjamin Herrenschmidt wrote:
    > On Sat, 2010-08-28 at 16:16 +0200, Peter Zijlstra wrote:
    > > Fix up powerpc to the new mmu_gather stuffs.
    >
    > Unfortunately, I think this is broken...
    >
    > First there's an actual bug here:
    >
    > > last = _switch(old_thread, new_thread);
    > >
    > > +#ifdef CONFIG_PPC64
    > > + if (task_thread_info(new)->local_flags & _TLF_LAZY_MMU) {
    > > + task_thread_info(new)->local_flags &= ~_TLF_LAZY_MMU;
    > > + batch = &__get_cpu_var(ppc64_tlb_batch);
    > > + batch->active = 1;
    > > + }
    > > +#endif
    > > +
    >
    > Here, you are coming out of _switch() which will have swapped the
    > stack and non-volatile registers to the state they were in when the
    > new task was originally switched-out. Thus "new" which is a local variable
    > (either on stack or in a non-volatile register) will now refer to whatever
    > was the next task back then.
    >
    > I suppose that's what's causing the similar patch you have in -rt to
    > fail btw. This could be fixed easily by using "current" instead.

    And here's a fix. Using current_thread_info() everywhere is better
    anyways, you are not supposed to touch the local_flags within anything
    but "current" so we may as well make it explicit.

    Feel free to fold that into your -rt patch and this one if you decide
    not to drop that whole part.

    diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
    index 6ee4b09..6e8b6de 100644
    --- a/arch/powerpc/kernel/process.c
    +++ b/arch/powerpc/kernel/process.c
    @@ -517,12 +517,12 @@ struct task_struct *__switch_to(struct task_struct *prev,

    batch = &__get_cpu_var(ppc64_tlb_batch);
    if (batch->active) {
    - task_thread_info(prev)->local_flags |= _TLF_LAZY_MMU;
    + current_thread_info()->local_flags |= _TLF_LAZY_MMU;
    if (batch->index)
    __flush_tlb_pending(batch);
    batch->active = 0;
    }
    -#endif
    +#endif /* CONFIG_PPC64 */

    local_irq_save(flags);

    @@ -539,12 +539,12 @@ struct task_struct *__switch_to(struct task_struct *prev,
    last = _switch(old_thread, new_thread);

    #ifdef CONFIG_PPC64
    - if (task_thread_info(new)->local_flags & _TLF_LAZY_MMU) {
    - task_thread_info(new)->local_flags &= ~_TLF_LAZY_MMU;
    + if (current_thread_info()->local_flags & _TLF_LAZY_MMU) {
    + current_thread_info()->local_flags &= ~_TLF_LAZY_MMU;
    batch = &__get_cpu_var(ppc64_tlb_batch);
    batch->active = 1;
    }
    -#endif
    +#endif /* CONFIG_PPC64 */

    local_irq_restore(flags);




    \
     
     \ /
      Last update: 2010-08-31 08:35    [W:0.021 / U:30.244 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site