Probably cache synchronization issue on SH7723

Probably cache synchronization issue on SH7723

[Markus Pietrek]

Hello,

when starting bigger graphical applications they suddenly stop with a 
segmentation fault or a bus error. If I execute the application a second 
time, everything is fine. Applications that are linked to shared 
libraries are more likely to fail than statically linked applications.

Changing the cache from write-back to write-through fixes it. Maybe the 
instruction cache and the data cache are not synchronized correctly?

I didn't have this problem with the linux-2.6.29 vanilla kernel. 
2.6.32rc2 from 
git://git.kernel.org/pub/scm/linux/kernel/git/lethal/sh-2.6 introduced 
it. And after a pull yesterday even /bin/init is failing.

On 9th Oct 2009, there was a similar bug report for the sh7785lcr. Does 
this platform now work?

Mit freundlichen Grüßen / Best Regards

Markus Pietrek
Software engineer

-- 
emtrion GmbH
Greschbachstr. 12
76229 Karlsruhe
GERMANY

Phone  +49 721 62725-45
Fax    +49 721 62725-19
E-mail Markus.Pietrek@emtrion.de
http://www.emtrion.de

_____________________________________

Amtsgericht Mannheim
HRB 110 300
Geschäftsführer: Dieter Baur, Ramona Maurer
_____________________________________

Important Note:
- This e-mail may contain trade secrets or privileged, undisclosed or 
otherwise confidential information.
- If you have received this e-mail in error, you are hereby notified that
   any review, copying or distribution of it is strictly prohibited.
- Please inform us immediately and destroy the original transmittal.
Thank you for your cooperation.

Re: Probably cache synchronization issue on SH7723

[Magnus Damm]

Hi Markus,

On Thu, Dec 3, 2009 at 9:36 PM, Markus Pietrek
<Markus.Pietrek@emtrion.de> wrote:
> when starting bigger graphical applications they suddenly stop with a
> segmentation fault or a bus error. If I execute the application a second
> time, everything is fine. Applications that are linked to shared libraries
> are more likely to fail than statically linked applications.
>
> Changing the cache from write-back to write-through fixes it. Maybe the
> instruction cache and the data cache are not synchronized correctly?
>
> I didn't have this problem with the linux-2.6.29 vanilla kernel. 2.6.32rc2
> from git://git.kernel.org/pub/scm/linux/kernel/git/lethal/sh-2.6 introduced
> it. And after a pull yesterday even /bin/init is failing.

I've found that sh7722 and sh7724 doesn't work with copy back cache
right now, see below. There may be other issues as well, but going
back to OK fixes it for my initramfs-only case.

Cheers,

/ magnus

BAD:

commit 39ac11c1607f1d566e7cf885acd403fa4f07f8a2
Author: Stuart Menefy <stuart.menefy@st.com>
Date:   Tue Oct 27 15:14:06 2009 +0000

   sh: Improve performance of SH4 versions of copy/clear_user_highpage

OK:

commit 49fb2cd2571e0134e5a12c5abab227696e4940c7
Merge: dfc3494 260af56
Author: Paul Mundt <lethal@linux-sh.org>
Date:   Tue Nov 24 16:32:11 2009 +0900

   Merge branch 'master' into sh/st-integration

Re: Probably cache synchronization issue on SH7723

[Paul Mundt]

On Thu, Dec 03, 2009 at 09:45:28PM +0900, Magnus Damm wrote:
> On Thu, Dec 3, 2009 at 9:36 PM, Markus Pietrek
> <Markus.Pietrek@emtrion.de> wrote:
> > when starting bigger graphical applications they suddenly stop with a
> > segmentation fault or a bus error. If I execute the application a second
> > time, everything is fine. Applications that are linked to shared libraries
> > are more likely to fail than statically linked applications.
> >
> > Changing the cache from write-back to write-through fixes it. Maybe the
> > instruction cache and the data cache are not synchronized correctly?
> >
> > I didn't have this problem with the linux-2.6.29 vanilla kernel. 2.6.32rc2
> > from git://git.kernel.org/pub/scm/linux/kernel/git/lethal/sh-2.6 introduced
> > it. And after a pull yesterday even /bin/init is failing.
> 
> I've found that sh7722 and sh7724 doesn't work with copy back cache
> right now, see below. There may be other issues as well, but going
> back to OK fixes it for my initramfs-only case.
> 
I've reverted the problematic parts of this patch for now, so that should
fix up these cases. There's an outstanding issue addressed by Matt's
patch that is being debugged now and which I will push once the remaining
faults on SH7724 have been resolved.

