On Fri, Oct 11, 2024 at 11:22:36PM +0000, Ackerley Tng wrote: > Replace arguments avoid_reserve and chg in dequeue_hugetlb_folio_vma() > so dequeue_hugetlb_folio_vma() is more understandable. > > The new argument, use_hstate_resv, indicates whether the folio to be > dequeued should be taken from reservations in hstate. > > If use_hstate_resv is true, the folio to be dequeued should be taken > from reservations in hstate and hence h->resv_huge_pages is > decremented, and the folio is marked so that the reservation is > restored. > > If use_hstate_resv is false, then a folio needs to be taken from the > pool and hence there must exist available_huge_pages(h), failing > which, goto err. > > The bool use_hstate_resv can be reused within > dequeue_hugetlb_folio_vma()'s caller, alloc_hugetlb_folio(). > > No functional changes are intended. > > As proof, the original two if conditions > > !vma_has_reserves(vma, chg) && !available_huge_pages(h) > > and > > avoid_reserve && !available_huge_pages(h) > > can be combined into > > (avoid_reserve || !vma_has_reserves(vma, chg)) > && !available_huge_pages(h). > > Applying de Morgan's theorem on > > avoid_reserve || !vma_has_reserves(vma, chg) > > yields > > !avoid_reserve && vma_has_reserves(vma, chg), > > hence the simplification is correct. > > Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx> I like this, it is a nice simplification and hugetlb revervation mechanism is already hard enough to follow. As already mentioned, Sean's changes look easier to follow. Reviewed-by: Oscar Salvador <osalvador@xxxxxxx> On a side note, we might want to convert the 'avoid_reserve' param from alloc_hugetlb_folio into a bool, as we are using it exactly like that, so it would seem more natutal. > --- > mm/hugetlb.c | 33 +++++++++++---------------------- > 1 file changed, 11 insertions(+), 22 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 190fa05635f4..73165c670739 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -1281,8 +1281,9 @@ static bool vma_has_reserves(struct vm_area_struct *vma, long chg) > } > > /* > - * Only the process that called mmap() has reserves for > - * private mappings. > + * Only the process that called mmap() has reserves for private > + * mappings. A child process with MAP_PRIVATE mappings created by their > + * parent have no page reserves. 'page reserves' looks odd. I would say 'reservations'. -- Oscar Salvador SUSE Labs
