[PATCH] parisc: improve vmap flush/invalidate

[PATCH] parisc: improve vmap flush/invalidate

[James Bottomley]

On parisc, we never implemented invalidate_kernel_vmap_range() because
it was unnecessary for the xfs use case.  However, we do need to
implement an invalidate for the opposite use case (which occurred in a
recent NFS change) where the user wants to read through the vmap range
and write via the kernel address.  There's an additional complexity to
this in that if the page has no userspace mappings, it might have dirty
cache lines in the kernel (indicated by the PG_dcache_dirty bit).  In
order to get full coherency, we need to flush these pages through the
kernel mapping before invalidating the vmap range.

James

---

diff --git a/arch/parisc/include/asm/cacheflush.h b/arch/parisc/include/asm/cacheflush.h
index f388a85..df6c56d 100644
--- a/arch/parisc/include/asm/cacheflush.h
+++ b/arch/parisc/include/asm/cacheflush.h
@@ -37,6 +37,13 @@ void flush_cache_all_local(void);
 void flush_cache_all(void);
 void flush_cache_mm(struct mm_struct *mm);
 
+#define ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE
+void flush_kernel_dcache_page_addr(void *addr);
+static inline void flush_kernel_dcache_page(struct page *page)
+{
+	flush_kernel_dcache_page_addr(page_address(page));
+}
+
 #define flush_kernel_dcache_range(start,size) \
 	flush_kernel_dcache_range_asm((start), (start)+(size));
 /* vmap range flushes and invalidates.  Architecturally, we don't need
@@ -50,6 +57,16 @@ static inline void flush_kernel_vmap_range(void *vaddr, int size)
 }
 static inline void invalidate_kernel_vmap_range(void *vaddr, int size)
 {
+	unsigned long start = (unsigned long)vaddr;
+	void *cursor = vaddr;
+
+	for ( ; cursor < vaddr + size; cursor += PAGE_SIZE) {
+		struct page *page = vmalloc_to_page(cursor);
+
+		if (test_and_clear_bit(PG_dcache_dirty, &page->flags))
+			flush_kernel_dcache_page(page);
+	}
+	flush_kernel_dcache_range_asm(start, start + size);
 }
 
 #define flush_cache_vmap(start, end)		flush_cache_all()
@@ -98,13 +115,6 @@ flush_anon_page(struct vm_area_struct *vma, struct page *page, unsigned long vma
 		flush_user_dcache_page(vmaddr);
 }
 
-#define ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE
-void flush_kernel_dcache_page_addr(void *addr);
-static inline void flush_kernel_dcache_page(struct page *page)
-{
-	flush_kernel_dcache_page_addr(page_address(page));
-}

[PATCH] parisc: Improve dcache flush on PA8800/PA8900

[John David Anglin]

On Thu, 20 Jan 2011, James Bottomley wrote:

> On parisc, we never implemented invalidate_kernel_vmap_range() because
> it was unnecessary for the xfs use case.  However, we do need to
> implement an invalidate for the opposite use case (which occurred in a
> recent NFS change) where the user wants to read through the vmap range
> and write via the kernel address.  There's an additional complexity to
> this in that if the page has no userspace mappings, it might have dirty
> cache lines in the kernel (indicated by the PG_dcache_dirty bit).  In
> order to get full coherency, we need to flush these pages through the
> kernel mapping before invalidating the vmap range.

The attached patch greatly improves SMP stability on my rp3440.  I
have now done several full GCC builds with make -j4 without any random
segmentation faults.  This is with the parisc next-for branch, built
with gcc version 4.5.3 20110101 (prerelease) and binutils 2.21.51.20110108.

The change includes James' invalidate_kernel_vmap_range patch which
as of last night, doesn't seem to be in linux-next..

Notes:

1) Changed spin_lock_irq to spin_lock in flush_dcache_mmap_lock because
   the former causes a problem with timer interrupts.
2) Changed update_mmu_cache to return immediately if the PFN is not valid.
   In testing, I found there was always a mapping, so this test was
   eliminated.  I also changed to using test_and_clear_bit as on arm.
3) Added a higher level lock to flush_dcache_page as I found in testing
   that the user mappings appeared to change while the code iterated
   over the user mappings.  Updated comment to better reflect the
   current status WRT inequivalent aliases.  Only updated old_addr
   for shared mappings.  This was simply to suppress the number of
   "INEQUIVALENT" messages.  As noted previously, too many causes the
   console getty process to stop working.  I think it may be possible
   to further optimize this loop.
4) Changed arch_get_unmapped_area to enforce alignment of shared
   MAP_FIXED regions as on arm.

Signed-off-by: John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

diff --git a/arch/parisc/include/asm/cacheflush.h b/arch/parisc/include/asm/cacheflush.h
index dc9286a..d5f1631 100644
--- a/arch/parisc/include/asm/cacheflush.h
+++ b/arch/parisc/include/asm/cacheflush.h
@@ -35,6 +35,13 @@ void flush_cache_all_local(void);
 void flush_cache_all(void);
 void flush_cache_mm(struct mm_struct *mm);
 
+#define ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE
+void flush_kernel_dcache_page_addr(void *addr);
+static inline void flush_kernel_dcache_page(struct page *page)
+{
+	flush_kernel_dcache_page_addr(page_address(page));
+}
+
 #define flush_kernel_dcache_range(start,size) \
 	flush_kernel_dcache_range_asm((start), (start)+(size));
 /* vmap range flushes and invalidates.  Architecturally, we don't need
@@ -48,6 +55,16 @@ static inline void flush_kernel_vmap_range(void *vaddr, int size)
 }
 static inline void invalidate_kernel_vmap_range(void *vaddr, int size)
 {
+	unsigned long start = (unsigned long)vaddr;
+	void *cursor = vaddr;
+
+	for ( ; cursor < vaddr + size; cursor += PAGE_SIZE) {
+		struct page *page = vmalloc_to_page(cursor);
+
+		if (test_and_clear_bit(PG_dcache_dirty, &page->flags))
+			flush_kernel_dcache_page(page);
+	}
+	flush_kernel_dcache_range_asm(start, start + size);
 }
 
 #define flush_cache_vmap(start, end)		flush_cache_all()
@@ -57,9 +74,9 @@ static inline void invalidate_kernel_vmap_range(void *vaddr, int size)
 extern void flush_dcache_page(struct page *page);
 
 #define flush_dcache_mmap_lock(mapping) \
-	spin_lock_irq(&(mapping)->tree_lock)
+	spin_lock(&(mapping)->tree_lock)
 #define flush_dcache_mmap_unlock(mapping) \
-	spin_unlock_irq(&(mapping)->tree_lock)
+	spin_unlock(&(mapping)->tree_lock)
 
 #define flush_icache_page(vma,page)	do { 		\
 	flush_kernel_dcache_page(page);			\
@@ -99,13 +116,6 @@ flush_anon_page(struct vm_area_struct *vma, struct page *page, unsigned long vma
 		flush_dcache_page_asm(page_to_phys(page), vmaddr);
 }
 
-#define ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE
-void flush_kernel_dcache_page_addr(void *addr);
-static inline void flush_kernel_dcache_page(struct page *page)
-{
-	flush_kernel_dcache_page_addr(page_address(page));
-}
-
 #ifdef CONFIG_DEBUG_RODATA
 void mark_rodata_ro(void);
 #endif
diff --git a/arch/parisc/kernel/cache.c b/arch/parisc/kernel/cache.c
index 3f11331..fa92dcb 100644
--- a/arch/parisc/kernel/cache.c
+++ b/arch/parisc/kernel/cache.c
@@ -75,14 +75,16 @@ EXPORT_SYMBOL(flush_cache_all_local);
 void
 update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t *ptep)
 {
-	struct page *page = pte_page(*ptep);
+        unsigned long pfn = pte_pfn(*ptep);
+        struct page *page;
 
-	if (pfn_valid(page_to_pfn(page)) && page_mapping(page) &&
-	    test_bit(PG_dcache_dirty, &page->flags)) {
+        if (!pfn_valid(pfn))
+                return;
 
-		flush_kernel_dcache_page(page);
-		clear_bit(PG_dcache_dirty, &page->flags);
-	} else if (parisc_requires_coherency())
+        page = pfn_to_page(pfn);
+
+        if (test_and_clear_bit(PG_dcache_dirty, &page->flags)
+	    || parisc_requires_coherency())
 		flush_kernel_dcache_page(page);
 }
 
@@ -295,10 +297,17 @@ void flush_dcache_page(struct page *page)
 	pgoff = page->index << (PAGE_CACHE_SHIFT - PAGE_SHIFT);
 
 	/* We have carefully arranged in arch_get_unmapped_area() that
-	 * *any* mappings of a file are always congruently mapped (whether
-	 * declared as MAP_PRIVATE or MAP_SHARED), so we only need
-	 * to flush one address here for them all to become coherent */
-
+	 * all shared mappings of a file are congruently mapped, so we
+	 * only need to flush one address here for them all to become
+	 * coherent.  However, non shared fixed mappings of executables
+	 * are not congruently mapped on the boundary page between text
+	 * and data.  Further, the data segment sometimes occurs before
+	 * the text segment.  While it is unlikely that a dirty cache
+	 * line would result from accesses through the text mapping,
+	 * it is possible that this could occur since binutils doesn't
+	 * ensure that the data segment starts on a page boundary.  */
+
+	spin_lock(&mapping->i_mmap_lock);
 	flush_dcache_mmap_lock(mapping);
 	vma_prio_tree_foreach(mpnt, &iter, &mapping->i_mmap, pgoff, pgoff) {
 		offset = (pgoff - mpnt->vm_pgoff) << PAGE_SHIFT;
@@ -307,11 +316,13 @@ void flush_dcache_page(struct page *page)
 		if (old_addr == 0 || (old_addr & (SHMLBA - 1)) != (addr & (SHMLBA - 1))) {
 			__flush_cache_page(mpnt, addr, page_to_phys(page));
 			if (old_addr)
-				printk(KERN_ERR "INEQUIVALENT ALIASES 0x%lx and 0x%lx in file %s\n", old_addr, addr, mpnt->vm_file ? mpnt->vm_file->f_path.dentry->d_name.name : "(null)");
-			old_addr = addr;
+				printk(KERN_ERR "INEQUIVALENT ALIASES 0x%lx and 0x%lx in file %s\n", old_addr, addr, mpnt->vm_file ? (char *) mpnt->vm_file->f_path.dentry->d_name.name : "(null)");
+			if (mpnt->vm_flags & VM_SHARED)
+				old_addr = addr;
 		}
 	}
 	flush_dcache_mmap_unlock(mapping);
