On Wed, May 14, 2025 at 4:43 PM Ackerley Tng <ackerleytng@xxxxxxxxxx> wrote: > > 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 Hi Ackerley, Can you help me better understand how useful the refactors in this and the preceding patch are for the series as a whole? It seems like you and Peter had two different, but mostly equivalent, directions with how this code should be refactored[1]. Do you gain much by replacing Peter's refactoring strategy? If it's mostly a stylistic thing, maybe it would be better to remove these patches just to get the number of patches to review down. The logic in these two patches looks good to me, and I think I do slightly prefer your approach. But if we could drop these patches (i.e., mail them separately), that's probably better. [1]: https://lore.kernel.org/linux-mm/20250107204002.2683356-5-peterx@xxxxxxxxxx/
