Messages in this thread | | | Date | Mon, 5 Feb 2024 15:47:31 +0100 | From | Michal Hocko <> | Subject | Re: [PATCH v2] mm: hugetlb: improve the handling of hugetlb allocation failure for freed or in-use hugetlb |
| |
On Mon 05-02-24 20:50:51, Baolin Wang wrote: > When handling the freed hugetlb or in-use hugetlb, we should ignore the > failure of alloc_buddy_hugetlb_folio() to dissolve the old hugetlb successfully, > since we did not use the new allocated hugetlb in this 2 cases. Moreover, > moving the allocation into the free hugetlb handling branch.
The changelog is a bit hard for me to understand. What about the following instead? alloc_and_dissolve_hugetlb_folio preallocates a new huge page before it takes hugetlb_lock. In 3 out of 4 cases the page is not really used and therefore the newly allocated page is just freed right away. This is wasteful and it might cause pre-mature failures in those cases.
Address that by moving the allocation down to the only case (hugetlb page is really in the free pages pool). We need to drop hugetlb_lock to do so and therefore need to recheck the page state after regaining it.
The patch is more of a cleanup than an actual fix to an existing problem. There are no known reports about pre-mature failures.
[...]
> @@ -3075,6 +3063,24 @@ static int alloc_and_dissolve_hugetlb_folio(struct hstate *h, > cond_resched(); > goto retry; > } else { > + if (!new_folio) { > + spin_unlock_irq(&hugetlb_lock); > + /* > + * Before dissolving the free hugetlb, we need to allocate > + * a new one for the pool to remain stable. Here, we > + * allocate the folio and 'prep' it by doing everything > + * but actually updating counters and adding to the pool. > + * This simplifies and let us do most of the processing > + * under the lock. > + */
This comment is not really needed anymore IMHO.
> + new_folio = alloc_buddy_hugetlb_folio(h, gfp_mask, nid, > + NULL, NULL); > + if (!new_folio) > + return -ENOMEM; > + __prep_new_hugetlb_folio(h, new_folio); > + goto retry; > + } > + > /* > * Ok, old_folio is still a genuine free hugepage. Remove it from > * the freelist and decrease the counters. These will be
-- Michal Hocko SUSE Labs
| |