diff mbox

mpc880 linux-2.6.32 slow running processes

Message ID 4D24B725.2020300@evidence.eu.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

michael Jan. 5, 2011, 6:23 p.m. UTC 
Hi

On 01/05/2011 07:09 PM, Rafael Beims wrote:
> Hello all,
> I'm working with an MPC880 board that is supposed to run linux-2.6.32. After
> some work, I could get the kernel up and running, mounting a rootfs via
> NFS.
> The problem that I'm facing now is that when I try to run any process (may
> be an ls, cat, or whatever), the response of the process is *very* slow
> (something like 10 to 20 seconds for a ls).
> Monitoring the network packets, I can see that the nfs protocol is exchanged
> just fine, but at some time it simply stops, starting again with a request
> from the board several seconds later.
> I'm thinking that the processor is running (something, I don't know what)
> for all this time before I can see requests coming from the board again. I
> pinged the board from the PC and during all this I can see the packets being
> answered.
>
Can you try this patch?

> My question to all is, did anyone see something like this already? Besides
> that, what is the status of the linux kernel support for the 8xx platform?
> Is it being actively tested / used today? I ask this because it seems that
> all the information on the internet very aged (forum discussions from 2005
> and below mostly).
>
> Is there something that I can do to try to narrow the cause of the problem?
>
> Anyway, any help would be truly appreciated. Please excuse me if this is not
> the right place to ask this too.
>
> Thanks and best regards,
> Rafael Beims
> rbeims@gmail.com
> (41) 8873-7565
> "What I hear, I forget. What I see, I remember. And what I do, I
> understand."
> - Chinese Proverb
>
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Comments

Scott Wood Jan. 5, 2011, 11:42 p.m. UTC | #1 
On Wed, 5 Jan 2011 19:23:33 +0100
michael <michael@evidence.eu.com> wrote:

> diff --git a/arch/powerpc/include/asm/pte-8xx.h b/arch/powerpc/include/asm/pte-8xx.h
> index dd5ea95..cb67076 100644
> --- a/arch/powerpc/include/asm/pte-8xx.h
> +++ b/arch/powerpc/include/asm/pte-8xx.h
> @@ -32,7 +32,7 @@
>  #define _PAGE_FILE	0x0002	/* when !present: nonlinear file mapping */
>  #define _PAGE_NO_CACHE	0x0002	/* I: cache inhibit */
>  #define _PAGE_SHARED	0x0004	/* No ASID (context) compare */
> -#define _PAGE_SPECIAL	0x0008	/* SW entry, forced to 0 by the TLB miss */
> +#define _PAGE_SPECIAL	0x0000	/* SW entry, forced to 0 by the TLB miss */

What do you think is going wrong with the special bit on 8xx?

Or might the change to set_pte_filter() alone be what fixed the problem?

> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index 5304093..1da03a8 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -173,21 +173,29 @@ static pte_t set_pte_filter(pte_t pte, unsigned long addr)
>  	pte = __pte(pte_val(pte) & ~_PAGE_HPTEFLAGS);
>  	if (pte_looks_normal(pte) && !(cpu_has_feature(CPU_FTR_COHERENT_ICACHE) ||
>  				       cpu_has_feature(CPU_FTR_NOEXECUTE))) {
> -		struct page *pg = maybe_pte_to_page(pte);
> -		if (!pg)
> +		unsigned long pfn = pte_pfn(pte);
> +		struct page *pg;
> +
> +		if (unlikely(!pfn_valid(pfn)))
>  			return pte;
> -		if (!test_bit(PG_arch_1, &pg->flags)) {
> +
> +		pg = pfn_to_page(pfn);
>  #ifdef CONFIG_8xx
> -			/* On 8xx, cache control instructions (particularly
> -			 * "dcbst" from flush_dcache_icache) fault as write
> -			 * operation if there is an unpopulated TLB entry
> -			 * for the address in question. To workaround that,
> -			 * we invalidate the TLB here, thus avoiding dcbst
> -			 * misbehaviour.
> -			 */
> -			/* 8xx doesn't care about PID, size or ind args */
> -			_tlbil_va(addr, 0, 0, 0);
> +		/* On 8xx, cache control instructions (particularly
> +		 * "dcbst" from flush_dcache_icache) fault as write
> +		 * operation if there is an unpopulated TLB entry
> +		 * for the address in question. To workaround that,
> +		 * we invalidate the TLB here, thus avoiding dcbst
> +		 * misbehaviour.
> +		 */
> +		/* 8xx doesn't care about PID, size or ind args */
> +		_tlbil_va(addr, 0, 0, 0);
>  #endif /* CONFIG_8xx */
> +
> +		if (!pg)
> +			return pte;
> +
> +		if (!PageReserved(pg) && !test_bit(PG_arch_1, &pg->flags)) {
>  			flush_dcache_icache_page(pg);
>  			set_bit(PG_arch_1, &pg->flags);
>  		}

Rex, do you recall under what specific circumstances the _tlbil_va is
needed?  Is it possible that it will be caused by a dcbst in other
contexts that are not dependent on the state of PG_arch_1?

-Scott
michael Jan. 6, 2011, 12:52 p.m. UTC | #2 
Hi

On 01/06/2011 12:42 AM, Scott Wood wrote:
> On Wed, 5 Jan 2011 19:23:33 +0100
> michael<michael@evidence.eu.com>  wrote:
>
>> diff --git a/arch/powerpc/include/asm/pte-8xx.h b/arch/powerpc/include/asm/pte-8xx.h
>> index dd5ea95..cb67076 100644
>> --- a/arch/powerpc/include/asm/pte-8xx.h
>> +++ b/arch/powerpc/include/asm/pte-8xx.h
>> @@ -32,7 +32,7 @@
>>   #define _PAGE_FILE	0x0002	/* when !present: nonlinear file mapping */
>>   #define _PAGE_NO_CACHE	0x0002	/* I: cache inhibit */
>>   #define _PAGE_SHARED	0x0004	/* No ASID (context) compare */
>> -#define _PAGE_SPECIAL	0x0008	/* SW entry, forced to 0 by the TLB miss */
>> +#define _PAGE_SPECIAL	0x0000	/* SW entry, forced to 0 by the TLB miss */
> What do you think is going wrong with the special bit on 8xx?
>
> Or might the change to set_pte_filter() alone be what fixed the problem?
>
Only the set_pte_filter doesn't fix the problem. The slow-down depends on
the __HAVE_ARCH_PTE_SPECIAL related code, but 2 months ago I didn't find the reason
and now I don't have the architecture. I will do some tests when I will came back in Italy.
>> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
>> index 5304093..1da03a8 100644
>> --- a/arch/powerpc/mm/pgtable.c
>> +++ b/arch/powerpc/mm/pgtable.c
>> @@ -173,21 +173,29 @@ static pte_t set_pte_filter(pte_t pte, unsigned long addr)
>>   	pte = __pte(pte_val(pte)&  ~_PAGE_HPTEFLAGS);
>>   	if (pte_looks_normal(pte)&&  !(cpu_has_feature(CPU_FTR_COHERENT_ICACHE) ||
>>   				       cpu_has_feature(CPU_FTR_NOEXECUTE))) {
>> -		struct page *pg = maybe_pte_to_page(pte);
>> -		if (!pg)
>> +		unsigned long pfn = pte_pfn(pte);
>> +		struct page *pg;
>> +
>> +		if (unlikely(!pfn_valid(pfn)))
>>   			return pte;
>> -		if (!test_bit(PG_arch_1,&pg->flags)) {
>> +
>> +		pg = pfn_to_page(pfn);
>>   #ifdef CONFIG_8xx
>> -			/* On 8xx, cache control instructions (particularly
>> -			 * "dcbst" from flush_dcache_icache) fault as write
>> -			 * operation if there is an unpopulated TLB entry
>> -			 * for the address in question. To workaround that,
>> -			 * we invalidate the TLB here, thus avoiding dcbst
>> -			 * misbehaviour.
>> -			 */
>> -			/* 8xx doesn't care about PID, size or ind args */
>> -			_tlbil_va(addr, 0, 0, 0);
>> +		/* On 8xx, cache control instructions (particularly
>> +		 * "dcbst" from flush_dcache_icache) fault as write
>> +		 * operation if there is an unpopulated TLB entry
>> +		 * for the address in question. To workaround that,
>> +		 * we invalidate the TLB here, thus avoiding dcbst
>> +		 * misbehaviour.
>> +		 */
>> +		/* 8xx doesn't care about PID, size or ind args */
>> +		_tlbil_va(addr, 0, 0, 0);
>>   #endif /* CONFIG_8xx */
>> +
>> +		if (!pg)
>> +			return pte;
>> +
>> +		if (!PageReserved(pg)&&  !test_bit(PG_arch_1,&pg->flags)) {
>>   			flush_dcache_icache_page(pg);
>>   			set_bit(PG_arch_1,&pg->flags);
>>   		}
> Rex, do you recall under what specific circumstances the _tlbil_va is
> needed?  Is it possible that it will be caused by a dcbst in other
> contexts that are not dependent on the state of PG_arch_1?
>
> -Scott
>
Michael
Joakim Tjernlund Jan. 6, 2011, 4:52 p.m. UTC | #3 
>>> +  /* On 8xx, cache control instructions (particularly
>>> +   * "dcbst" from flush_dcache_icache) fault as write
>>> +   * operation if there is an unpopulated TLB entry
>>> +   * for the address in question. To workaround that,
>>> +   * we invalidate the TLB here, thus avoiding dcbst
>>> +   * misbehaviour.
>>> +   */
>>> +  /* 8xx doesn't care about PID, size or ind args */
>>> +  _tlbil_va(addr, 0, 0, 0);
>>>   #endif /* CONFIG_8xx */
>>> +
>>> +  if (!pg)
>>> +   return pte;
>>> +
>>> +  if (!PageReserved(pg)&&  !test_bit(PG_arch_1,&pg->flags)) {
>>>      flush_dcache_icache_page(pg);
>>>      set_bit(PG_arch_1,&pg->flags);
>>>     }
>> Rex, do you recall under what specific circumstances the _tlbil_va is
>> needed?  Is it possible that it will be caused by a dcbst in other
>> contexts that are not dependent on the state of PG_arch_1?

The 8xx tlbil_va should not be needed in recent 2.6 after I fixed the 8xx TLB code
to workaround the dcbst bug there instead. See 
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=0a2ab51ffb8dfdf51402dcfb446629648c96bc78;hp=60e071fee994ff98c37d03a4a7c5a3f8b1e3b8e5

Not sure what release it went into though.

 Jocke
Rafael Beims Jan. 7, 2011, 10 a.m. UTC | #4 
On Thu, Jan 6, 2011 at 2:52 PM, Joakim Tjernlund
<joakim.tjernlund@transmode.se> wrote:

>
> The 8xx tlbil_va should not be needed in recent 2.6 after I fixed the 8xx TLB code
> to workaround the dcbst bug there instead. See
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=0a2ab51ffb8dfdf51402dcfb446629648c96bc78;hp=60e071fee994ff98c37d03a4a7c5a3f8b1e3b8e5
>
> Not sure what release it went into though.
>
>  Jocke

I saw that the commit you mention did make it in the 2.6.33 version. I
will try to boot it here and see if the problem is also solved in this
version.

Rafael
Joakim Tjernlund Jan. 8, 2011, 9:43 p.m. UTC | #5 
Rafael Beims <rbeims@gmail.com> wrote on 2011/01/07 11:00:56:
>
> On Thu, Jan 6, 2011 at 2:52 PM, Joakim Tjernlund
> <joakim.tjernlund@transmode.se> wrote:
>
> >
> > The 8xx tlbil_va should not be needed in recent 2.6 after I fixed the 8xx TLB code
> > to workaround the dcbst bug there instead. See
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=0a2ab51ffb8dfdf51402dcfb446629648c96bc78;hp=60e071fee994ff98c37d03a4a7c5a3f8b1e3b8e5
> >
> > Not sure what release it went into though.
> >
> >  Jocke
>
> I saw that the commit you mention did make it in the 2.6.33 version. I
> will try to boot it here and see if the problem is also solved in this
> version.

Once you have tested it and it works, please send a patch to remove the 8xx workaround.
Make sure Scott is cc:ed
Rafael Beims Jan. 10, 2011, 4:35 p.m. UTC | #6 
>
> Once you have tested it and it works, please send a patch to remove the 8xx workaround.
> Make sure Scott is cc:ed
>
>

I tested linux-2.6.33 on my ppc880 board today, and even without the
slowdown.patch applied, the board runs processes with good
performance.
It really seems that the problem is solved from linux-2.6.33 on.

I'm not sure what you mean by sending a patch to remove the
workaround. The only thing that I did in the 2.6.32 version was to
apply the slowdown.patch attached in the message from Michael.

Could you clarify please?

Thanks for all the help so far,
Rafael
Joakim Tjernlund Jan. 10, 2011, 4:55 p.m. UTC | #7 
Rafael Beims <rbeims@gmail.com> wrote on 2011/01/10 17:35:38:
> >
> > Once you have tested it and it works, please send a patch to remove the 8xx workaround.
> > Make sure Scott is cc:ed
> >
> >
>
> I tested linux-2.6.33 on my ppc880 board today, and even without the
> slowdown.patch applied, the board runs processes with good
> performance.
> It really seems that the problem is solved from linux-2.6.33 on.
>
> I'm not sure what you mean by sending a patch to remove the
> workaround. The only thing that I did in the 2.6.32 version was to
> apply the slowdown.patch attached in the message from Michael.
>
> Could you clarify please?

Yes, this part in arch/powerpc/mm/pgtable.c:
#ifdef CONFIG_8xx
			/* On 8xx, cache control instructions (particularly
			 * "dcbst" from flush_dcache_icache) fault as write
			 * operation if there is an unpopulated TLB entry
			 * for the address in question. To workaround that,
			 * we invalidate the TLB here, thus avoiding dcbst
			 * misbehaviour.
			 */
			/* 8xx doesn't care about PID, size or ind args */
			_tlbil_va(addr, 0, 0, 0);
#endif /* CONFIG_8xx */

Should be removed in >= 2.6.33 kernels.
My 8xx TLB work fixes this problem more efficiently.

    Jocke
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/pte-8xx.h b/arch/powerpc/include/asm/pte-8xx.h
index dd5ea95..cb67076 100644
--- a/arch/powerpc/include/asm/pte-8xx.h
+++ b/arch/powerpc/include/asm/pte-8xx.h
@@ -32,7 +32,7 @@ 
 #define _PAGE_FILE	0x0002	/* when !present: nonlinear file mapping */
 #define _PAGE_NO_CACHE	0x0002	/* I: cache inhibit */
 #define _PAGE_SHARED	0x0004	/* No ASID (context) compare */
-#define _PAGE_SPECIAL	0x0008	/* SW entry, forced to 0 by the TLB miss */
+#define _PAGE_SPECIAL	0x0000	/* SW entry, forced to 0 by the TLB miss */
 
 /* These five software bits must be masked out when the entry is loaded
  * into the TLB.
diff --git a/arch/powerpc/include/asm/pte-common.h b/arch/powerpc/include/asm/pte-common.h
index f2b3701..3b88376 100644
--- a/arch/powerpc/include/asm/pte-common.h
+++ b/arch/powerpc/include/asm/pte-common.h
@@ -176,5 +176,7 @@  extern unsigned long bad_call_to_PMD_PAGE_SIZE(void);
 #define HAVE_PAGE_AGP
 
 /* Advertise support for _PAGE_SPECIAL */
+#if _PAGE_SPECIAL != 0
 #define __HAVE_ARCH_PTE_SPECIAL
+#endif
 
diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
index 5304093..1da03a8 100644
--- a/arch/powerpc/mm/pgtable.c
+++ b/arch/powerpc/mm/pgtable.c
@@ -173,21 +173,29 @@  static pte_t set_pte_filter(pte_t pte, unsigned long addr)
 	pte = __pte(pte_val(pte) & ~_PAGE_HPTEFLAGS);
 	if (pte_looks_normal(pte) && !(cpu_has_feature(CPU_FTR_COHERENT_ICACHE) ||
 				       cpu_has_feature(CPU_FTR_NOEXECUTE))) {
-		struct page *pg = maybe_pte_to_page(pte);
-		if (!pg)
+		unsigned long pfn = pte_pfn(pte);
+		struct page *pg;
+
+		if (unlikely(!pfn_valid(pfn)))
 			return pte;
-		if (!test_bit(PG_arch_1, &pg->flags)) {
+
+		pg = pfn_to_page(pfn);
 #ifdef CONFIG_8xx
-			/* On 8xx, cache control instructions (particularly
-			 * "dcbst" from flush_dcache_icache) fault as write
-			 * operation if there is an unpopulated TLB entry
-			 * for the address in question. To workaround that,
-			 * we invalidate the TLB here, thus avoiding dcbst
-			 * misbehaviour.
-			 */
-			/* 8xx doesn't care about PID, size or ind args */
-			_tlbil_va(addr, 0, 0, 0);
+		/* On 8xx, cache control instructions (particularly
+		 * "dcbst" from flush_dcache_icache) fault as write
+		 * operation if there is an unpopulated TLB entry
+		 * for the address in question. To workaround that,
+		 * we invalidate the TLB here, thus avoiding dcbst
+		 * misbehaviour.
+		 */
+		/* 8xx doesn't care about PID, size or ind args */
+		_tlbil_va(addr, 0, 0, 0);
 #endif /* CONFIG_8xx */
+
+		if (!pg)
+			return pte;
+
+		if (!PageReserved(pg) && !test_bit(PG_arch_1, &pg->flags)) {
 			flush_dcache_icache_page(pg);
 			set_bit(PG_arch_1, &pg->flags);
 		}