+	spin_unlock(&mapping->i_mmap_lock);
 }
 EXPORT_SYMBOL(flush_dcache_page);
 
diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c
index c9b9322..f0cb56e 100644
--- a/arch/parisc/kernel/sys_parisc.c
+++ b/arch/parisc/kernel/sys_parisc.c
@@ -92,11 +92,12 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
 {
 	if (len > TASK_SIZE)
 		return -ENOMEM;
-	/* Might want to check for cache aliasing issues for MAP_FIXED case
-	 * like ARM or MIPS ??? --BenH.
-	 */
-	if (flags & MAP_FIXED)
+	if (flags & MAP_FIXED) {
+		if ((flags & MAP_SHARED) &&
+		    (addr - (pgoff << PAGE_SHIFT)) & (SHMLBA - 1))
+			return -EINVAL;
 		return addr;
+	}
 	if (!addr)
 		addr = TASK_UNMAPPED_BASE;
 

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[James Bottomley]

On Tue, 2011-02-08 at 12:15 -0500, John David Anglin wrote:
> On Thu, 20 Jan 2011, James Bottomley wrote:
> 
> > On parisc, we never implemented invalidate_kernel_vmap_range() because
> > it was unnecessary for the xfs use case.  However, we do need to
> > implement an invalidate for the opposite use case (which occurred in a
> > recent NFS change) where the user wants to read through the vmap range
> > and write via the kernel address.  There's an additional complexity to
> > this in that if the page has no userspace mappings, it might have dirty
> > cache lines in the kernel (indicated by the PG_dcache_dirty bit).  In
> > order to get full coherency, we need to flush these pages through the
> > kernel mapping before invalidating the vmap range.
> 
> The attached patch greatly improves SMP stability on my rp3440.  I
> have now done several full GCC builds with make -j4 without any random
> segmentation faults.  This is with the parisc next-for branch, built
> with gcc version 4.5.3 20110101 (prerelease) and binutils 2.21.51.20110108.
> 
> The change includes James' invalidate_kernel_vmap_range patch which
> as of last night, doesn't seem to be in linux-next..

doing this makes it a bit harder to see what you're changing.  It also
can't possibly affect your testing, because it's an edge case for xfs
journals currently.

> Notes:
> 
> 1) Changed spin_lock_irq to spin_lock in flush_dcache_mmap_lock because
>    the former causes a problem with timer interrupts.

This looks reasonable ... architectures which don't use the lock do
this.  Unfortunately, flush_dcache_page can be called from interrupt
context, so our only choices are to eliminate the lock or to use it in
an interrupt safe manner.  If we use a non interrupt safe lock, it will
cause a deadlock eventually.

> 2) Changed update_mmu_cache to return immediately if the PFN is not valid.
>    In testing, I found there was always a mapping, so this test was
>    eliminated.  I also changed to using test_and_clear_bit as on arm.

This is just an optimisation, so I'm fine with it.

> 3) Added a higher level lock to flush_dcache_page as I found in testing
>    that the user mappings appeared to change while the code iterated
>    over the user mappings.  Updated comment to better reflect the
>    current status WRT inequivalent aliases.  Only updated old_addr
>    for shared mappings.  This was simply to suppress the number of
>    "INEQUIVALENT" messages.  As noted previously, too many causes the
>    console getty process to stop working.  I think it may be possible
>    to further optimize this loop.

So this is a big problem.  The flush_dcache_mmap_lock/unlock is supposed
to be our protection against this.  If that's not working the generic
code is broken.  But I'm not clear what you think is changing.  The lock
protects the mapping prio tree ... that can't change as we iterate
otherwise we'll walk off a stale entry and go boom.  The actual status
of the mappings is allowed to change.

> 4) Changed arch_get_unmapped_area to enforce alignment of shared
>    MAP_FIXED regions as on arm.

This one looks good.

James

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[John David Anglin]

> > The change includes James' invalidate_kernel_vmap_range patch which
> > as of last night, doesn't seem to be in linux-next..
> 
> doing this makes it a bit harder to see what you're changing.  It also
> can't possibly affect your testing, because it's an edge case for xfs
> journals currently.

I'll pull that part out and retest.

> > 1) Changed spin_lock_irq to spin_lock in flush_dcache_mmap_lock because
> >    the former causes a problem with timer interrupts.
> 
> This looks reasonable ... architectures which don't use the lock do
> this.  Unfortunately, flush_dcache_page can be called from interrupt
> context, so our only choices are to eliminate the lock or to use it in
> an interrupt safe manner.  If we use a non interrupt safe lock, it will
> cause a deadlock eventually.

I see occassional slow timer interrupt messages and I suspect the expect
timeouts that I see during the GCC testsuite are also caused by disabling
interrupts for too long.  I also encountered situations in the GCC
build where certain applications (gnat1) would run much longer than
normal (seemed like scheduling was broken).  At the smae time, xntpd
seemed to be consuming more cycles than normal.

Arm has some checks in __flush_dcache_aliase that may eliminate some
flushes and reduce the time interrupts are disabled in flush_dcache_page,
but I don't trust the VM_MAYSHARE one.

> > 3) Added a higher level lock to flush_dcache_page as I found in testing
> >    that the user mappings appeared to change while the code iterated
> >    over the user mappings.  Updated comment to better reflect the
> >    current status WRT inequivalent aliases.  Only updated old_addr
> >    for shared mappings.  This was simply to suppress the number of
> >    "INEQUIVALENT" messages.  As noted previously, too many causes the
> >    console getty process to stop working.  I think it may be possible
> >    to further optimize this loop.
> 
> So this is a big problem.  The flush_dcache_mmap_lock/unlock is supposed
> to be our protection against this.  If that's not working the generic
> code is broken.  But I'm not clear what you think is changing.  The lock
> protects the mapping prio tree ... that can't change as we iterate
> otherwise we'll walk off a stale entry and go boom.  The actual status
> of the mappings is allowed to change.

