All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Andrea Arcangeli <aarcange@redhat.com>,
	Avi Kivity <avi@redhat.com>, Thomas Gleixner <tglx@linutronix.de>,
	Rik van Riel <riel@redhat.com>, Ingo Molnar <mingo@elte.hu>,
	akpm@linux-foundation.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	David Miller <davem@davemloft.net>,
	Hugh Dickins <hugh.dickins@tiscali.co.uk>,
	Mel Gorman <mel@csn.ul.ie>, Nick Piggin <npiggin@suse.de>,
	Paul McKenney <paulmck@linux.vnet.ibm.com>,
	Yanmin Zhang <yanmin_zhang@linux.intel.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>
Subject: Re: [PATCH 08/20] powerpc: Preemptible mmu_gather
Date: Tue, 31 Aug 2010 16:31:05 +1000	[thread overview]
Message-ID: <1283236265.2151.35.camel@pasglop> (raw)
In-Reply-To: <1283236004.2151.33.camel@pasglop>

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);
 



  reply	other threads:[~2010-08-31  6:32 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-28 14:16 [PATCH 00/20] mm: Preemptibility -v4 Peter Zijlstra
2010-08-28 14:16 ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 01/20] powerpc: Use call_rcu_sched() for pagetables Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-31  6:10   ` Benjamin Herrenschmidt
2010-08-28 14:16 ` [PATCH 02/20] mm: Improve page_lock_anon_vma() comment Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 03/20] mm: Rename drop_anon_vma to put_anon_vma Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 15:08   ` Pekka Enberg
2010-08-28 14:16 ` [PATCH 04/20] mm: Move anon_vma ref out from under CONFIG_KSM Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 05/20] mm: Simplify anon_vma refcounts Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 15:13   ` Pekka Enberg
2010-08-28 14:16 ` [PATCH 06/20] mm: Use refcounts for page_lock_anon_vma() Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 07/20] mm: Preemptible mmu_gather Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 08/20] powerpc: " Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-31  6:26   ` Benjamin Herrenschmidt
2010-08-31  6:31     ` Benjamin Herrenschmidt [this message]
2010-08-31  9:14       ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 09/20] sparc: " Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 10/20] s390: preemptible mmu_gather Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 11/20] arm: Preemptible mmu_gather Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 12/20] sh: " Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 13/20] um: " Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 14/20] ia64: " Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-30 15:44   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 15/20] mm, powerpc: Move the RCU page-table freeing into generic code Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 16/20] lockdep, mutex: Provide mutex_lock_nest_lock Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 17/20] mutex: Provide mutex_is_contended Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 18/20] mm: Convert i_mmap_lock and anon_vma->lock to mutexes Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 19/20] mm: Extended batches for generic mmu_gather Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:16 ` [PATCH 20/20] mm: Optimize page_lock_anon_vma() fast-path Peter Zijlstra
2010-08-28 14:16   ` Peter Zijlstra
2010-08-28 14:32 ` [PATCH 00/20] mm: Preemptibility -v4 Peter Zijlstra
2010-08-28 22:28   ` David Miller
2010-08-28 22:41     ` Peter Zijlstra
2010-08-28 14:56 ` Piotr Hosowicz
2010-08-28 15:10   ` Peter Zijlstra
2010-08-28 15:17     ` Piotr Hosowicz
2010-08-28 15:23       ` Peter Zijlstra
2010-08-28 16:01         ` Piotr Hosowicz
2010-08-29 12:46         ` Piotr Hosowicz
2010-08-29 13:37           ` Peter Zijlstra
2010-08-29 13:43             ` Piotr Hosowicz
2010-08-31 14:02           ` Piotr Hosowicz
2010-08-31 14:14             ` Piotr Hosowicz
2010-09-02 14:53             ` Piotr Hosowicz
2010-08-28 15:19 ` Pekka Enberg
2010-08-28 15:27   ` Peter Zijlstra
     [not found] ` <AANLkTikSm2Mq8hGNac9rpFH-3pvryw2kW57EP45Ny6Vp@mail.gmail.com>
2010-09-14  5:36   ` Alex,Shi
2010-09-14  7:42     ` Peter Zijlstra
2010-10-18 11:24 [PATCH 00/20] mm: Preemptibility -v5 Peter Zijlstra
2010-10-18 11:24 ` [PATCH 08/20] powerpc: Preemptible mmu_gather Peter Zijlstra
2010-10-18 11:24   ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1283236265.2151.35.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=avi@redhat.com \
    --cc=davem@davemloft.net \
    --cc=hugh.dickins@tiscali.co.uk \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mel@csn.ul.ie \
    --cc=mingo@elte.hu \
    --cc=npiggin@suse.de \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=riel@redhat.com \
    --cc=sfr@canb.auug.org.au \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=yanmin_zhang@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.