Interpreting map_chg_state inline, within alloc_hugetlb_folio(), improves readability. Instead of having cow_from_owner and the result of vma_needs_reservation() compute a map_chg_state, and then interpreting map_chg_state within alloc_hugetlb_folio() to determine whether to + Get a page from the subpool or + Charge cgroup reservations or + Commit vma reservations or + Clean up reservations This refactoring makes those decisions just based on whether a vma_reservation_exists. If a vma_reservation_exists, the subpool had already been debited and the cgroup had been charged, hence alloc_hugetlb_folio() should not double-debit or double-charge. If the vma reservation can't be used (as in cow_from_owner), then the vma reservation effectively does not exist and vma_reservation_exists is set to false. The conditions for committing reservations or cleaning are also updated to be paired with the corresponding conditions guarding reservation creation. Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx> Change-Id: I22d72a2cae61fb64dc78e0a870b254811a06a31e --- mm/hugetlb.c | 94 ++++++++++++++++++++++------------------------------ 1 file changed, 39 insertions(+), 55 deletions(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 597f2b9f62b5..67144af7ab79 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -2968,25 +2968,6 @@ void wait_for_freed_hugetlb_folios(void) flush_work(&free_hpage_work); } -typedef enum { - /* - * For either 0/1: we checked the per-vma resv map, and one resv - * count either can be reused (0), or an extra needed (1). - */ - MAP_CHG_REUSE = 0, - MAP_CHG_NEEDED = 1, - /* - * Cannot use per-vma resv count can be used, hence a new resv - * count is enforced. - * - * NOTE: This is mostly identical to MAP_CHG_NEEDED, except - * that currently vma_needs_reservation() has an unwanted side - * effect to either use end() or commit() to complete the - * transaction. Hence it needs to differenciate from NEEDED. - */ - MAP_CHG_ENFORCED = 2, -} map_chg_state; - /* * NOTE! "cow_from_owner" represents a very hacky usage only used in CoW * faults of hugetlb private mappings on top of a non-page-cache folio (in @@ -3000,46 +2981,45 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma, struct hugepage_subpool *spool = subpool_vma(vma); struct hstate *h = hstate_vma(vma); bool subpool_reservation_exists; + bool vma_reservation_exists; bool reservation_exists; + bool charge_cgroup_rsvd; struct folio *folio; - long retval; - map_chg_state map_chg; int ret, idx; struct hugetlb_cgroup *h_cg = NULL; gfp_t gfp = htlb_alloc_mask(h) | __GFP_RETRY_MAYFAIL; idx = hstate_index(h); - /* Whether we need a separate per-vma reservation? */ if (cow_from_owner) { /* * Special case! Since it's a CoW on top of a reserved * page, the private resv map doesn't count. So it cannot * consume the per-vma resv map even if it's reserved. */ - map_chg = MAP_CHG_ENFORCED; + vma_reservation_exists = false; } else { /* * Examine the region/reserve map to determine if the process - * has a reservation for the page to be allocated. A return - * code of zero indicates a reservation exists (no change). + * has a reservation for the page to be allocated and debit the + * reservation. If the number of pages required is 0, + * reservation exists. */ - retval = vma_needs_reservation(h, vma, addr); - if (retval < 0) + int npages_req = vma_needs_reservation(h, vma, addr); + + if (npages_req < 0) return ERR_PTR(-ENOMEM); - map_chg = retval ? MAP_CHG_NEEDED : MAP_CHG_REUSE; + + vma_reservation_exists = npages_req == 0; } /* - * Whether we need a separate global reservation? - * - * Processes that did not create the mapping will have no - * reserves as indicated by the region/reserve map. Check - * that the allocation will not exceed the subpool limit. - * Or if it can get one from the pool reservation directly. + * Debit subpool only if a vma reservation does not exist. If + * vma_reservation_exists, the vma reservation was either moved from the + * subpool or taken directly from hstate in hugetlb_reserve_pages() */ subpool_reservation_exists = false; - if (map_chg) { + if (!vma_reservation_exists) { int npages_req = hugepage_subpool_get_pages(spool, 1); if (npages_req < 0) @@ -3047,13 +3027,16 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma, subpool_reservation_exists = npages_req == 0; } - reservation_exists = !map_chg || subpool_reservation_exists; + + reservation_exists = vma_reservation_exists || subpool_reservation_exists; /* - * If this allocation is not consuming a per-vma reservation, - * charge the hugetlb cgroup now. + * If a vma_reservation_exists, we can skip charging hugetlb + * reservations since that was charged in hugetlb_reserve_pages() when + * the reservation was recorded on the resv_map. */ - if (map_chg) { + charge_cgroup_rsvd = !vma_reservation_exists; + if (charge_cgroup_rsvd) { ret = hugetlb_cgroup_charge_cgroup_rsvd( idx, pages_per_huge_page(h), &h_cg); if (ret) @@ -3091,10 +3074,8 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma, } hugetlb_cgroup_commit_charge(idx, pages_per_huge_page(h), h_cg, folio); - /* If allocation is not consuming a reservation, also store the - * hugetlb_cgroup pointer on the page. - */ - if (map_chg) { + + if (charge_cgroup_rsvd) { hugetlb_cgroup_commit_charge_rsvd(idx, pages_per_huge_page(h), h_cg, folio); } @@ -3103,25 +3084,27 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma, hugetlb_set_folio_subpool(folio, spool); - if (map_chg != MAP_CHG_ENFORCED) { - /* commit() is only needed if the map_chg is not enforced */ - retval = vma_commit_reservation(h, vma, addr); + /* If vma accounting wasn't bypassed earlier, follow up with commit. */ + if (!cow_from_owner) { + int ret = vma_commit_reservation(h, vma, addr); /* - * Check for possible race conditions. When it happens.. - * The page was added to the reservation map between - * vma_needs_reservation and vma_commit_reservation. - * This indicates a race with hugetlb_reserve_pages. + * If there is a discrepancy in reservation status between the + * time of vma_needs_reservation() and vma_commit_reservation(), + * then there the page must have been added to the reservation + * map between vma_needs_reservation() and + * vma_commit_reservation(). + * * Adjust for the subpool count incremented above AND * in hugetlb_reserve_pages for the same page. Also, * the reservation count added in hugetlb_reserve_pages * no longer applies. */ - if (unlikely(map_chg == MAP_CHG_NEEDED && retval == 0)) { + if (unlikely(!vma_reservation_exists && ret == 0)) { long rsv_adjust; rsv_adjust = hugepage_subpool_put_pages(spool, 1); hugetlb_acct_memory(h, -rsv_adjust); - if (map_chg) { + if (charge_cgroup_rsvd) { spin_lock_irq(&hugetlb_lock); hugetlb_cgroup_uncharge_folio_rsvd( hstate_index(h), pages_per_huge_page(h), @@ -3149,14 +3132,15 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma, out_uncharge_cgroup: hugetlb_cgroup_uncharge_cgroup(idx, pages_per_huge_page(h), h_cg); out_uncharge_cgroup_reservation: - if (map_chg) + if (charge_cgroup_rsvd) hugetlb_cgroup_uncharge_cgroup_rsvd(idx, pages_per_huge_page(h), h_cg); out_subpool_put: - if (map_chg) + if (!vma_reservation_exists) hugepage_subpool_put_pages(spool, 1); out_end_reservation: - if (map_chg != MAP_CHG_ENFORCED) + /* If vma accounting wasn't bypassed earlier, cleanup. */ + if (!cow_from_owner) vma_end_reservation(h, vma, addr); return ERR_PTR(-ENOSPC); } -- 2.49.0.1045.g170613ef41-goog