Specifically, what led me to assume that there was an issue with the
code as is was various "INEQUIVALENT" messages with a difference between
old_addr and addr of 0x150000.  This is the only difference other
than +/- 0x1000 that I have seen.  The GCC libmudflap testsuite triggers
a number of these odd messages.  Here's some samples.

Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x4014b000 and 0x4014a000 in file ld-2.11.2.so
Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406b8000 and 0x40568000 in file libc-2.11.2.so
Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406b9000 and 0x40569000 in file libc-2.11.2.so
Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406ba000 and 0x4056a000 in file libc-2.11.2.so
Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406bb000 and 0x4056b000 in file libc-2.11.2.so
Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406bc000 and 0x4056c000 in file libc-2.11.2.so
Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406bd000 and 0x4056d000 in file libc-2.11.2.so
Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406be000 and 0x4056e000 in file libc-2.11.2.so
Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406bf000 and 0x4056f000 in file libc-2.11.2.so
Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406c1000 and 0x406c0000 in file libc-2.11.2.so

The messages occur because there is a core dump when the test is run is
run by the expect/dejagnu framework.  I think all the messages above
were generated by a single core dump.  The test doesn't fail when I compile
and run manually.  From the above, I had the sense that the addresses
were changing while the loop while it was being iterated.  With the extra
lock, I haven't seen this. 

Possibly, the answer is to disable interrupts at the &mapping->i_mmap_lock
lock, and skip VMAs if they are not in current mm.  What do you think?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[James Bottomley]

On Tue, 2011-02-08 at 13:45 -0500, John David Anglin wrote:
> > > The change includes James' invalidate_kernel_vmap_range patch which
> > > as of last night, doesn't seem to be in linux-next..
> > 
> > doing this makes it a bit harder to see what you're changing.  It also
> > can't possibly affect your testing, because it's an edge case for xfs
> > journals currently.
> 
> I'll pull that part out and retest.
> 
> > > 1) Changed spin_lock_irq to spin_lock in flush_dcache_mmap_lock because
> > >    the former causes a problem with timer interrupts.
> > 
> > This looks reasonable ... architectures which don't use the lock do
> > this.  Unfortunately, flush_dcache_page can be called from interrupt
> > context, so our only choices are to eliminate the lock or to use it in
> > an interrupt safe manner.  If we use a non interrupt safe lock, it will
> > cause a deadlock eventually.
> 
> I see occassional slow timer interrupt messages and I suspect the expect
> timeouts that I see during the GCC testsuite are also caused by disabling
> interrupts for too long.  I also encountered situations in the GCC
> build where certain applications (gnat1) would run much longer than
> normal (seemed like scheduling was broken).  At the smae time, xntpd
> seemed to be consuming more cycles than normal.

Right, the problem here is the inequivalent aliases I picked up in
testing.  To flush them I loop over all the vma's prio tree.  For a vma
shared by a lot of processes (like the ones in glibc) that's a pretty
large tree traversal.  Once we're sure we don't get inequivalent
aliases, I can apply the patch below and the lock will be held for a
minute fraction of what it was previously.


> Arm has some checks in __flush_dcache_aliase that may eliminate some
> flushes and reduce the time interrupts are disabled in flush_dcache_page,
> but I don't trust the VM_MAYSHARE one.
> 
> > > 3) Added a higher level lock to flush_dcache_page as I found in testing
> > >    that the user mappings appeared to change while the code iterated
> > >    over the user mappings.  Updated comment to better reflect the
> > >    current status WRT inequivalent aliases.  Only updated old_addr
> > >    for shared mappings.  This was simply to suppress the number of
> > >    "INEQUIVALENT" messages.  As noted previously, too many causes the
> > >    console getty process to stop working.  I think it may be possible
> > >    to further optimize this loop.
> > 
> > So this is a big problem.  The flush_dcache_mmap_lock/unlock is supposed
> > to be our protection against this.  If that's not working the generic
> > code is broken.  But I'm not clear what you think is changing.  The lock
> > protects the mapping prio tree ... that can't change as we iterate
> > otherwise we'll walk off a stale entry and go boom.  The actual status
> > of the mappings is allowed to change.
> 
> Specifically, what led me to assume that there was an issue with the
> code as is was various "INEQUIVALENT" messages with a difference between
> old_addr and addr of 0x150000.  This is the only difference other
> than +/- 0x1000 that I have seen.  The GCC libmudflap testsuite triggers
> a number of these odd messages.  Here's some samples.
> 
> Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x4014b000 and 0x4014a000 in file ld-2.11.2.so
> Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406b8000 and 0x40568000 in file libc-2.11.2.so
> Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406b9000 and 0x40569000 in file libc-2.11.2.so
> Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406ba000 and 0x4056a000 in file libc-2.11.2.so
> Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406bb000 and 0x4056b000 in file libc-2.11.2.so
> Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406bc000 and 0x4056c000 in file libc-2.11.2.so
> Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406bd000 and 0x4056d000 in file libc-2.11.2.so
> Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406be000 and 0x4056e000 in file libc-2.11.2.so
> Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406bf000 and 0x4056f000 in file libc-2.11.2.so
> Feb  6 04:50:56 mx3210 kernel: INEQUIVALENT ALIASES 0x406c1000 and 0x406c0000 in file libc-2.11.2.so
> 
> The messages occur because there is a core dump when the test is run is
> run by the expect/dejagnu framework.  I think all the messages above
> were generated by a single core dump.  The test doesn't fail when I compile
> and run manually.  From the above, I had the sense that the addresses
> were changing while the loop while it was being iterated.  With the extra
> lock, I haven't seen this. 
> 
> Possibly, the answer is to disable interrupts at the &mapping->i_mmap_lock
> lock, and skip VMAs if they are not in current mm.  What do you think?

They already are distabled by the flush_dcache_mmap_lock

Can we assume these addresses are some type of artifact and just apply
the patch below anyway?  On current parisc, it's what we do (we don't
flush anything beyond the first mapping).

James

---

diff --git a/arch/parisc/kernel/cache.c b/arch/parisc/kernel/cache.c
index 3f11331..c60cc42 100644
--- a/arch/parisc/kernel/cache.c
+++ b/arch/parisc/kernel/cache.c
@@ -276,10 +276,13 @@ __flush_cache_page(struct vm_area_struct *vma, unsigned long vmaddr,
 void flush_dcache_page(struct page *page)
 {
 	struct address_space *mapping = page_mapping(page);
-	struct vm_area_struct *mpnt;
+	struct vm_area_struct *mpnt = NULL;
 	struct prio_tree_iter iter;
 	unsigned long offset;
-	unsigned long addr, old_addr = 0;
+	unsigned long addr = 0;
+#ifdef DEBUG_ALIASES
+	unsigned long old_addr = 0;
+#endif
 	pgoff_t pgoff;
 
 	if (mapping && !mapping_mapped(mapping)) {
@@ -304,14 +307,25 @@ void flush_dcache_page(struct page *page)
 		offset = (pgoff - mpnt->vm_pgoff) << PAGE_SHIFT;
 		addr = mpnt->vm_start + offset;
 
+#ifdef DEBUG_ALIASES
 		if (old_addr == 0 || (old_addr & (SHMLBA - 1)) != (addr & (SHMLBA - 1))) {
 			__flush_cache_page(mpnt, addr, page_to_phys(page));
 			if (old_addr)
 				printk(KERN_ERR "INEQUIVALENT ALIASES 0x%lx and 0x%lx in file %s\n", old_addr, addr, mpnt->vm_file ? mpnt->vm_file->f_path.dentry->d_name.name : "(null)");
 			old_addr = addr;
 		}
+#else
+		break;
+#endif
 	}
 	flush_dcache_mmap_unlock(mapping);
+#ifndef DEBUG_ALIASES
+	/* since we have only one page, flush outside the lock.  If the loop
+	 * didn't happen (no mappings), mpnt will be NULL */
+	if (mpnt)
+		__flush_cache_page(mpnt, addr, page_to_phys(page));
+#endif
+
 }
 EXPORT_SYMBOL(flush_dcache_page);
 

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[John David Anglin]

> Can we assume these addresses are some type of artifact and just apply
> the patch below anyway?  On current parisc, it's what we do (we don't
> flush anything beyond the first mapping).

