On Fri, 2012-11-02 at 04:20 -0700, Michel Lespinasse wrote:
> On Fri, Nov 2, 2012 at 4:08 AM, James Bottomley
> <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > On Fri, 2012-11-02 at 09:34 +0000, James Bottomley wrote:
> >> > Looking at the arch/parisc/kernel/sys_parisc.c implementation of
> >> > get_shared_area(), I do have a concern though. The function basically
> >> > ignores the pgoff argument, so that if one creates a shared mapping of
> >> > pages 0-N of a file, and then a separate shared mapping of pages 1-N
> >> > of that same file, both will have the same cache offset for their
> >> > starting address.
> >> >
> >> > This looks like this would create obvious aliasing issues. Am I
> >> > misreading this ? I can't understand how this could work good enough
> >> > to be undetected, so there must be something I'm missing here ???
> >>
> >> No, I think it does.
> >>
> >> I also think the pgoff != 0 case is rare, so it doesn't show very often.
> >> To test this, I booted up with a printk for every pgoff != 0 and it
> >> didn't trigger, so it's probably not seen in the ordinary course of
> >> events.
> >
> > I got it to show: Git uses mmap2 with large page offsets. However, it
> > seems to use offsets with a granularity of 16MB which is 4x our aliasing
> > congruency, so an equivalency problem still wouldn't show up.
>
> All right. Thanks for looking it up - I just wasn't sure if my reading
> of the code was correct (and, I am surprised this isn't more common :)
Well, I can fix it like this, but until something actually causes an
inequivalency, I'll keep it as a low priority bug fix.
James
---
diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c
index 7426e40..f76c108 100644
--- a/arch/parisc/kernel/sys_parisc.c
+++ b/arch/parisc/kernel/sys_parisc.c
@@ -73,6 +73,8 @@ static unsigned long get_shared_area(struct address_space *mapping,
struct vm_area_struct *vma;
int offset = mapping ? get_offset(mapping) : 0;
+ offset = (offset + (pgoff << PAGE_SHIFT)) & 0x3FF000;
+
addr = DCACHE_ALIGN(addr - offset) + offset;
for (vma = find_vma(current->mm, addr); ; vma = vma->vm_next) {
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