In any event, these changes are bound for 2.6.33, so stock 2.6.32 should
never have been impacted by any of this, and the remaining issues should
be sorted before 2.6.33-rc1. We'll give it another spin for 2.6.34.

AW: Probably cache synchronization issue on SH7723

[Pietrek, Markus]

Hi,

Lacking some understanding of the caching system I'm not sure why in update_cache() some dirty pages are skipped. The following patch fixes my problem, but I'm not sure about any negative side effects.


diff --git a/arch/sh/mm/cache.c b/arch/sh/mm/cache.c
index e9415d3..30c3d7e 100644
--- a/arch/sh/mm/cache.c
+++ b/arch/sh/mm/cache.c
@@ -136,8 +136,7 @@ void __update_cache(struct vm_area_struct *vma,
                if (dirty) {
                        unsigned long addr = (unsigned long)page_address(page);

-                       if (pages_do_alias(addr, address & PAGE_MASK))
-                               __flush_purge_region((void *)addr, PAGE_SIZE);
+                        __flush_purge_region((void *)addr, PAGE_SIZE);
                }
        }
 }

Best regards,
Markus Pietrek

-----Ursprüngliche Nachricht-----
Von: linux-sh-owner@vger.kernel.org [mailto:linux-sh-owner@vger.kernel.org] Im Auftrag von Paul Mundt
Gesendet: Freitag, 4. Dezember 2009 07:48
An: Magnus Damm
Cc: Pietrek, Markus; linux-sh@vger.kernel.org
Betreff: Re: Probably cache synchronization issue on SH7723

On Thu, Dec 03, 2009 at 09:45:28PM +0900, Magnus Damm wrote:
> On Thu, Dec 3, 2009 at 9:36 PM, Markus Pietrek
> <Markus.Pietrek@emtrion.de> wrote:
> > when starting bigger graphical applications they suddenly stop with a
> > segmentation fault or a bus error. If I execute the application a second
> > time, everything is fine. Applications that are linked to shared libraries
> > are more likely to fail than statically linked applications.
> >
> > Changing the cache from write-back to write-through fixes it. Maybe the
> > instruction cache and the data cache are not synchronized correctly?
> >
> > I didn't have this problem with the linux-2.6.29 vanilla kernel. 2.6.32rc2
> > from git://git.kernel.org/pub/scm/linux/kernel/git/lethal/sh-2.6 introduced
> > it. And after a pull yesterday even /bin/init is failing.
>
> I've found that sh7722 and sh7724 doesn't work with copy back cache
> right now, see below. There may be other issues as well, but going
> back to OK fixes it for my initramfs-only case.
>
I've reverted the problematic parts of this patch for now, so that should
fix up these cases. There's an outstanding issue addressed by Matt's
patch that is being debugged now and which I will push once the remaining
faults on SH7724 have been resolved.

In any event, these changes are bound for 2.6.33, so stock 2.6.32 should
never have been impacted by any of this, and the remaining issues should
be sorted before 2.6.33-rc1. We'll give it another spin for 2.6.34.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

_____________________________________

Amtsgericht Mannheim
HRB 110 300
Geschäftsführer: Dieter Baur, Ramona Maurer
_____________________________________

Important Note:
- This e-mail may contain trade secrets or privileged, undisclosed or otherwise confidential information.
- If you have received this e-mail in error, you are hereby notified that any review, copying or distribution of it is strictly prohibited.
- Please inform us immediately and destroy the original transmittal.

Thank you for your cooperation.

Re: Probably cache synchronization issue on SH7723

[Matt Fleming]

On Tue, Dec 22, 2009 at 05:18:16PM +0100, Pietrek, Markus wrote:
> Hi, 
>
> Lacking some understanding of the caching system I'm not sure why in
> update_cache() some dirty pages are skipped. The following patch fixes
> my problem, but I'm not sure about any negative side effects.

Interesting! Good investigative work.

It's an optimisation so that we don't flush the dcache more than we need
to. There shouldn't be any negative side effects of your patch, it's
just a bit inefficient.