I like the idea but using the first mapping is not correct.  All pages
get flushed during core dumps and there is almost always inequivalent
aliases for the page containing the boundary between text and data.
The text map usually comes first.

I don't think the addresses are an artifact.  However, I don't know
how they occur.  I saw another bunch in the C testsuite last night.
There are thousands of tests, so it's hard to know which test triggered
the messages.

I'm now thinking that the dynamic loader sometimes does multiple maps
of libc.  It's using MAP_FIXED.  The current code changes old_addr
everytime an inequivalent alias occurs.  This makes it a bit difficult
to see what's happening.

The usual case would be just one address to flush.  Would it be too
much overhead to do a prescan to determine if there are inequivalent
aliases before doing any flushes?  I think I'll try this.

Dave

> diff --git a/arch/parisc/kernel/cache.c b/arch/parisc/kernel/cache.c
> index 3f11331..c60cc42 100644
> --- a/arch/parisc/kernel/cache.c
> +++ b/arch/parisc/kernel/cache.c
> @@ -276,10 +276,13 @@ __flush_cache_page(struct vm_area_struct *vma, unsigned long vmaddr,
>  void flush_dcache_page(struct page *page)
>  {
>  	struct address_space *mapping = page_mapping(page);
> -	struct vm_area_struct *mpnt;
> +	struct vm_area_struct *mpnt = NULL;
>  	struct prio_tree_iter iter;
>  	unsigned long offset;
> -	unsigned long addr, old_addr = 0;
> +	unsigned long addr = 0;
> +#ifdef DEBUG_ALIASES
> +	unsigned long old_addr = 0;
> +#endif
>  	pgoff_t pgoff;
>  
>  	if (mapping && !mapping_mapped(mapping)) {
> @@ -304,14 +307,25 @@ void flush_dcache_page(struct page *page)
>  		offset = (pgoff - mpnt->vm_pgoff) << PAGE_SHIFT;
>  		addr = mpnt->vm_start + offset;
>  
> +#ifdef DEBUG_ALIASES
>  		if (old_addr == 0 || (old_addr & (SHMLBA - 1)) != (addr & (SHMLBA - 1))) {
>  			__flush_cache_page(mpnt, addr, page_to_phys(page));
>  			if (old_addr)
>  				printk(KERN_ERR "INEQUIVALENT ALIASES 0x%lx and 0x%lx in file %s\n", old_addr, addr, mpnt->vm_file ? mpnt->vm_file->f_path.dentry->d_name.name : "(null)");
>  			old_addr = addr;
>  		}
> +#else
> +		break;
> +#endif
>  	}
>  	flush_dcache_mmap_unlock(mapping);
> +#ifndef DEBUG_ALIASES
> +	/* since we have only one page, flush outside the lock.  If the loop
> +	 * didn't happen (no mappings), mpnt will be NULL */
> +	if (mpnt)
> +		__flush_cache_page(mpnt, addr, page_to_phys(page));
> +#endif
> +
>  }
>  EXPORT_SYMBOL(flush_dcache_page);
>  
> 
> 


-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[John David Anglin]

> The usual case would be just one address to flush.  Would it be too
> much overhead to do a prescan to determine if there are inequivalent
> aliases before doing any flushes?  I think I'll try this.

Testing the following.  Dropped debug message.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

diff --git a/arch/parisc/kernel/cache.c b/arch/parisc/kernel/cache.c
index 3f11331..e57c3e8 100644
--- a/arch/parisc/kernel/cache.c
+++ b/arch/parisc/kernel/cache.c
@@ -75,14 +75,16 @@ EXPORT_SYMBOL(flush_cache_all_local);
 void
 update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t *ptep)
 {
-	struct page *page = pte_page(*ptep);
+        unsigned long pfn = pte_pfn(*ptep);
+        struct page *page;
 
-	if (pfn_valid(page_to_pfn(page)) && page_mapping(page) &&
-	    test_bit(PG_dcache_dirty, &page->flags)) {
+        if (!pfn_valid(pfn))
+                return;
 
-		flush_kernel_dcache_page(page);
-		clear_bit(PG_dcache_dirty, &page->flags);
-	} else if (parisc_requires_coherency())
+        page = pfn_to_page(pfn);
+
+        if (test_and_clear_bit(PG_dcache_dirty, &page->flags)
+	    || parisc_requires_coherency())
 		flush_kernel_dcache_page(page);
 }
 
