Message ID | 20090925013528.GA2584@compile2.chatsunix.int.mrv.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
On Thu, 2009-09-24 at 18:35 -0700, Rex Feany wrote: > > Then I can boot and get to a shell, but userspace is slow. 8 seconds > to mount > /proc (vs. less then a second using my old kernel)! Maybe this is an > unrelated issue? I'm pretty clueless about the details, I'm sorry. > PG_arch_1 is used to prevent a cache flush unless it is actually > needed? > Then why would changing the location of the tlbil_va() make a > difference? I think there's more finishyness to 8xx than we thought. IE. That tlbil_va might have more reasons to be there than what the comment seems to advertize. Can you try to move it even higher up ? IE. Unconditionally at the beginning of set_pte_filter ? Also, if that doesn't help, can you try putting one in set_access_flags_filter() just below ? (Beware that there's two different versions of both functions, only the first one is compiled/used on 8xx). It's going to be hard for me to get that "right" since I don't really know what's going on with the core here, but I suppose if we get it moving along with extra tlb invalidations, that should be "good enough" until somebody who really knows what's going on comes up with possibly a better fix. Cheers, Ben.
Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org): > On Thu, 2009-09-24 at 18:35 -0700, Rex Feany wrote: > > > > Then I can boot and get to a shell, but userspace is slow. 8 seconds > > to mount > > /proc (vs. less then a second using my old kernel)! Maybe this is an > > unrelated issue? I'm pretty clueless about the details, I'm sorry. > > PG_arch_1 is used to prevent a cache flush unless it is actually > > needed? > > Then why would changing the location of the tlbil_va() make a > > difference? > > I think there's more finishyness to 8xx than we thought. IE. That > tlbil_va might have more reasons to be there than what the comment > seems to advertize. Can you try to move it even higher up ? IE. > Unconditionally at the beginning of set_pte_filter ? > > Also, if that doesn't help, can you try putting one in > set_access_flags_filter() just below ? > > (Beware that there's two different versions of both functions, only the > first one is compiled/used on 8xx). > > It's going to be hard for me to get that "right" since I don't really > know what's going on with the core here, but I suppose if we get it > moving along with extra tlb invalidations, that should be "good enough" > until somebody who really knows what's going on comes up with possibly > a better fix. I've tried sticking tlbil_va() in those places, nothing seems to help. In some cases userspace is slow, in other cases userspace is faster and unstable: sometimes commands hang, sometimes I am able to ctrl-c and and kill it, sometimes I get other strange crashes or falures (so far no kernel oopses though). take care! /rex.
> > Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org): > > > On Thu, 2009-09-24 at 18:35 -0700, Rex Feany wrote: > > > > > > Then I can boot and get to a shell, but userspace is slow. 8 seconds > > > to mount > > > /proc (vs. less then a second using my old kernel)! Maybe this is an > > > unrelated issue? I'm pretty clueless about the details, I'm sorry. > > > PG_arch_1 is used to prevent a cache flush unless it is actually > > > needed? > > > Then why would changing the location of the tlbil_va() make a > > > difference? > > > > I think there's more finishyness to 8xx than we thought. IE. That > > tlbil_va might have more reasons to be there than what the comment > > seems to advertize. Can you try to move it even higher up ? IE. > > Unconditionally at the beginning of set_pte_filter ? > > > > Also, if that doesn't help, can you try putting one in > > set_access_flags_filter() just below ? > > > > (Beware that there's two different versions of both functions, only the > > first one is compiled/used on 8xx). > > > > It's going to be hard for me to get that "right" since I don't really > > know what's going on with the core here, but I suppose if we get it > > moving along with extra tlb invalidations, that should be "good enough" > > until somebody who really knows what's going on comes up with possibly > > a better fix. > > I've tried sticking tlbil_va() in those places, nothing seems to help. > In some cases userspace is slow, in other cases userspace is faster and > unstable: sometimes commands hang, sometimes I am able to ctrl-c and > and kill it, sometimes I get other strange crashes or falures (so far no > kernel oopses though). This is exactly what you get when the "cache insn does not update DAR" bug hits you.
On Mon, 2009-09-28 at 18:21 -0700, Rex Feany wrote: > > It's going to be hard for me to get that "right" since I don't really > > know what's going on with the core here, but I suppose if we get it > > moving along with extra tlb invalidations, that should be "good enough" > > until somebody who really knows what's going on comes up with possibly > > a better fix. > > I've tried sticking tlbil_va() in those places, nothing seems to help. > In some cases userspace is slow, in other cases userspace is faster and > unstable: sometimes commands hang, sometimes I am able to ctrl-c and > and kill it, sometimes I get other strange crashes or falures (so far no > kernel oopses though). And you are positive that with 2.6.31 and your other patch, it works both fast and stable ? This is strange... the code should be mostly identical. I'll have a second look and see if I can get you a patch that reproduce -exactly- the behaviour of 2.6.31 plus your patch. Cheers, Ben.
On Tue, 2009-09-29 at 08:26 +0200, Joakim Tjernlund wrote: > > I've tried sticking tlbil_va() in those places, nothing seems to > help. > > In some cases userspace is slow, in other cases userspace is faster > and > > unstable: sometimes commands hang, sometimes I am able to ctrl-c and > > and kill it, sometimes I get other strange crashes or falures (so > far no > > kernel oopses though). > > This is exactly what you get when the "cache insn does not update DAR" > bug hits > you. But then why was it working fine before ? Or it wasn't ? Ben.
Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org): > On Mon, 2009-09-28 at 18:21 -0700, Rex Feany wrote: > > > It's going to be hard for me to get that "right" since I don't really > > > know what's going on with the core here, but I suppose if we get it > > > moving along with extra tlb invalidations, that should be "good enough" > > > until somebody who really knows what's going on comes up with possibly > > > a better fix. > > > > I've tried sticking tlbil_va() in those places, nothing seems to help. > > In some cases userspace is slow, in other cases userspace is faster and > > unstable: sometimes commands hang, sometimes I am able to ctrl-c and > > and kill it, sometimes I get other strange crashes or falures (so far no > > kernel oopses though). > > And you are positive that with 2.6.31 and your other patch, it works > both fast and stable ? This is strange... the code should be mostly > identical. I'll have a second look and see if I can get you a patch that > reproduce -exactly- the behaviour of 2.6.31 plus your patch. The difference is night and day - with 2.6.31 I can boot into single user mode, I can run our custom software (I left it running over night with very simple test script - no crashes). With the top of the tree I can sometimes boot into a shell, and if it isn't crashing or hanging on boot it runs very slow (10+ seconds to do anything, if I am lucky). thanks! /rex
diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c index 5304093..d927269 100644 --- a/arch/powerpc/mm/pgtable.c +++ b/arch/powerpc/mm/pgtable.c @@ -176,7 +176,7 @@ static pte_t set_pte_filter(pte_t pte, unsigned long addr) struct page *pg = maybe_pte_to_page(pte); if (!pg) return pte; - if (!test_bit(PG_arch_1, &pg->flags)) { + #ifdef CONFIG_8xx /* On 8xx, cache control instructions (particularly * "dcbst" from flush_dcache_icache) fault as write @@ -188,6 +188,8 @@ static pte_t set_pte_filter(pte_t pte, unsigned long addr) /* 8xx doesn't care about PID, size or ind args */ _tlbil_va(addr, 0, 0, 0); #endif /* CONFIG_8xx */ + + if (!test_bit(PG_arch_1, &pg->flags)) { flush_dcache_icache_page(pg); set_bit(PG_arch_1, &pg->flags); }