| Message ID | 1352155633-8648-16-git-send-email-walken@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/05/2012 05:47 PM, Michel Lespinasse wrote: > Update the sparc32 arch_get_unmapped_area function to make use of > vm_unmapped_area() instead of implementing a brute force search. > > Signed-off-by: Michel Lespinasse <walken@google.com> Reviewed-by: Rik van Riel <riel@redhat.com>
From: Michel Lespinasse <walken@google.com> Date: Mon, 5 Nov 2012 14:47:12 -0800 > Update the sparc32 arch_get_unmapped_area function to make use of > vm_unmapped_area() instead of implementing a brute force search. > > Signed-off-by: Michel Lespinasse <walken@google.com> Hmmm... > - if (flags & MAP_SHARED) > - addr = COLOUR_ALIGN(addr); > - else > - addr = PAGE_ALIGN(addr); What part of vm_unmapped_area() is going to duplicate this special aligning logic we need on sparc?
On Mon, Nov 5, 2012 at 5:25 PM, David Miller <davem@davemloft.net> wrote: > From: Michel Lespinasse <walken@google.com> > Date: Mon, 5 Nov 2012 14:47:12 -0800 > >> Update the sparc32 arch_get_unmapped_area function to make use of >> vm_unmapped_area() instead of implementing a brute force search. >> >> Signed-off-by: Michel Lespinasse <walken@google.com> > > Hmmm... > >> - if (flags & MAP_SHARED) >> - addr = COLOUR_ALIGN(addr); >> - else >> - addr = PAGE_ALIGN(addr); > > What part of vm_unmapped_area() is going to duplicate this special > aligning logic we need on sparc? The idea there is that you can specify the desired alignment mask and offset using info.align_mask and info.align_offset. Now, I just noticed that the old code actually always uses an alignment offset of 0 instead of basing it on pgoff. I'm not sure why that is, but it looks like this may be an issue ?
On 11/05/2012 08:25 PM, David Miller wrote: > From: Michel Lespinasse <walken@google.com> > Date: Mon, 5 Nov 2012 14:47:12 -0800 > >> Update the sparc32 arch_get_unmapped_area function to make use of >> vm_unmapped_area() instead of implementing a brute force search. >> >> Signed-off-by: Michel Lespinasse <walken@google.com> > > Hmmm... > >> - if (flags & MAP_SHARED) >> - addr = COLOUR_ALIGN(addr); >> - else >> - addr = PAGE_ALIGN(addr); > > What part of vm_unmapped_area() is going to duplicate this special > aligning logic we need on sparc? > That would be this part: +found: + /* We found a suitable gap. Clip it with the original low_limit. */ + if (gap_start < info->low_limit) + gap_start = info->low_limit; + + /* Adjust gap address to the desired alignment */ + gap_start += (info->align_offset - gap_start) & info->align_mask; + + VM_BUG_ON(gap_start + info->length > info->high_limit); + VM_BUG_ON(gap_start + info->length > gap_end); + return gap_start; +}
From: Rik van Riel <riel@redhat.com> Date: Tue, 06 Nov 2012 02:30:07 -0500 > On 11/05/2012 08:25 PM, David Miller wrote: >> From: Michel Lespinasse <walken@google.com> >> Date: Mon, 5 Nov 2012 14:47:12 -0800 >> >>> Update the sparc32 arch_get_unmapped_area function to make use of >>> vm_unmapped_area() instead of implementing a brute force search. >>> >>> Signed-off-by: Michel Lespinasse <walken@google.com> >> >> Hmmm... >> >>> - if (flags & MAP_SHARED) >>> - addr = COLOUR_ALIGN(addr); >>> - else >>> - addr = PAGE_ALIGN(addr); >> >> What part of vm_unmapped_area() is going to duplicate this special >> aligning logic we need on sparc? >> > > That would be this part: > > +found: > + /* We found a suitable gap. Clip it with the original low_limit. */ > + if (gap_start < info->low_limit) > + gap_start = info->low_limit; > + > + /* Adjust gap address to the desired alignment */ > + gap_start += (info->align_offset - gap_start) & info->align_mask; > + > + VM_BUG_ON(gap_start + info->length > info->high_limit); > + VM_BUG_ON(gap_start + info->length > gap_end); > + return gap_start; > +} Ok, now I understand. Works for me: Acked-by: David S. Miller <davem@davemloft.net>
diff --git a/arch/sparc/kernel/sys_sparc_32.c b/arch/sparc/kernel/sys_sparc_32.c index 0c9b31b22e07..653899849b27 100644 --- a/arch/sparc/kernel/sys_sparc_32.c +++ b/arch/sparc/kernel/sys_sparc_32.c @@ -39,6 +39,7 @@ asmlinkage unsigned long sys_getpagesize(void) unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags) { struct vm_area_struct * vmm; + struct vm_unmapped_area_info info; if (flags & MAP_FIXED) { /* We do not accept a shared mapping if it would violate @@ -56,21 +57,14 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsi if (!addr) addr = TASK_UNMAPPED_BASE; - if (flags & MAP_SHARED) - addr = COLOUR_ALIGN(addr); - else - addr = PAGE_ALIGN(addr); - - for (vmm = find_vma(current->mm, addr); ; vmm = vmm->vm_next) { - /* At this point: (!vmm || addr < vmm->vm_end). */ - if (TASK_SIZE - PAGE_SIZE - len < addr) - return -ENOMEM; - if (!vmm || addr + len <= vmm->vm_start) - return addr; - addr = vmm->vm_end; - if (flags & MAP_SHARED) - addr = COLOUR_ALIGN(addr); - } + info.flags = 0; + info.length = len; + info.low_limit = addr; + info.high_limit = TASK_SIZE; + info.align_mask = (flags & MAP_SHARED) ? + (PAGE_MASK & (SHMLBA - 1)) : 0; + info.align_offset = pgoff << PAGE_SHIFT; + return vm_unmapped_area(&info); } /*
Update the sparc32 arch_get_unmapped_area function to make use of vm_unmapped_area() instead of implementing a brute force search. Signed-off-by: Michel Lespinasse <walken@google.com> --- arch/sparc/kernel/sys_sparc_32.c | 24 +++++++++--------------- 1 files changed, 9 insertions(+), 15 deletions(-)