@@ -276,10 +278,10 @@ __flush_cache_page(struct vm_area_struct *vma, unsigned long vmaddr,
 void flush_dcache_page(struct page *page)
 {
 	struct address_space *mapping = page_mapping(page);
-	struct vm_area_struct *mpnt;
+	struct vm_area_struct *mpnt, *old_mpnt;
 	struct prio_tree_iter iter;
 	unsigned long offset;
-	unsigned long addr, old_addr = 0;
+	unsigned long addr, old_addr;
 	pgoff_t pgoff;
 
 	if (mapping && !mapping_mapped(mapping)) {
@@ -292,26 +294,51 @@ void flush_dcache_page(struct page *page)
 	if (!mapping)
 		return;
 
+	old_addr = 0;
+	old_mpnt = NULL;
 	pgoff = page->index << (PAGE_CACHE_SHIFT - PAGE_SHIFT);
 
 	/* We have carefully arranged in arch_get_unmapped_area() that
-	 * *any* mappings of a file are always congruently mapped (whether
-	 * declared as MAP_PRIVATE or MAP_SHARED), so we only need
-	 * to flush one address here for them all to become coherent */
+	 * all shared mappings of a file are congruently mapped, so we
+	 * only need to flush one address here for them all to become
+	 * coherent.  However, non-shared fixed mappings of executables
+	 * are not congruently mapped on the boundary page between text
+	 * and data.  Further, the data segment sometimes occurs before
+	 * the text segment.  While it is unlikely that a dirty cache
+	 * line would result from accesses through the text mapping,
+	 * it is possible that this could occur since binutils doesn't
+	 * ensure that the data segment starts on a page boundary.  */
 
 	flush_dcache_mmap_lock(mapping);
+
+	/* Scan for inequivalent aliases.  */
 	vma_prio_tree_foreach(mpnt, &iter, &mapping->i_mmap, pgoff, pgoff) {
 		offset = (pgoff - mpnt->vm_pgoff) << PAGE_SHIFT;
 		addr = mpnt->vm_start + offset;
 
-		if (old_addr == 0 || (old_addr & (SHMLBA - 1)) != (addr & (SHMLBA - 1))) {
-			__flush_cache_page(mpnt, addr, page_to_phys(page));
-			if (old_addr)
-				printk(KERN_ERR "INEQUIVALENT ALIASES 0x%lx and 0x%lx in file %s\n", old_addr, addr, mpnt->vm_file ? mpnt->vm_file->f_path.dentry->d_name.name : "(null)");
+		if (old_addr == 0) {
 			old_addr = addr;
+			old_mpnt = mpnt;
 		}
+		else if ((old_addr & (SHMLBA - 1)) != (addr & (SHMLBA - 1)))
+			goto flush_inequivalent;
+	}
+	flush_dcache_mmap_unlock(mapping);
+
+	/* Handle common case where all aliases are equivalent.  */
+	if (old_addr)
+		__flush_cache_page(old_mpnt, old_addr, page_to_phys(page));
+	return;
+
+flush_inequivalent:
+	vma_prio_tree_foreach(mpnt, &iter, &mapping->i_mmap, pgoff, pgoff) {
+		offset = (pgoff - mpnt->vm_pgoff) << PAGE_SHIFT;
+		addr = mpnt->vm_start + offset;
+
+		__flush_cache_page(mpnt, addr, page_to_phys(page));
 	}
 	flush_dcache_mmap_unlock(mapping);
+	return;
 }
 EXPORT_SYMBOL(flush_dcache_page);
 
diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c
index c9b9322..f0cb56e 100644
--- a/arch/parisc/kernel/sys_parisc.c
+++ b/arch/parisc/kernel/sys_parisc.c
@@ -92,11 +92,12 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
 {
 	if (len > TASK_SIZE)
 		return -ENOMEM;
-	/* Might want to check for cache aliasing issues for MAP_FIXED case
-	 * like ARM or MIPS ??? --BenH.
-	 */
-	if (flags & MAP_FIXED)
+	if (flags & MAP_FIXED) {
+		if ((flags & MAP_SHARED) &&
+		    (addr - (pgoff << PAGE_SHIFT)) & (SHMLBA - 1))
+			return -EINVAL;
 		return addr;
+	}
 	if (!addr)
 		addr = TASK_UNMAPPED_BASE;
 

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[John David Anglin]

On Thu, 10 Feb 2011, John David Anglin wrote:

> > The usual case would be just one address to flush.  Would it be too
> > much overhead to do a prescan to determine if there are inequivalent
> > aliases before doing any flushes?  I think I'll try this.
> 
> Testing the following.  Dropped debug message.

Although I didn't see any slow timer messages, I still had a number of
GCC testsuite timeouts.  I didn't see these timeouts with interrupts
disabled.  So, I plan to see if read write locks can be used without
disabling interrupts.

In any case, I believe it is useful to proceed with the parts of the
change where there is agreement.  Note that the change to update_mmu_cache
is more than a optimization when parisc_requires_coherency is true as
the code no longer flushes when the pfn is not valid.

As noted, I'm seeing much improved stability on my rp3440 with this change.

Signed-off-by: John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

diff --git a/arch/parisc/kernel/cache.c b/arch/parisc/kernel/cache.c
index 3f11331..e57c3e8 100644
--- a/arch/parisc/kernel/cache.c
+++ b/arch/parisc/kernel/cache.c
@@ -75,14 +75,16 @@ EXPORT_SYMBOL(flush_cache_all_local);
 void
 update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t *ptep)
 {
-	struct page *page = pte_page(*ptep);
+        unsigned long pfn = pte_pfn(*ptep);
+        struct page *page;
 
-	if (pfn_valid(page_to_pfn(page)) && page_mapping(page) &&
-	    test_bit(PG_dcache_dirty, &page->flags)) {
+        if (!pfn_valid(pfn))
+                return;
 
-		flush_kernel_dcache_page(page);
-		clear_bit(PG_dcache_dirty, &page->flags);
-	} else if (parisc_requires_coherency())
+        page = pfn_to_page(pfn);
+
+        if (test_and_clear_bit(PG_dcache_dirty, &page->flags)
+	    || parisc_requires_coherency())
 		flush_kernel_dcache_page(page);
 }
 
diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c
index c9b9322..f0cb56e 100644
--- a/arch/parisc/kernel/sys_parisc.c
+++ b/arch/parisc/kernel/sys_parisc.c
@@ -92,11 +92,12 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
 {
 	if (len > TASK_SIZE)
 		return -ENOMEM;
-	/* Might want to check for cache aliasing issues for MAP_FIXED case
-	 * like ARM or MIPS ??? --BenH.
-	 */
-	if (flags & MAP_FIXED)
+	if (flags & MAP_FIXED) {
+		if ((flags & MAP_SHARED) &&
+		    (addr - (pgoff << PAGE_SHIFT)) & (SHMLBA - 1))
+			return -EINVAL;
 		return addr;
+	}
 	if (!addr)
 		addr = TASK_UNMAPPED_BASE;
 

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[John David Anglin]

On Sat, 12 Feb 2011, John David Anglin wrote:

> On Thu, 10 Feb 2011, John David Anglin wrote:
> 
> > > The usual case would be just one address to flush.  Would it be too
> > > much overhead to do a prescan to determine if there are inequivalent
> > > aliases before doing any flushes?  I think I'll try this.
> > 
> > Testing the following.  Dropped debug message.
> 
> Although I didn't see any slow timer messages, I still had a number of
> GCC testsuite timeouts.  I didn't see these timeouts with interrupts
> disabled.  So, I plan to see if read write locks can be used without
> disabling interrupts.

Read write locks won't work.  "spin_lock_irq(&mapping->tree_lock)" is
used extensively in the mm code.

To get a better idea of the maps when we have INEQUIVALENT ALIASES, I
changed the code to print the complete set.  old_addr is the first address
in the list.

Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x40a7e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x40a7e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x41a7e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x41e7e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x41e7e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027e000 in
 file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x40a7e000 in
 file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in
 file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in
 file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x41e7e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x41e7e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4127e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4127e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x40e7e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x40a7e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4127e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027e000 in file libc-2.11.2.so with flags 0x8100077
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x40a7d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x40a7d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x41a7d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x41e7d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x41e7d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x40a7d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x41e7d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x41e7d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4127d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4127d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x40e7d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x40a7d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4127d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4067d000 in file libc-2.11.2.so with flags 0x8100075
Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027d000 in file libc-2.11.2.so with flags 0x8100075

As can be seen, the list is quite long and contains numerous duplicate entries.
I think all the entries with VM_WRITE set in flags are equivalent.  Same for
the entries where VM_WRITE is not set.  I'm not sure what can be assumed
about the list order, but the two sets are grouped in this case.  So, the
technique used in James' change seems reasonably good for this case.

It might be two passes over the list should be used to first flush shared
writeable maps, and then another pass to invalidate the other maps.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[John David Anglin]

On Sun, 13 Feb 2011, John David Anglin wrote:

> Feb 13 07:00:24 mx3210 kernel: INEQUIVALENT ALIASES 0x40a7e000 and 0x4027d000 in file libc-2.11.2.so with flags 0x8100075

I'm testing a binutils change to the default linker scrip for ld to
align the file offset for the data segment to a page boundary.  This
should eliminate this source of inequivalent aliases.

> It might be two passes over the list should be used to first flush shared
> writeable maps, and then another pass to invalidate the other maps.

I made the following change to flush_dcache_page().  Although a bit ugly,
it handles the boundary page issue in one pass.

With it, I see another type of inequivalent aliases in the GCC C testsuite:

Feb 14 07:48:06 mx3210 kernel: INEQUIVALENT ALIASES: page 0x4282e520 addr 0x40738000 in file libc-2.11.2.so with flags 0x8100075
Feb 14 07:48:06 mx3210 kernel: INEQUIVALENT ALIASES: page 0x4282e520 addr 0x40738000 in file libc-2.11.2.so with flags 0x8100075
Feb 14 07:48:06 mx3210 kernel: INEQUIVALENT ALIASES: page 0x4282e520 addr 0x40738000 in file libc-2.11.2.so with flags 0x8100075
...
Feb 14 07:48:06 mx3210 kernel: INEQUIVALENT ALIASES: page 0x4282e520 addr 0x4033
8000 in file libc-2.11.2.so with flags 0x8100075
Feb 14 07:48:06 mx3210 kernel: INEQUIVALENT ALIASES: page 0x4282e520 addr 0x40f3
8000 in file libc-2.11.2.so with flags 0x8100075
Feb 14 07:48:06 mx3210 kernel: INEQUIVALENT ALIASES: page 0x4282e520 addr 0x411e
8000 in file libc-2.11.2.so with flags 0x8000071

i haven't tracked down where these come from but this looks like a glibc issue.
Note the final vm_flags value for the final map.  This is the inequivalent one.
As far as I know, the GCC testsuite does not play with MAP_FIXED maps.

Carlos, any thoughts on where this might come from?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

diff --git a/arch/parisc/kernel/cache.c b/arch/parisc/kernel/cache.c
index 3f11331..f90a330 100644
--- a/arch/parisc/kernel/cache.c
+++ b/arch/parisc/kernel/cache.c
@@ -276,10 +278,10 @@ __flush_cache_page(struct vm_area_struct *vma, unsigned long vmaddr,
 void flush_dcache_page(struct page *page)
 {
 	struct address_space *mapping = page_mapping(page);
-	struct vm_area_struct *mpnt;
+	struct vm_area_struct *mpnt, *mpnt_wrt, *mpnt_nwrt;
 	struct prio_tree_iter iter;
 	unsigned long offset;
-	unsigned long addr, old_addr = 0;
+	unsigned long addr, addr_wrt, addr_nwrt;
 	pgoff_t pgoff;
 
 	if (mapping && !mapping_mapped(mapping)) {
@@ -292,26 +294,69 @@ void flush_dcache_page(struct page *page)
 	if (!mapping)
 		return;
 
+	addr_wrt = 0;
+	mpnt_wrt = NULL;
+	addr_nwrt = 0;
+	mpnt_nwrt = NULL;
 	pgoff = page->index << (PAGE_CACHE_SHIFT - PAGE_SHIFT);
 
 	/* We have carefully arranged in arch_get_unmapped_area() that
-	 * *any* mappings of a file are always congruently mapped (whether
-	 * declared as MAP_PRIVATE or MAP_SHARED), so we only need
-	 * to flush one address here for them all to become coherent */
+	 * all shared mappings of a file are congruently mapped, so we
+	 * only need to flush one address here for them all to become
+	 * coherent.  However, non-shared fixed mappings of executables
+	 * are not congruently mapped on the boundary page between text
+	 * and data.  Further, the data segment sometimes occurs before
+	 * the text segment.  While it is unlikely that a dirty cache
+	 * line would result from accesses through the text mapping,
+	 * it is possible that this could occur since binutils doesn't
+	 * ensure that the data segment starts on a page boundary.  */
 
 	flush_dcache_mmap_lock(mapping);
+
+	/* Scan for inequivalent aliases.  */
 	vma_prio_tree_foreach(mpnt, &iter, &mapping->i_mmap, pgoff, pgoff) {
 		offset = (pgoff - mpnt->vm_pgoff) << PAGE_SHIFT;
 		addr = mpnt->vm_start + offset;
 
-		if (old_addr == 0 || (old_addr & (SHMLBA - 1)) != (addr & (SHMLBA - 1))) {
-			__flush_cache_page(mpnt, addr, page_to_phys(page));
-			if (old_addr)
-				printk(KERN_ERR "INEQUIVALENT ALIASES 0x%lx and 0x%lx in file %s\n", old_addr, addr, mpnt->vm_file ? mpnt->vm_file->f_path.dentry->d_name.name : "(null)");
-			old_addr = addr;
+		if (mpnt->vm_flags & VM_WRITE) {
+			if (addr_wrt == 0) {
+				addr_wrt = addr;
+				mpnt_wrt = mpnt;
+			}
+			else if ((addr_wrt & (SHMLBA - 1)) != (addr & (SHMLBA - 1)))
+				goto flush_inequivalent;
 		}
+		else {
+			if (addr_nwrt == 0) {
+				addr_nwrt = addr;
+				mpnt_nwrt = mpnt;
+			}
+			else if ((addr_nwrt & (SHMLBA - 1)) != (addr & (SHMLBA - 1)))
+				goto flush_inequivalent;
+		}
+	}
+	flush_dcache_mmap_unlock(mapping);
+
+	/* Common case where all writeable aliases are equivalent and
+	 * all non writeable aliases are equivalent.  */
+	if (addr_wrt)
+		__flush_cache_page(mpnt_wrt, addr_wrt, page_to_phys(page));
+	if (addr_nwrt &&
+	    (!addr_wrt || (addr_nwrt & (SHMLBA - 1)) != (addr_wrt & (SHMLBA - 1))))
+		__flush_cache_page(mpnt_nwrt, addr_nwrt, page_to_phys(page));
+	return;
+
+flush_inequivalent:
+	vma_prio_tree_foreach(mpnt, &iter, &mapping->i_mmap, pgoff, pgoff) {
+		offset = (pgoff - mpnt->vm_pgoff) << PAGE_SHIFT;
+		addr = mpnt->vm_start + offset;
+
+		printk(KERN_ERR "INEQUIVALENT ALIASES: page 0x%lx addr 0x%lx in file %s with flags 0x%lx\n", (unsigned long) page, addr, mpnt->vm_file ? (char *) mpnt->vm_file->f_path.dentry->d_name.name : "(null)", mpnt->vm_flags);
+
+		__flush_cache_page(mpnt, addr, page_to_phys(page));
 	}
 	flush_dcache_mmap_unlock(mapping);
+	return;
 }
 EXPORT_SYMBOL(flush_dcache_page);
 

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[Carlos O'Donell]

On Mon, Feb 14, 2011 at 8:58 PM, John David Anglin
<dave@hiauly1.hia.nrc.ca> wrote:
> Feb 14 07:48:06 mx3210 kernel: INEQUIVALENT ALIASES: page 0x4282e520 =
addr 0x411e
> 8000 in file libc-2.11.2.so with flags 0x8000071
>
> i haven't tracked down where these come from but this looks like a gl=
ibc issue.
> Note the final vm_flags value for the final map. =A0This is the inequ=
ivalent one.
> As far as I know, the GCC testsuite does not play with MAP_FIXED maps=
=2E
>
> Carlos, any thoughts on where this might come from?

Could you more clearly describe exactly what is the problem from
glibc's perspective?

The dynamic loader will do a non-fixed mapping to cover the entire
range of the PIC code needed to load into memory. This ensures it has
enough space to map segments. Then it follows up by using MAP_FIXED to
exactly map the segment from the file into memory. It also maps
remaining pages at MAP_FIXED from fd=3D-1 (zero fill).

Locale archives are also mapped in via MAP_FIXED (after a non-fixed map=
ping).

Malloc uses MAP_FIXED and PROT_NONE (inaccessible) when shrinking
internal heaps e.g. it uses MAP_FIXED to single out the pages that are
no longer accessible (in a secure glibc it uses madvise
(MADV_DONTNEED).

Malloc under some conditions may mremap with MREMAP_FIXED.

Those are the glibc instances of MAP_FIXED maps that I can find.

Cheers,
Carlos.
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
 in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[John David Anglin]

On Tue, 15 Feb 2011, Carlos O'Donell wrote:

> On Mon, Feb 14, 2011 at 8:58 PM, John David Anglin
> <dave@hiauly1.hia.nrc.ca> wrote:
> > Feb 14 07:48:06 mx3210 kernel: INEQUIVALENT ALIASES: page 0x4282e52=
0 addr 0x411e
> > 8000 in file libc-2.11.2.so with flags 0x8000071
> >
> > i haven't tracked down where these come from but this looks like a =
glibc issue.
> > Note the final vm_flags value for the final map. =A0This is the ine=
quivalent one.
> > As far as I know, the GCC testsuite does not play with MAP_FIXED ma=
ps.
> >
> > Carlos, any thoughts on where this might come from?
>=20
> Could you more clearly describe exactly what is the problem from
> glibc's perspective?

It's a little hard to tell since I haven't found the application
the causes the above inequivalent aliases.

=46rom a kernel perspective, we need to avoid multiple maps of the same
memory page with inequivalent virtual addresses (i.e., the maps must
start at the same address & 0x3fffff).

Inequivalent maps only result from the use of MAP_FIXED.  All
the maps with VM_READ and VM_EXEC are equivalent in this case,
so it's fairly clear that the map is for a code segment of
libc-2.11.2.so.  There is one map with just the VM_READ flag
that is not equivalent.  Because there are multiple maps to the
same memory page, the page is in effect shared.

Ideally, glibc should use equivalent aliases for all maps that
it creates when they are "shared" as above.

> The dynamic loader will do a non-fixed mapping to cover the entire
> range of the PIC code needed to load into memory. This ensures it has
> enough space to map segments. Then it follows up by using MAP_FIXED t=
o
> exactly map the segment from the file into memory. It also maps
> remaining pages at MAP_FIXED from fd=3D-1 (zero fill).

I tend to think this is the problem area.  I will try to find a
testcase.

> Locale archives are also mapped in via MAP_FIXED (after a non-fixed m=
apping).
>=20
> Malloc uses MAP_FIXED and PROT_NONE (inaccessible) when shrinking
> internal heaps e.g. it uses MAP_FIXED to single out the pages that ar=
e
> no longer accessible (in a secure glibc it uses madvise
> (MADV_DONTNEED).
>=20
> Malloc under some conditions may mremap with MREMAP_FIXED.
>=20
> Those are the glibc instances of MAP_FIXED maps that I can find.

None of these appear to involve executable maps to a file.

Dave
--=20
J. David Anglin                                  dave.anglin@nrc-cnrc.g=
c.ca
National Research Council of Canada              (613) 990-0752 (FAX: 9=
52-6602)
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
 in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[James Bottomley]

On Tue, 2011-02-15 at 11:47 -0500, John David Anglin wrote:
> On Tue, 15 Feb 2011, Carlos O'Donell wrote:
> 
> > On Mon, Feb 14, 2011 at 8:58 PM, John David Anglin
> > <dave@hiauly1.hia.nrc.ca> wrote:
> > > Feb 14 07:48:06 mx3210 kernel: INEQUIVALENT ALIASES: page 0x4282e520 addr 0x411e
> > > 8000 in file libc-2.11.2.so with flags 0x8000071
> > >
> > > i haven't tracked down where these come from but this looks like a glibc issue.
> > > Note the final vm_flags value for the final map.  This is the inequivalent one.
> > > As far as I know, the GCC testsuite does not play with MAP_FIXED maps.
> > >
> > > Carlos, any thoughts on where this might come from?
> > 
> > Could you more clearly describe exactly what is the problem from
> > glibc's perspective?
> 
> It's a little hard to tell since I haven't found the application
> the causes the above inequivalent aliases.

There was a prink in the inequivalent alias detector that printed
task->comm (that's the process name) if you add it back it will give
more information.

> >From a kernel perspective, we need to avoid multiple maps of the same
> memory page with inequivalent virtual addresses (i.e., the maps must
> start at the same address & 0x3fffff).

If I understand what you've done: you already patched the kernel to
force MAP_FIXED onto this boundary.  If that's the case, the actual
problem must be an offset mapping; meaning something maps a region and
then maps a subregion of it.  I've got to guess this is the dynamic
loader.

> Inequivalent maps only result from the use of MAP_FIXED.  All
> the maps with VM_READ and VM_EXEC are equivalent in this case,
> so it's fairly clear that the map is for a code segment of
> libc-2.11.2.so.  There is one map with just the VM_READ flag
> that is not equivalent.  Because there are multiple maps to the
> same memory page, the page is in effect shared.
> 
> Ideally, glibc should use equivalent aliases for all maps that
> it creates when they are "shared" as above.
> 
> > The dynamic loader will do a non-fixed mapping to cover the entire
> > range of the PIC code needed to load into memory. This ensures it has
> > enough space to map segments. Then it follows up by using MAP_FIXED to
> > exactly map the segment from the file into memory. It also maps
> > remaining pages at MAP_FIXED from fd=-1 (zero fill).
> 
> I tend to think this is the problem area.  I will try to find a
> testcase.

Great, thanks!  Just getting an application (or .c file) that does this
will give us a better opportunity to analyse what the problem is.

James

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[John David Anglin]

> There was a prink in the inequivalent alias detector that printed
> task->comm (that's the process name) if you add it back it will give
> more information.

Will add and rerun GCC testsuite.

> > >From a kernel perspective, we need to avoid multiple maps of the same
> > memory page with inequivalent virtual addresses (i.e., the maps must
> > start at the same address & 0x3fffff).
> 
> If I understand what you've done: you already patched the kernel to
> force MAP_FIXED onto this boundary.  If that's the case, the actual
> problem must be an offset mapping; meaning something maps a region and
> then maps a subregion of it.  I've got to guess this is the dynamic
> loader.

This only occurs for MAP_FIXED and MAP_SHARED.  MAP_FIXED alone
still maps at the address specified by the application.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

binutils change break glibc build

[John David Anglin]

Hi Carlos,

I would like to apply the following binutils change to align data
segments to a page boundary.  However, it breaks building glibc.
Problem occurs in call_init trying to run constructors for new
libc.so.6.

Any thoughts?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Index: emulparams/hppalinux.sh
===================================================================
RCS file: /cvs/src/src/ld/emulparams/hppalinux.sh,v
retrieving revision 1.14
diff -u -3 -p -r1.14 hppalinux.sh
--- emulparams/hppalinux.sh	22 Oct 2008 05:20:44 -0000	1.14
+++ emulparams/hppalinux.sh	15 Feb 2011 14:30:24 -0000
@@ -8,6 +8,8 @@ NO_REL_RELOCS=yes
 TEXT_START_ADDR=0x10000
 TARGET_PAGE_SIZE=0x10000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
+DATA_ADDR="ALIGN(${MAXPAGESIZE})"
+SHLIB_DATA_ADDR="ALIGN(${MAXPAGESIZE})"
 ARCH=hppa
 MACHINE=hppa1.1    # We use 1.1 specific features.
 NOP=0x08000240

Re: [PATCH] parisc: Improve dcache flush on PA8800/PA8900

[John David Anglin]

On Tue, 15 Feb 2011, James Bottomley wrote:

> > I tend to think this is the problem area.  I will try to find a
> > testcase.
> 
> Great, thanks!  Just getting an application (or .c file) that does this
> will give us a better opportunity to analyse what the problem is.

I have found a testcase.  Unfortunately, it's almost impossible to debug.
It a GCC guality test.  These tests launch gdb and then have gdb print
debug info at various breakpoints.  If run with strace, this prevents
gdb from attaching and the inequivalent alias state doesn't occur ;(

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: binutils change break glibc build

[John David Anglin]

On Wed, 16 Feb 2011, John David Anglin wrote:

> Hi Carlos,
> 
> I would like to apply the following binutils change to align data
> segments to a page boundary.  However, it breaks building glibc.
> Problem occurs in call_init trying to run constructors for new
> libc.so.6.

It's probably not a glibc issue:

/home2/dave/debian/glibc/eglibc-2.11.2/build-tree/hppa-libc/libc.so.6:     file 
format elf32-hppa-linux

Contents of section .init_array:
 159008 ffffffff 0015bee2 00000000           ............    

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: binutils change break glibc build

[John David Anglin]

> On Wed, 16 Feb 2011, John David Anglin wrote:
> 
> > Hi Carlos,
> > 
> > I would like to apply the following binutils change to align data
> > segments to a page boundary.  However, it breaks building glibc.
> > Problem occurs in call_init trying to run constructors for new
> > libc.so.6.
> 
> It's probably not a glibc issue:
> 
> /home2/dave/debian/glibc/eglibc-2.11.2/build-tree/hppa-libc/libc.so.6:     file 
> format elf32-hppa-linux
> 
> Contents of section .init_array:
>  159008 ffffffff 0015bee2 00000000           ............    

Hmmm, there is no .init_array section in the installed libc.  So, maybe
it's not the patch.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: binutils change break glibc build

[John David Anglin]

On Wed, 16 Feb 2011, John David Anglin wrote:

> > On Wed, 16 Feb 2011, John David Anglin wrote:
> > 
> > > Hi Carlos,
> > > 
> > > I would like to apply the following binutils change to align data
> > > segments to a page boundary.  However, it breaks building glibc.
> > > Problem occurs in call_init trying to run constructors for new
> > > libc.so.6.
> > 
> > It's probably not a glibc issue:
> > 
> > /home2/dave/debian/glibc/eglibc-2.11.2/build-tree/hppa-libc/libc.so.6:     file 
> > format elf32-hppa-linux
> > 
> > Contents of section .init_array:
> >  159008 ffffffff 0015bee2 00000000           ............    
> 
> Hmmm, there is no .init_array section in the installed libc.  So, maybe
> it's not the patch.

It seems that glibc uses the .init_array section.  See for example
dlfcn.c.  Howe did this work before?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: binutils change break glibc build

[John David Anglin]

On Wed, 16 Feb 2011, John David Anglin wrote:

> > > It's probably not a glibc issue:
> > > 
> > > /home2/dave/debian/glibc/eglibc-2.11.2/build-tree/hppa-libc/libc.so.6:     file 
> > > format elf32-hppa-linux
> > > 
> > > Contents of section .init_array:
> > >  159008 ffffffff 0015bee2 00000000           ............    
> > 
> > Hmmm, there is no .init_array section in the installed libc.  So, maybe
> > it's not the patch.

This is a binutils bug introduced by merging .ctors into .init_array.
That's where the ffffffff and 00000000 values come from.  They actual
mark the limits of the .ctors list.

It can be avoided by building binutils with --disable-initfini-array.
This is not exactly obvious...

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: binutils change break glibc build

[Mike Frysinger]

On Wed, Feb 16, 2011 at 8:09 PM, John David Anglin wrote:
> On Wed, 16 Feb 2011, John David Anglin wrote:
>> I would like to apply the following binutils change to align data
>> segments to a page boundary. =A0However, it breaks building glibc.
>> Problem occurs in call_init trying to run constructors for new
>> libc.so.6.
>
> It's probably not a glibc issue:
>
> /home2/dave/debian/glibc/eglibc-2.11.2/build-tree/hppa-libc/libc.so.6=
: =A0 =A0 file
> format elf32-hppa-linux
>
> Contents of section .init_array:
> =A0159008 ffffffff 0015bee2 00000000 =A0 =A0 =A0 =A0 =A0 ............

glibc-2.12 and earlier (and perhaps 2.12.[12] too) are known to be
miscompiled by binutils versions newer than 2.21.50.xxx.  the issue
has been fixed in glibc-2.13 though ...

http://bugs.gentoo.org/351177
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
 in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: binutils change break glibc build

[John David Anglin]

> On Wed, Feb 16, 2011 at 8:09 PM, John David Anglin wrote:
> > On Wed, 16 Feb 2011, John David Anglin wrote:
> >> I would like to apply the following binutils change to align data
> >> segments to a page boundary. =A0However, it breaks building glibc.
> >> Problem occurs in call_init trying to run constructors for new
> >> libc.so.6.
> >
> > It's probably not a glibc issue:
> >
> > /home2/dave/debian/glibc/eglibc-2.11.2/build-tree/hppa-libc/libc.so.6: =
> =A0 =A0 file
> > format elf32-hppa-linux
> >
> > Contents of section .init_array:
> > =A0159008 ffffffff 0015bee2 00000000 =A0 =A0 =A0 =A0 =A0 ............
> 
> glibc-2.12 and earlier (and perhaps 2.12.[12] too) are known to be
> miscompiled by binutils versions newer than 2.21.50.xxx.  the issue
> has been fixed in glibc-2.13 though ...
> 
> http://bugs.gentoo.org/351177

While this is probably the same issue, there is no debugging of the segv
in the above bug report or in the referenced bugzilla report:
http://sourceware.org/bugzilla/show_bug.cgi?id=12379

I filed this report last night:
http://sourceware.org/bugzilla/show_bug.cgi?id=12499

My analysis indicates the segv is a binutils issue.  It was likely
introduced by the following change:

2010-12-15  H.J. Lu  <hongjiu.lu@intel.com>

        * Makefile.am (GENSCRIPTS): Add @enable_initfini_array@.

	* NEWS: Mention SORT_BY_INIT_PRIORITY.

	* configure.in: Add AC_CANONICAL_BUILD.
	Add --enable-initfini-array.

	* genscripts.sh (ENABLE_INITFINI_ARRAY): New.
	...

Binutils should not merge the special function pointers used to mark
the start and end of the .ctors/.dtors lists.  This could be tested
for in libc, but this is inefficient.  On architectures that use
function descriptors, function pointer comparisons are messy.  Do
we want to call the infamous cffc in the dynamic loader?  Casts of
function pointers to integer types are implementation defined and
pointer sizes can vary.  So, the simple hack could break.

In my opinion, binutils should be fixed to work with old glibc
versions.  There is a work around in my binutils PR.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: binutils change break glibc build

[Carlos O'Donell]

On Fri, Feb 18, 2011 at 9:39 AM, John David Anglin
<dave@hiauly1.hia.nrc.ca> wrote:
> Binutils should not merge the special function pointers used to mark
> the start and end of the .ctors/.dtors lists. =A0This could be tested
> for in libc, but this is inefficient. =A0On architectures that use
> function descriptors, function pointer comparisons are messy. =A0Do
> we want to call the infamous cffc in the dynamic loader? =A0Casts of
> function pointers to integer types are implementation defined and
> pointer sizes can vary. =A0So, the simple hack could break.
>
> In my opinion, binutils should be fixed to work with old glibc
> versions. =A0There is a work around in my binutils PR.

I would like __cffc to eventually go away. It's only required because
of the incorrectly implemented PLABEL support in binutils. A PLABEL in
the executable should not be resolved to a PLT entry, it should remain
a relocation for the dynamic loader to resolve. Once that's fixed,
eventually __cffc will go away. That's a project that is only agenda
after some glibc cleanup.

Cheers,
Carlos.
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
 in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: binutils change break glibc build

[John David Anglin]

On Wed, 16 Feb 2011, John David Anglin wrote:

> I would like to apply the following binutils change to align data
> segments to a page boundary.  However, it breaks building glibc.
> Problem occurs in call_init trying to run constructors for new
> libc.so.6.

After investigation, I found that the change to emulparams/hppalinux.sh
was not the cause of the glibc breakage.  So, I have committed the change
to align data segments to a maxpagesize boundary (currently 0x1000).

This fixes one source of inequivalent aliases at the expense of
somewhat larger binaries.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: binutils change break glibc build

[Mike Frysinger]

On Fri, Feb 18, 2011 at 1:40 PM, John David Anglin wrote:
> On Wed, 16 Feb 2011, John David Anglin wrote:
>> I would like to apply the following binutils change to align data
>> segments to a page boundary. =A0However, it breaks building glibc.
>> Problem occurs in call_init trying to run constructors for new
>> libc.so.6.
>
> After investigation, I found that the change to emulparams/hppalinux.=
sh
> was not the cause of the glibc breakage. =A0So, I have committed the =
change
> to align data segments to a maxpagesize boundary (currently 0x1000).
>
> This fixes one source of inequivalent aliases at the expense of
> somewhat larger binaries.

as long as we dont move to the ridiculous mips-sized default
alignment, i'm sure the new behavior is more sane ;)
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
 in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: binutils change break glibc build

[Carlos O'Donell]

On Fri, Feb 18, 2011 at 1:40 PM, John David Anglin
<dave@hiauly1.hia.nrc.ca> wrote:
> On Wed, 16 Feb 2011, John David Anglin wrote:
>
>> I would like to apply the following binutils change to align data
>> segments to a page boundary. =A0However, it breaks building glibc.
>> Problem occurs in call_init trying to run constructors for new
>> libc.so.6.
>
> After investigation, I found that the change to emulparams/hppalinux.=
sh
> was not the cause of the glibc breakage. =A0So, I have committed the =
change
> to align data segments to a maxpagesize boundary (currently 0x1000).
>
> This fixes one source of inequivalent aliases at the expense of
> somewhat larger binaries.

A minor loss for a significant gain :-)

Cheers,
Carlos.
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
 in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: binutils change break glibc build

[John David Anglin]

On Fri, 18 Feb 2011, Carlos O'Donell wrote:

> > This fixes one source of inequivalent aliases at the expense of
> > somewhat larger binaries.
> 
> A minor loss for a significant gain :-)

After rebuilding glibc with the new binutils, I no longer see any
INEQUIVALENT ALIASES messages building and checking GCC.

My suggestion to use VM_WRITE doesn't work as a method to distinguish
aliases as gdb sets up writeable maps to text pages.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: binutils change break glibc build

[Carlos O'Donell]

On Mon, Feb 21, 2011 at 7:13 PM, John David Anglin
<dave@hiauly1.hia.nrc.ca> wrote:
> On Fri, 18 Feb 2011, Carlos O'Donell wrote:
>
>> > This fixes one source of inequivalent aliases at the expense of
>> > somewhat larger binaries.
>>
>> A minor loss for a significant gain :-)
>
> After rebuilding glibc with the new binutils, I no longer see any
> INEQUIVALENT ALIASES messages building and checking GCC.
>
> My suggestion to use VM_WRITE doesn't work as a method to distinguish
> aliases as gdb sets up writeable maps to text pages.

Excellent, does that mean that glibc does not create inequivalent
aliases with all of its tom-foolery with mmap?

Cheers,
Carlos.

Re: binutils change break glibc build

[John David Anglin]

On Tue, 22 Feb 2011, Carlos O'Donell wrote:

> Excellent, does that mean that glibc does not create inequivalent
> aliases with all of its tom-foolery with mmap?

It's hard to say for sure.  The kernel message only occurs when a
data cache flush is needed for a specific page.  Typically, that's
when a segv or abort occurs.

I believe that we are still doing full cache flushes on exec, exit
and fork.  I tried recently to kill the flush on fork but the kernel
tried to kill init.  This needs more study.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)