On Fri, 25 May 2018 13:16:45 -0700 (PDT) David Rientjes <rient...@google.com>
wrote:
> When charging to a hugetlb_cgroup fails, alloc_huge_page() returns
> ERR_PTR(-ENOSPC) which will cause VM_FAULT_SIGBUS to be returned to the
> page fault handler.
>
> Instead, return the proper error code, ERR_PTR(-ENOMEM), so VM_FAULT_OOM
> is handled correctly. This is consistent with failing mem cgroup charges
> in the non-hugetlb fault path.
>
> At the same time, restructure the return paths of alloc_huge_page() so it
> is consistent.
Patch doesn't appear to match the changelog?
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2006,8 +2006,10 @@ struct page *alloc_huge_page(struct vm_area_struct
> *vma,
> * code of zero indicates a reservation exists (no change).
> */
> map_chg = gbl_chg = vma_needs_reservation(h, vma, addr);
> - if (map_chg < 0)
> - return ERR_PTR(-ENOMEM);
> + if (map_chg < 0) {
> + ret = -ENOMEM;
> + goto out;
> + }
This doesn't change the return value.
> /*
> * Processes that did not create the mapping will have no
> @@ -2019,8 +2021,8 @@ struct page *alloc_huge_page(struct vm_area_struct *vma,
> if (map_chg || avoid_reserve) {
> gbl_chg = hugepage_subpool_get_pages(spool, 1);
> if (gbl_chg < 0) {
> - vma_end_reservation(h, vma, addr);
> - return ERR_PTR(-ENOSPC);
> + ret = -ENOSPC;
> + goto out_reservation;
> }
Nor does this.
> /*
> @@ -2049,8 +2051,10 @@ struct page *alloc_huge_page(struct vm_area_struct
> *vma,
> if (!page) {
> spin_unlock(&hugetlb_lock);
> page = alloc_buddy_huge_page_with_mpol(h, vma, addr);
> - if (!page)
> + if (!page) {
> + ret = -ENOSPC;
> goto out_uncharge_cgroup;
> + }
Nor does this.
> if (!avoid_reserve && vma_has_reserves(vma, gbl_chg)) {
> SetPagePrivate(page);
> h->resv_huge_pages--;
> @@ -2087,8 +2091,10 @@ struct page *alloc_huge_page(struct vm_area_struct
> *vma,
> out_subpool_put:
> if (map_chg || avoid_reserve)
> hugepage_subpool_put_pages(spool, 1);
> +out_reservation:
> vma_end_reservation(h, vma, addr);
> - return ERR_PTR(-ENOSPC);
> +out:
> + return ERR_PTR(ret);
> }
>
It would be nice if you could add a comment over alloc_huge_page()
explaining the return values (at least). Why sometimes ENOMEM, other
times ENOSPC?
