Re: can't flush tlb on e500

Re: can't flush tlb on e500

[Benjamin Herrenschmidt]

On Wed, 2009-05-20 at 19:12 +0900, Saito Hideo wrote:

> I think that the tlb should be cleared before mm->context.id is set
> MMU_NO_CONTEXT.

You are right, this definitely looks like a bug on platforms that have
HW support for the tlbil instruction (and thus care about the PID for
flushing) which afaik is only the case of recent freescale chips.

Have you verified that this change fixes your problem ?

Can you re-submit to linuxppc-dev@ozlabs.org mailing list, along with
proper changeset comment and signed-off-by: line ?

Cheers,
Ben.

> --- arch/powerpc/mm/mmu_context_nohash.c.orig	2009-03-24
> 08:12:14.000000000 +0900
> +++ arch/powerpc/mm/mmu_context_nohash.c	2009-05-20 18:33:53.000000000 +0900
> @@ -122,22 +122,22 @@ static unsigned int steal_context_up(uns
>  	struct mm_struct *mm;
>  	int cpu = smp_processor_id();
> 
>  	/* Pick up the victim mm */
>  	mm = context_mm[id];
> 
>  	pr_debug("[%d] steal context %d from mm @%p\n", cpu, id, mm);
> 
> -	/* Mark this mm has having no context anymore */
> -	mm->context.id = MMU_NO_CONTEXT;
> -
>  	/* Flush the TLB for that context */
>  	local_flush_tlb_mm(mm);
> 
> +	/* Mark this mm has having no context anymore */
> +	mm->context.id = MMU_NO_CONTEXT;
> +
>  	/* XXX This clear should ultimately be part of local_flush_tlb_mm */
>  	__clear_bit(id, stale_map[cpu]);
> 
>  	return id;
>  }
> 
>  #ifdef DEBUG_MAP_CONSISTENCY
>  static void context_check_map(void)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: can't flush tlb on e500

[Benjamin Herrenschmidt]

On Fri, 2009-05-22 at 19:27 +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2009-05-20 at 19:12 +0900, Saito Hideo wrote:
> 
> > I think that the tlb should be cleared before mm->context.id is set
> > MMU_NO_CONTEXT.
> 
> You are right, this definitely looks like a bug on platforms that have
> HW support for the tlbil instruction (and thus care about the PID for
> flushing) which afaik is only the case of recent freescale chips.

In fact, you are doubly right in that it also happens on other platforms
because local_flush_tlb_mm() will check if the PID is MMU_NO_CONTEXT
regardless of what tlbilx supports.. oops

Looks like I only ran my context torture test with CONFIG_SMP enabled.
Shame on me.

> Have you verified that this change fixes your problem ?
> 
> Can you re-submit to linuxppc-dev@ozlabs.org mailing list, along with
> proper changeset comment and signed-off-by: line ?
> 
> Cheers,
> Ben.
> 
> > --- arch/powerpc/mm/mmu_context_nohash.c.orig	2009-03-24
> > 08:12:14.000000000 +0900
> > +++ arch/powerpc/mm/mmu_context_nohash.c	2009-05-20 18:33:53.000000000 +0900
> > @@ -122,22 +122,22 @@ static unsigned int steal_context_up(uns
> >  	struct mm_struct *mm;
> >  	int cpu = smp_processor_id();
> > 
> >  	/* Pick up the victim mm */
> >  	mm = context_mm[id];
> > 
> >  	pr_debug("[%d] steal context %d from mm @%p\n", cpu, id, mm);
> > 
> > -	/* Mark this mm has having no context anymore */
> > -	mm->context.id = MMU_NO_CONTEXT;
> > -
> >  	/* Flush the TLB for that context */
> >  	local_flush_tlb_mm(mm);
> > 
> > +	/* Mark this mm has having no context anymore */
> > +	mm->context.id = MMU_NO_CONTEXT;
> > +
> >  	/* XXX This clear should ultimately be part of local_flush_tlb_mm */
> >  	__clear_bit(id, stale_map[cpu]);
> > 
> >  	return id;
> >  }
> > 
> >  #ifdef DEBUG_MAP_CONSISTENCY
> >  static void context_check_map(void)
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

can't flush tlb on e500

[Hideo Saito]

Hi all,

This report came from LKML, although I should post it here first,
the report said that there is a regression in linux-2.6.29 as to flushing a TLB entry that was going to be re-used when a context overflowed from the TLB.

On Fri, May 22, 2009 at 6:27 PM, Benjamin Herrenschmidt wrote:
> Have you verified that this change fixes your problem ?

I tested using "hackbench 100" and it was done successfully on my platform with e500(MPC8548).

>
> Can you re-submit to linuxppc-dev@ozlabs.org mailing list, along with
> proper changeset comment and signed-off-by: line ?

I am sorry I don't know as to the proper changeset comment.

Signed-off-by: Hideo Saito <hsaito.ppc@gmail.com>
---
--- arch/powerpc/mm/mmu_context_nohash.c.orig	2009-03-24 08:12:14.000000000 +0900
+++ arch/powerpc/mm/mmu_context_nohash.c	2009-05-20 18:33:53.000000000 +0900
@@ -122,22 +122,22 @@ static unsigned int steal_context_up(uns
 	struct mm_struct *mm;
 	int cpu = smp_processor_id();
 
 	/* Pick up the victim mm */
 	mm = context_mm[id];
 
 	pr_debug("[%d] steal context %d from mm @%p\n", cpu, id, mm);
 
-	/* Mark this mm has having no context anymore */
-	mm->context.id = MMU_NO_CONTEXT;
-
 	/* Flush the TLB for that context */
 	local_flush_tlb_mm(mm);
 
+	/* Mark this mm has having no context anymore */
+	mm->context.id = MMU_NO_CONTEXT;
+
 	/* XXX This clear should ultimately be part of local_flush_tlb_mm */
 	__clear_bit(id, stale_map[cpu]);
 
 	return id;
 }
 
 #ifdef DEBUG_MAP_CONSISTENCY
 static void context_check_map(void)

Re: can't flush tlb on e500

[Benjamin Herrenschmidt]

On Mon, 2009-05-25 at 10:33 +0900, Hideo Saito wrote:
> 
> This report came from LKML, although I should post it here first,
> the report said that there is a regression in linux-2.6.29 as to
> flushing a TLB entry that was going to be re-used when a context
> overflowed from the TLB.
> 
> On Fri, May 22, 2009 at 6:27 PM, Benjamin Herrenschmidt wrote:
> > Have you verified that this change fixes your problem ?
> 
> I tested using "hackbench 100" and it was done successfully on my
> platform with e500(MPC8548).
> 
> >
> > Can you re-submit to linuxppc-dev@ozlabs.org mailing list, along
> with
> > proper changeset comment and signed-off-by: line ?
> 
> I am sorry I don't know as to the proper changeset comment.
> 
Well, that simply means a proper description of the bug and
the fix :-)

I'll cook up one.

Cheers,
Ben.