| Message ID | 20091015004127.GA15570@laura.chatsunix.int.mrv.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote: > The biggest problem for me turned out to be the MMU context IDs being > clamped to 32 when the 8xx only has 16. With this, things are a bit more > stable :) Ugh ? The clamp went upstream ? That sucks... let me fix that asap Cheers, Ben. > diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c > index c2f93dc..15e00c5 100644 > --- a/arch/powerpc/mm/mmu_context_nohash.c > +++ b/arch/powerpc/mm/mmu_context_nohash.c > @@ -404,7 +404,8 @@ void __init mmu_context_init(void) > } > > #ifdef DEBUG_CLAMP_LAST_CONTEXT > - last_context = DEBUG_CLAMP_LAST_CONTEXT; > + if (last_context > DEBUG_CLAMP_LAST_CONTEXT) > + last_context = DEBUG_CLAMP_LAST_CONTEXT; > #endif > /* > * Allocate the maps used by context management > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c > index e7dae82..26fb6b9 100644 > --- a/arch/powerpc/mm/fault.c > +++ b/arch/powerpc/mm/fault.c > @@ -40,7 +40,7 @@ > #include <asm/uaccess.h> > #include <asm/tlbflush.h> > #include <asm/siginfo.h> > - > +#include <mm/mmu_decl.h> > > #ifdef CONFIG_KPROBES > static inline int notify_page_fault(struct pt_regs *regs) > @@ -246,6 +246,12 @@ good_area: > goto bad_area; > #endif /* CONFIG_6xx */ > #if defined(CONFIG_8xx) > + /* 8xx sometimes need to load a invalid/non-present TLBs. > + * These must be invalidated separately as linux mm don't. > + */ > + if (error_code & 0x40000000) /* no translation? */ > + _tlbil_va(address, 0, 0, 0); > + > /* The MPC8xx seems to always set 0x80000000, which is > * "undefined". Of those that can be set, this is the only > * one which seems bad.
On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote: > The biggest problem for me turned out to be the MMU context IDs being > clamped to 32 when the 8xx only has 16. With this, things are a bit more > stable :) This is with Joakim patches or without ? Ben. > diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c > index c2f93dc..15e00c5 100644 > --- a/arch/powerpc/mm/mmu_context_nohash.c > +++ b/arch/powerpc/mm/mmu_context_nohash.c > @@ -404,7 +404,8 @@ void __init mmu_context_init(void) > } > > #ifdef DEBUG_CLAMP_LAST_CONTEXT > - last_context = DEBUG_CLAMP_LAST_CONTEXT; > + if (last_context > DEBUG_CLAMP_LAST_CONTEXT) > + last_context = DEBUG_CLAMP_LAST_CONTEXT; > #endif > /* > * Allocate the maps used by context management > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c > index e7dae82..26fb6b9 100644 > --- a/arch/powerpc/mm/fault.c > +++ b/arch/powerpc/mm/fault.c > @@ -40,7 +40,7 @@ > #include <asm/uaccess.h> > #include <asm/tlbflush.h> > #include <asm/siginfo.h> > - > +#include <mm/mmu_decl.h> > > #ifdef CONFIG_KPROBES > static inline int notify_page_fault(struct pt_regs *regs) > @@ -246,6 +246,12 @@ good_area: > goto bad_area; > #endif /* CONFIG_6xx */ > #if defined(CONFIG_8xx) > + /* 8xx sometimes need to load a invalid/non-present TLBs. > + * These must be invalidated separately as linux mm don't. > + */ > + if (error_code & 0x40000000) /* no translation? */ > + _tlbil_va(address, 0, 0, 0); > + > /* The MPC8xx seems to always set 0x80000000, which is > * "undefined". Of those that can be set, this is the only > * one which seems bad.
Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org): > On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote: > > The biggest problem for me turned out to be the MMU context IDs being > > clamped to 32 when the 8xx only has 16. With this, things are a bit more > > stable :) > > This is with Joakim patches or without ? with just one: adding tlbil_va() to do_page_fault(). take care! /rex.
On Wed, 2009-10-14 at 18:08 -0700, Rex Feany wrote: > Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org): > > > On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote: > > > The biggest problem for me turned out to be the MMU context IDs being > > > clamped to 32 when the 8xx only has 16. With this, things are a bit more > > > stable :) > > > > This is with Joakim patches or without ? > > with just one: adding tlbil_va() to do_page_fault(). Ah cool, at least that keeps my sanity, I didn't get how it could work at all without that bit :-) Scott, Joakim: It will be interesting to figure out exactly what is the condition where that is necessary. It definitely should make the one in set_pte_filter() or ptep_set_access_flags_filter() unnecessary. Joakim, you mentioned increased amount of TLB errors or misses when doing that too often, that sounds a bit weird and worth investigating Finally we could implement preload from update_mmu_cache() but that's a different matter alltogether. Cheers, Ben.
Rex Feany <RFeany@mrv.com> wrote on 15/10/2009 03:08:55: > > Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org): > > > On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote: > > > The biggest problem for me turned out to be the MMU context IDs being > > > clamped to 32 when the 8xx only has 16. With this, things are a bit more > > > stable :) > > > > This is with Joakim patches or without ? > > with just one: adding tlbil_va() to do_page_fault(). Looking forward too hear about the the other patches too :)
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 15/10/2009 03:12:50: > > On Wed, 2009-10-14 at 18:08 -0700, Rex Feany wrote: > > Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org): > > > > > On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote: > > > > The biggest problem for me turned out to be the MMU context IDs being > > > > clamped to 32 when the 8xx only has 16. With this, things are a bit more > > > > stable :) > > > > > > This is with Joakim patches or without ? > > > > with just one: adding tlbil_va() to do_page_fault(). > > Ah cool, at least that keeps my sanity, I didn't get how it could work > at all without that bit :-) > > Scott, Joakim: It will be interesting to figure out exactly what is the > condition where that is necessary. It definitely should make the one > in set_pte_filter() or ptep_set_access_flags_filter() unnecessary. > > Joakim, you mentioned increased amount of TLB errors or misses when > doing that too often, that sounds a bit weird and worth investigating I didn't say that, did I? More like: if I don't do tlbil_va at all I get a lot of extra/duplicate TLB errors for the same address. Adding the patch makes these go away. I guess one could do tlbil_va unconditionally but I didn't see any improvements from doing that, but this was all on 2.4 > > Finally we could implement preload from update_mmu_cache() but that's a > different matter alltogether. > > Cheers, > Ben. > > > >
On Thu, 2009-10-15 at 07:42 +0200, Joakim Tjernlund wrote: > I didn't say that, did I? More like: > if I don't do tlbil_va at all I get a lot of extra/duplicate TLB > errors > for the same address. Adding the patch makes these go away. > I guess one could do tlbil_va unconditionally but I didn't > see any improvements from doing that, but this was all on 2.4 Ok so I misparsed you. I was advocating going unconditional here and though you said it would increase the miss rate. Cheers, Ben
diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c index c2f93dc..15e00c5 100644 --- a/arch/powerpc/mm/mmu_context_nohash.c +++ b/arch/powerpc/mm/mmu_context_nohash.c @@ -404,7 +404,8 @@ void __init mmu_context_init(void) } #ifdef DEBUG_CLAMP_LAST_CONTEXT - last_context = DEBUG_CLAMP_LAST_CONTEXT; + if (last_context > DEBUG_CLAMP_LAST_CONTEXT) + last_context = DEBUG_CLAMP_LAST_CONTEXT; #endif /* * Allocate the maps used by context management diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index e7dae82..26fb6b9 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -40,7 +40,7 @@ #include <asm/uaccess.h> #include <asm/tlbflush.h> #include <asm/siginfo.h> - +#include <mm/mmu_decl.h> #ifdef CONFIG_KPROBES static inline int notify_page_fault(struct pt_regs *regs) @@ -246,6 +246,12 @@ good_area: goto bad_area; #endif /* CONFIG_6xx */ #if defined(CONFIG_8xx) + /* 8xx sometimes need to load a invalid/non-present TLBs. + * These must be invalidated separately as linux mm don't. + */ + if (error_code & 0x40000000) /* no translation? */ + _tlbil_va(address, 0, 0, 0); + /* The MPC8xx seems to always set 0x80000000, which is * "undefined". Of those that can be set, this is the only * one which seems bad.