Having looked at the MIPS __update_cache() function I'm wondering if the
bug is that we should be flushing the cache if the page is executable.

Could you try this patch?

diff --git a/arch/sh/mm/cache.c b/arch/sh/mm/cache.c
index e9415d3..8460bbf 100644
--- a/arch/sh/mm/cache.c
+++ b/arch/sh/mm/cache.c
@@ -126,8 +126,9 @@ void __update_cache(struct vm_area_struct *vma,
 {
 	struct page *page;
 	unsigned long pfn = pte_pfn(pte);
+	int exec = (vma->vm_flags & VM_EXEC);
 
-	if (!boot_cpu_data.dcache.n_aliases)
+	if (!boot_cpu_data.dcache.n_aliases && !exec)
 		return;
 
 	page = pfn_to_page(pfn);
@@ -136,7 +137,7 @@ void __update_cache(struct vm_area_struct *vma,
 		if (dirty) {
 			unsigned long addr = (unsigned long)page_address(page);
 
-			if (pages_do_alias(addr, address & PAGE_MASK))
+			if (exec || pages_do_alias(addr, address & PAGE_MASK))
 				__flush_purge_region((void *)addr, PAGE_SIZE);
 		}
 	}

AW: Probably cache synchronization issue on SH7723

[Pietrek, Markus]

Hi Matt,

update_mmu_cache() is called with vma = NULL in arch/sh/mm/fault_32.c. If we then assume exec=1, the patch works fine.

diff --git a/arch/sh/mm/cache.c b/arch/sh/mm/cache.c
index e9415d3..054bb8a 100644
--- a/arch/sh/mm/cache.c
+++ b/arch/sh/mm/cache.c
@@ -126,8 +126,10 @@ void __update_cache(struct vm_area_struct *vma,
 {
        struct page *page;
        unsigned long pfn = pte_pfn(pte);
+       /* flush any dirty data page before using it as a code page */
+       int exec = (vma != NULL) ? (vma->vm_flags & VM_EXEC) : 1;

-       if (!boot_cpu_data.dcache.n_aliases)
+       if (!boot_cpu_data.dcache.n_aliases && !exec)
                return;

        page = pfn_to_page(pfn);
@@ -136,7 +138,7 @@ void __update_cache(struct vm_area_struct *vma,
                if (dirty) {
                        unsigned long addr = (unsigned long)page_address(page);

-                       if (pages_do_alias(addr, address & PAGE_MASK))
+                       if (exec || pages_do_alias(addr, address & PAGE_MASK))
                                __flush_purge_region((void *)addr, PAGE_SIZE);
                }
        }

Best regards,
Markus Pietrek

-----Ursprüngliche Nachricht-----
Von: Matt Fleming [mailto:matt@console-pimps.org]
Gesendet: Dienstag, 22. Dezember 2009 17:52
An: Pietrek, Markus
Cc: linux-sh@vger.kernel.org
Betreff: Re: Probably cache synchronization issue on SH7723

On Tue, Dec 22, 2009 at 05:18:16PM +0100, Pietrek, Markus wrote:
> Hi,
>
> Lacking some understanding of the caching system I'm not sure why in
> update_cache() some dirty pages are skipped. The following patch fixes
> my problem, but I'm not sure about any negative side effects.

Interesting! Good investigative work.

It's an optimisation so that we don't flush the dcache more than we need
to. There shouldn't be any negative side effects of your patch, it's
just a bit inefficient.

Having looked at the MIPS __update_cache() function I'm wondering if the
bug is that we should be flushing the cache if the page is executable.

Could you try this patch?

diff --git a/arch/sh/mm/cache.c b/arch/sh/mm/cache.c
index e9415d3..8460bbf 100644
--- a/arch/sh/mm/cache.c
+++ b/arch/sh/mm/cache.c
@@ -126,8 +126,9 @@ void __update_cache(struct vm_area_struct *vma,
 {
        struct page *page;
        unsigned long pfn = pte_pfn(pte);
+       int exec = (vma->vm_flags & VM_EXEC);

-       if (!boot_cpu_data.dcache.n_aliases)
+       if (!boot_cpu_data.dcache.n_aliases && !exec)
                return;

        page = pfn_to_page(pfn);
@@ -136,7 +137,7 @@ void __update_cache(struct vm_area_struct *vma,
                if (dirty) {
                        unsigned long addr = (unsigned long)page_address(page);

-                       if (pages_do_alias(addr, address & PAGE_MASK))
+                       if (exec || pages_do_alias(addr, address & PAGE_MASK))
                                __flush_purge_region((void *)addr, PAGE_SIZE);
                }
        }


_____________________________________

Amtsgericht Mannheim
HRB 110 300
Geschäftsführer: Dieter Baur, Ramona Maurer
_____________________________________

Important Note:
- This e-mail may contain trade secrets or privileged, undisclosed or otherwise confidential information.
- If you have received this e-mail in error, you are hereby notified that any review, copying or distribution of it is strictly prohibited.
- Please inform us immediately and destroy the original transmittal.

Thank you for your cooperation.

Re: Probably cache synchronization issue on SH7723

[Matt Fleming]

On Wed, Dec 23, 2009 at 09:38:10AM +0100, Pietrek, Markus wrote:
> Hi Matt,
> 
> update_mmu_cache() is called with vma = NULL in
> arch/sh/mm/fault_32.c. If we then assume exec=1, the patch works
> fine.
> 
> diff --git a/arch/sh/mm/cache.c b/arch/sh/mm/cache.c
> index e9415d3..054bb8a 100644
> --- a/arch/sh/mm/cache.c
> +++ b/arch/sh/mm/cache.c
> @@ -126,8 +126,10 @@ void __update_cache(struct vm_area_struct *vma,
>  {
>         struct page *page;
>         unsigned long pfn = pte_pfn(pte);
> +       /* flush any dirty data page before using it as a code page */
> +       int exec = (vma != NULL) ? (vma->vm_flags & VM_EXEC) : 1;
> 

I'm assuming that if you set exec to 0 when vma = NULL then you still
see the cache issues? In which case, this patch probably works because
it's aggressively flushing the cache and unfortunately will be papering
over the real bug.

What sort of workload do you have that triggers this bug? Did you say it
was a graphics app? Is the app available anywhere? If I could reproduce
it on my board it would make it easier for me to diagnose the bug.

If we can't find the cause of this bug soon I imagine that Paul will
queue up some band-aid patch so that, whilst there will be a performance
loss, at least things will work correctly ;-)

Re: Probably cache synchronization issue on SH7723

[Paul Mundt]

On Wed, Dec 23, 2009 at 11:33:08PM +0000, Matt Fleming wrote:
> On Wed, Dec 23, 2009 at 09:38:10AM +0100, Pietrek, Markus wrote:
> > Hi Matt,
> > 
> > update_mmu_cache() is called with vma = NULL in
> > arch/sh/mm/fault_32.c. If we then assume exec=1, the patch works
> > fine.
> > 
> > diff --git a/arch/sh/mm/cache.c b/arch/sh/mm/cache.c
> > index e9415d3..054bb8a 100644
> > --- a/arch/sh/mm/cache.c
> > +++ b/arch/sh/mm/cache.c
> > @@ -126,8 +126,10 @@ void __update_cache(struct vm_area_struct *vma,
> >  {
> >         struct page *page;
> >         unsigned long pfn = pte_pfn(pte);
> > +       /* flush any dirty data page before using it as a code page */
> > +       int exec = (vma != NULL) ? (vma->vm_flags & VM_EXEC) : 1;
> > 
> 
> I'm assuming that if you set exec to 0 when vma = NULL then you still
> see the cache issues? In which case, this patch probably works because
> it's aggressively flushing the cache and unfortunately will be papering
> over the real bug.
> 
> What sort of workload do you have that triggers this bug? Did you say it
> was a graphics app? Is the app available anywhere? If I could reproduce
> it on my board it would make it easier for me to diagnose the bug.
> 
> If we can't find the cause of this bug soon I imagine that Paul will
> queue up some band-aid patch so that, whilst there will be a performance
> loss, at least things will work correctly ;-)

Well, the aliasing test was added as an optimization, but dropping it and
doing the flush on any PG_dcache_dirty hit is fine too. The exec case is
a bit misleading as the purge in question only touches the D-cache lines,
whilst the VM_EXEC case needs I-cache tidying. This is further
complicated by the fact that I-cache tidying may be required regardless
of whether there is a page mapping or not. However, that's a largely
secondary issue. If the crashes observed go away with more aggressive
D-cache flushing, then it's not likely to be exec related given that the
I-cache remains unaltered.

So, I'm inclined to just apply Markus's original patch that drops the
pages_do_alias() test and restores the old behaviour of just aggressively
flushing in the PG_dcache_dirty case. If there's a reproduceable testcase
for circumventing the aliasing test then obviously that's worth looking
in to for future optimizations.

AW: Probably cache synchronization issue on SH7723

[Pietrek, Markus]

Hi Matt,

> I'm assuming that if you set exec to 0 when vma = NULL then you still
> see the cache issues? In which case, this patch probably works because
> it's aggressively flushing the cache and unfortunately will be papering
> over the real bug.

With exec = 0, the cache issues are still present.

> What sort of workload do you have that triggers this bug? Did you say
> it
> was a graphics app? Is the app available anywhere? If I could reproduce
> it on my board it would make it easier for me to diagnose the bug.

The bug happens mostly on graphic applications like ts_test from the touch library libts or any Qt based application. There is a way to increase the probability to appear to almost 100% on my HiCO.DIMM7723 board when using nfsroot.

When booting by network, a lot of shared libraries have already been loaded before I can launch ts_test. So their pages might already been synchronized in the instruction/data cache. But if I mount the rootfs a second time and chroot into it and then start the application, it always crashes. The next calls might crash, but the third and later calls always works. To see the crash a second time I need to unmount the filesystem and mount it again.

BusyBox v1.12.4 (2009-12-09 12:43:33 CET) built-in shell (ash)
Enter 'help' for a list of built-in commands.

~ # mount -o nolock 192.168.2.11:/nfsroot/hico7723/rootfs /mnt
~ # chroot /mnt
~ # mount -a; mdev -s; . /etc/profile._tslib
BusyBox v1.12.4 (2009-12-09 12:43:33 CET) built-in shell (ash)
Enter 'help' for a list of built-in commands.
~ # ts_test
Segmentation fault
~ # ts_test
Illegal instruction
~ # ts_test
^Csignal 2 caught
~ # exit
~ # umount /mnt/proc; umount /mnt/sys; umount /mnt/tmp; umount /mnt/mnt; umount
/mnt/var/run; umount /mnt/var/log; umount /mnt
~ # mount -o nolock 192.168.2.11:/nfsroot/hico7723/rootfs /mnt
~ # chroot /mnt
~ # mount -a; mdev -s; . /etc/profile._tslib
BusyBox v1.12.4 (2009-12-09 12:43:33 CET) built-in shell (ash)
Enter 'help' for a list of built-in commands.
~ # ts_test
Inconsistency detected by ld.so: dynamic-link.h: 165: elf_get_dynamic_info: Assertion `info[9]->d_un.d_val = sizeof (Elf32_Rela)' failed!
~ # ts_test
^Csignal 2 caught
~ #


> If we can't find the cause of this bug soon I imagine that Paul will
> queue up some band-aid patch so that, whilst there will be a
> performance
> loss, at least things will work correctly ;-)

The loss might not be as big as having to use the cache in write-through mode ;-)



_____________________________________

Amtsgericht Mannheim
HRB 110 300
Gesch?ftsf?hrer: Dieter Baur, Ramona Maurer
_____________________________________

Important Note:
- This e-mail may contain trade secrets or privileged, undisclosed or otherwise confidential information.
- If you have received this e-mail in error, you are hereby notified that any review, copying or distribution of it is strictly prohibited.
- Please inform us immediately and destroy the original transmittal.

Thank you for your cooperation.

Re: Probably cache synchronization issue on SH7723

[Paul Mundt]

On Thu, Dec 24, 2009 at 10:50:31AM +0100, Pietrek, Markus wrote:
> > If we can't find the cause of this bug soon I imagine that Paul will
> > queue up some band-aid patch so that, whilst there will be a
> > performance loss, at least things will work correctly ;-)
> 
> The loss might not be as big as having to use the cache in write-through mode ;-)
> 
I pushed out a reworked version of your patch, so give current git a try
and see if that fixes up your remaining issues. These changes will be
reflected in 2.6.33-rc2.