hugetlb: only adjust reservation during unmapping if mapcount is 0
Subsystems (2)
PATCH 1/1 hugetlb: only adjust reservation during unmapping if mapcount is 0
Since df7a6d1f6405, __unmap_hugepage_range can adjust reservations. In the case of folio mapped in both a parent and a child, if the parent unmaps the range first, the reservation adjustment will result in an underflow of the reserved count. Once the child unmaps the range, the count is restored.
This patch changes __unmap_hugepage_range() so it now checks the mapcount before adjusting the reservation.
--- mm/hugetlb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/hugetlb.c +1 -1
@@ -5252,7 +5252,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,spin_lock_irq (&hugetlb_lock);if (!h->surplus_huge_pages &&__vma_private_lock (vma) && -folio_test_anon (folio)) { +folio_mapcount (folio) ==0 &&folio_test_anon (folio)) {folio_set_hugetlb_restore_reserve (folio);/* Reservation to be adjusted after the spin lock */ adjust_reservation = true;
Signature · 5 lines
2.39.1 -- Guillaume Morin <[email protected]>
Quoted 2 lines · Guillaume Morin
This patch changes
__unmap_hugepage_range()so it now checks the mapcount before adjusting the reservation.
Can you please rephrase this in imperative setence.
Quoted 3 lines
Fixes df7a6d1f6405 ("mm/hugetlb: restore the reservation if needed")Signed-off-by Guillaume Morin <[email protected]>
mm/hugetlb.c void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
Quoted 5 lines
mm/hugetlb.c +0
-0@@ -5252,7 +5252,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
There are some comments before this section, that describe this code.
Could a short note be added for the new one, so the intent survives? Something like "restore only on the last unmap, otherwise the owner empties its resv map while a child still maps the folio" would do. One line is enough.
mm/hugetlb.c void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma, · line 5258
Quoted 7 lines
mm/hugetlb.c +1
-1spin_lock_irq (&hugetlb_lock);if (!h->surplus_huge_pages &&__vma_private_lock (vma) && -folio_test_anon (folio)) { +folio_mapcount (folio) ==0 &&folio_test_anon (folio)) {folio_set_hugetlb_restore_reserve (folio);/* Reservation to be adjusted after the spin lock */ adjust_reservation = true;
For what it is worth, I confirmed the original df7a6d1f6405 case is
preserved: a singly mapped folio still goes 1 -> 0 across hugetlb_remove_rmap(), so the MADV_DONTNEED restore still fires.
Thanks for the fix,
--breno
David Hildenbrand (Arm)
reviewerQuoted 24 lines · Guillaume Morin
Since
df7a6d1f6405,__unmap_hugepage_rangecan adjust reservations. In the case of folio mapped in both a parent and a child, if the parent unmaps the range first, the reservation adjustment will result in an underflow of the reserved count. Once the child unmaps the range, the count is restored.
This patch changes__unmap_hugepage_range()so it now checks the mapcount before adjusting the reservation.Fixes df7a6d1f6405 ("mm/hugetlb: restore the reservation if needed")Signed-off-by Guillaume Morin <[email protected]>Cc Muchun Song <[email protected]>Cc Breno Leitao <[email protected]>Cc Rik van Riel <[email protected]>Cc <[email protected]>--- mm/hugetlb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/hugetlb.c +0
-0
mm/hugetlb.c void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma, · line 5255
Quoted 5 lines
mm/hugetlb.c +1
-1@@ -5252,7 +5252,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,spin_lock_irq (&hugetlb_lock);if (!h->surplus_huge_pages &&__vma_private_lock (vma) && -folio_test_anon (folio)) { +folio_mapcount (folio) ==0 &&folio_test_anon (folio)) {
There is folio_mapped(), can that be used instead?
Signature · 3 lines
Cheers, David
Quoted 27 lines · David Hildenbrand (Arm)
Quoted 25 lines · Guillaume Morin
Since
df7a6d1f6405,__unmap_hugepage_rangecan adjust reservations. In the case of folio mapped in both a parent and a child, if the parent unmaps the range first, the reservation adjustment will result in an underflow of the reserved count. Once the child unmaps the range, the count is restored.
This patch changes__unmap_hugepage_range()so it now checks the mapcount before adjusting the reservation.Fixes df7a6d1f6405 ("mm/hugetlb: restore the reservation if needed")Signed-off-by Guillaume Morin <[email protected]>Cc Muchun Song <[email protected]>Cc Breno Leitao <[email protected]>Cc Rik van Riel <[email protected]>Cc <[email protected]>--- mm/hugetlb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/hugetlb.c +0
-0@@ -5252,7 +5252,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
Quoted 6 lines
mm/hugetlb.c void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma, · line 5255
Quoted 4 lines
mm/hugetlb.c +1
-1spin_lock_irq (&hugetlb_lock);if (!h->surplus_huge_pages &&__vma_private_lock (vma) && -folio_test_anon (folio)) { +folio_mapcount (folio) ==0 &&folio_test_anon (folio)) {There is
folio_mapped(), can that be used instead?
Fixed in v3
Signature · 1 line
Guillaume Morin <[email protected]>