Take locating vma and checks on it to a separate helper (it will be
shared between MREMAP_FIXED/non-MREMAP_FIXED cases when we split
them in the next patch)
Signed-off-by: Al Viro <[email protected]>
---
mm/mremap.c | 88 +++++++++++++++++++++++++++++++++++++----------------------
1 files changed, 55 insertions(+), 33 deletions(-)
diff --git a/mm/mremap.c b/mm/mremap.c
index 97bff25..6776136 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -261,6 +261,58 @@ static unsigned long move_vma(struct vm_area_struct *vma,
return new_addr;
}
+static struct vm_area_struct *vma_to_resize(unsigned long addr,
+ unsigned long old_len, unsigned long new_len, unsigned long *p)
+{
+ struct mm_struct *mm = current->mm;
+ struct vm_area_struct *vma = find_vma(mm, addr);
+
+ if (!vma || vma->vm_start > addr)
+ goto Efault;
+
+ if (is_vm_hugetlb_page(vma))
+ goto Einval;
+
+ /* We can't remap across vm area boundaries */
+ if (old_len > vma->vm_end - addr)
+ goto Efault;
+
+ if (vma->vm_flags & (VM_DONTEXPAND | VM_PFNMAP)) {
+ if (new_len > old_len)
+ goto Efault;
+ }
+
+ if (vma->vm_flags & VM_LOCKED) {
+ unsigned long locked, lock_limit;
+ locked = mm->locked_vm << PAGE_SHIFT;
+ lock_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur;
+ locked += new_len - old_len;
+ if (locked > lock_limit && !capable(CAP_IPC_LOCK))
+ goto Eagain;
+ }
+
+ if (!may_expand_vm(mm, (new_len - old_len) >> PAGE_SHIFT))
+ goto Enomem;
+
+ if (vma->vm_flags & VM_ACCOUNT) {
+ unsigned long charged = (new_len - old_len) >> PAGE_SHIFT;
+ if (security_vm_enough_memory(charged))
+ goto Efault;
+ *p = charged;
+ }
+
+ return vma;
+
+Efault: /* very odd choice for most of the cases, but... */
+ return ERR_PTR(-EFAULT);
+Einval:
+ return ERR_PTR(-EINVAL);
+Enomem:
+ return ERR_PTR(-ENOMEM);
+Eagain:
+ return ERR_PTR(-EAGAIN);
+}
+
/*
* Expand (or shrink) an existing mapping, potentially moving it at the
* same time (controlled by the MREMAP_MAYMOVE flag and available VM space)
@@ -340,41 +392,12 @@ unsigned long do_mremap(unsigned long addr,
/*
* Ok, we need to grow.. or relocate.
*/
- ret = -EFAULT;
- vma = find_vma(mm, addr);
- if (!vma || vma->vm_start > addr)
- goto out;
- if (is_vm_hugetlb_page(vma)) {
- ret = -EINVAL;
- goto out;
- }
- /* We can't remap across vm area boundaries */
- if (old_len > vma->vm_end - addr)
- goto out;
- if (vma->vm_flags & (VM_DONTEXPAND | VM_PFNMAP)) {
- if (new_len > old_len)
- goto out;
- }
- if (vma->vm_flags & VM_LOCKED) {
- unsigned long locked, lock_limit;
- locked = mm->locked_vm << PAGE_SHIFT;
- lock_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur;
- locked += new_len - old_len;
- ret = -EAGAIN;
- if (locked > lock_limit && !capable(CAP_IPC_LOCK))
- goto out;
- }
- if (!may_expand_vm(mm, (new_len - old_len) >> PAGE_SHIFT)) {
- ret = -ENOMEM;
+ vma = vma_to_resize(addr, old_len, new_len, &charged);
+ if (IS_ERR(vma)) {
+ ret = PTR_ERR(vma);
goto out;
}
- if (vma->vm_flags & VM_ACCOUNT) {
- charged = (new_len - old_len) >> PAGE_SHIFT;
- if (security_vm_enough_memory(charged))
- goto out_nc;
- }
-
/* old_len exactly to the end of the area..
* And we're not relocating the area.
*/
@@ -430,7 +453,6 @@ unsigned long do_mremap(unsigned long addr,
out:
if (ret & ~PAGE_MASK)
vm_unacct_memory(charged);
-out_nc:
return ret;
}
--
1.5.6.5
Al,
just checking: do you think this is ready for merging now? The lack of
RFC seems to imply it is, but I don't see the Ack's from David from your
previous series, for example.
Linus
On Mon, Dec 07, 2009 at 09:17:40AM -0800, Linus Torvalds wrote:
>
> Al,
> just checking: do you think this is ready for merging now? The lack of
> RFC seems to imply it is, but I don't see the Ack's from David from your
> previous series, for example.
Which David? ;-) All sparc-related bits seem to have been ACKed by
davem.
Hugetlbfs tests seem to be OK with this series, but there's an odd
thing going on - expanding mremap() crossing into hugepage range on
ppc takes a while to convert it into 4Kb pages. Fair enough, but
it seems to work _much_ faster if the process is ptraced. NFI why;
waiting for dgw to get around to profiling it...
I'd love to see comments from ia64 and s390 folks, actually.
Known issues:
* expand_stack() one. Missing checks, not obvious which way to
deal with that. Same as in mainline. Itanic and sparc32 are killable
by that, on itanic with hw 32bit support it looks even nastier (likely
escalation to execution of user code in ring 0).
* remap_file_pages() seems to ignore any alignment rules; it
is OK wrt to address range (it works on existing vma), but cache
coherency might be buggered. Same as in mainline.
* spufs (ppc cell stuff) does 64Kb-paged mappings; what happens
upon mremap() and friends? VM_HUGETLB will *not* be set on it, so the
usual checks in mm/* are going to miss. _Probably_ unchanged compared
to mainline, but I'd really like to understand how it's supposed to work.
On Mon, 7 Dec 2009, Al Viro wrote:
>
> Which David? ;-) All sparc-related bits seem to have been ACKed by
> davem.
That's the David I meant. I've seen the ack, but the acks weren't actually
quoted in your latest series.
Linus
On Mon, Dec 07, 2009 at 11:35:16AM -0800, Linus Torvalds wrote:
>
>
> On Mon, 7 Dec 2009, Al Viro wrote:
> >
> > Which David? ;-) All sparc-related bits seem to have been ACKed by
> > davem.
>
> That's the David I meant. I've seen the ack, but the acks weren't actually
> quoted in your latest series.
Hugh had just caught a lovely piece of idiocy in do_brk() patch; that
call of get_unmapped_area() in there should take MAP_FIXED, not
MAP_FIXED | flags. Harmless (we do get MAP_FIXED anyway and the only
other flag that gets looked at by methods is MAP_SHARED, which is
ignored when file is NULL), but definitely bogus.
I'll fix that before sending a pull request.
Hi!
> Take locating vma and checks on it to a separate helper (it will be
> shared between MREMAP_FIXED/non-MREMAP_FIXED cases when we split
> them in the next patch)
> +static struct vm_area_struct *vma_to_resize(unsigned long addr,
> + unsigned long old_len, unsigned long new_len, unsigned long *p)
> +{
> + struct mm_struct *mm = current->mm;
> + struct vm_area_struct *vma = find_vma(mm, addr);
> +
> + if (!vma || vma->vm_start > addr)
> + goto Efault;
> +
> + if (is_vm_hugetlb_page(vma))
> + goto Einval;
> +
> + /* We can't remap across vm area boundaries */
> + if (old_len > vma->vm_end - addr)
> + goto Efault;
...
> +Efault: /* very odd choice for most of the cases, but... */
> + return ERR_PTR(-EFAULT);
> +Einval:
> + return ERR_PTR(-EINVAL);
> +Enomem:
> + return ERR_PTR(-ENOMEM);
> +Eagain:
> + return ERR_PTR(-EAGAIN);
> +}
Could we just do returns directly, w/o the gotos? In this case I do
not think goto has advantages, as there's no cleanup to do...
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html