Patchwork mpc880 linux-2.6.32 slow running processes

login
register
mail settings
Submitter michael
Date Jan. 5, 2011, 6:23 p.m.
Message ID <4D24B725.2020300@evidence.eu.com>
Download mbox | patch
Permalink /patch/77627/
State Not Applicable
Headers show

Comments

michael - Jan. 5, 2011, 6:23 p.m.
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
Scott Wood - Jan. 5, 2011, 11:42 p.m.
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.
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.
>>> +  /* 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.
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.
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.
>
> 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.
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

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