Skip to content

Commit

Permalink
mm,hugetlb: simplify error handling in hugetlb_cow()
Browse files Browse the repository at this point in the history
When returning from hugetlb_cow(), we always (1) put back the refcount
for each referenced page -- always 'old', and 'new' if allocation was
successful.  And (2) retake the page table lock right before returning,
as the callers expects.  This logic can be simplified and encapsulated,
as proposed in this patch.  In addition to cleaner code, we also shave a
few bytes off the instruction text:

   text    data     bss     dec     hex filename
  28399     462   41328   70189   1122d mm/hugetlb.o-baseline
  28367     462   41328   70157   1120d mm/hugetlb.o-patched

Passes libhugetlbfs testcases.

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
Cc: Aswin Chandramouleeswaran <aswin@hp.com>
Acked-by: David Rientjes <rientjes@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Davidlohr Bueso authored and Linus Torvalds committed Aug 7, 2014
1 parent 2f4612a commit ad4404a
Showing 1 changed file with 16 additions and 19 deletions.
35 changes: 16 additions & 19 deletions mm/hugetlb.c
Original file line number Diff line number Diff line change
Expand Up @@ -2808,7 +2808,7 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
{
struct hstate *h = hstate_vma(vma);
struct page *old_page, *new_page;
int outside_reserve = 0;
int ret = 0, outside_reserve = 0;
unsigned long mmun_start; /* For mmu_notifiers */
unsigned long mmun_end; /* For mmu_notifiers */

Expand Down Expand Up @@ -2838,14 +2838,14 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,

page_cache_get(old_page);

/* Drop page table lock as buddy allocator may be called */
/*
* Drop page table lock as buddy allocator may be called. It will
* be acquired again before returning to the caller, as expected.
*/
spin_unlock(ptl);
new_page = alloc_huge_page(vma, address, outside_reserve);

if (IS_ERR(new_page)) {
long err = PTR_ERR(new_page);
page_cache_release(old_page);

/*
* If a process owning a MAP_PRIVATE mapping fails to COW,
* it is due to references held by a child and an insufficient
Expand All @@ -2854,6 +2854,7 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
* may get SIGKILLed if it later faults.
*/
if (outside_reserve) {
page_cache_release(old_page);
BUG_ON(huge_pte_none(pte));
unmap_ref_private(mm, vma, old_page, address);
BUG_ON(huge_pte_none(pte));
Expand All @@ -2869,24 +2870,18 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
return 0;
}

/* Caller expects lock to be held */
spin_lock(ptl);
if (err == -ENOMEM)
return VM_FAULT_OOM;
else
return VM_FAULT_SIGBUS;
ret = (PTR_ERR(new_page) == -ENOMEM) ?
VM_FAULT_OOM : VM_FAULT_SIGBUS;
goto out_release_old;
}

/*
* When the original hugepage is shared one, it does not have
* anon_vma prepared.
*/
if (unlikely(anon_vma_prepare(vma))) {
page_cache_release(new_page);
page_cache_release(old_page);
/* Caller expects lock to be held */
spin_lock(ptl);
return VM_FAULT_OOM;
ret = VM_FAULT_OOM;
goto out_release_all;
}

copy_user_huge_page(new_page, old_page, address, vma,
Expand All @@ -2896,6 +2891,7 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
mmun_start = address & huge_page_mask(h);
mmun_end = mmun_start + huge_page_size(h);
mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end);

/*
* Retake the page table lock to check for racing updates
* before the page tables are altered
Expand All @@ -2916,12 +2912,13 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
}
spin_unlock(ptl);
mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end);
out_release_all:
page_cache_release(new_page);
out_release_old:
page_cache_release(old_page);

/* Caller expects lock to be held */
spin_lock(ptl);
return 0;
spin_lock(ptl); /* Caller expects lock to be held */
return ret;
}

/* Return the pagecache page at a given address within a VMA */
Expand Down

0 comments on commit ad4404a

Please sign in to comment.