hugetlb: only adjust reservation during unmapping if mapcount is 0
Subsystems (2)
PATCH v2 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.
Change __unmap_hugepage_range() to check the mapcount before adjusting the reservation.
---
Changes in v2:
- Document the intent in comment
- Tweak the commit message
mm/hugetlb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
mm/hugetlb.c +4 -2
@@ -5244,7 +5244,9 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,/* * Restore the reservation for anonymous page, otherwise the -* backing page could be stolen by someone. +* backing page could be stolen by someone. Restore only on the +* last unmap, otherwise the owner could empty its resv map +* while the folio is still mapped by a child. * If there we are freeing a surplus, do not set the restore * reservation bit. */
@@ -5252,7 +5254,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 14 lines · Guillaume Morin
mm/hugetlb.c +0
-0@@ -5252,7 +5254,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)&&mm/hugetlb.c +1
-1- folio_test_anon(folio)) { + folio_mapcount(folio) == 0 &&
Quoted 5 lines
folio_test_anon(folio)) {folio_set_hugetlb_restore_reserve(folio); /* Reservation to be adjusted after the spin lock */
adjust_reservation= true;
The safety of this seems to depend on the caller of __unmap_hugepage_range() holding i_mmap_lock_write(mapping)
Could you document that with a quick comment above this block, and also add a lockdep_assert to the start of __unmap_hugepage_range() to make sure that the mapping is indeed locked for write?
This seems like a fairly fragile area, and the locking requirements should not be left undocumented.
Signature · 1 line
All Rights Reversed.
Quoted 25 lines · Rik van Riel
Quoted 14 lines · Guillaume Morin
mm/hugetlb.c +0
-0@@ -5252,7 +5254,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)&&mm/hugetlb.c +1
-1- folio_test_anon(folio)) { + folio_mapcount(folio) == 0 &&Quoted 5 lines
folio_test_anon(folio)) {folio_set_hugetlb_restore_reserve(folio); /* Reservation to be adjusted after the spin lock */
adjust_reservation= true;The safety of this seems to depend on the caller of
__unmap_hugepage_range()holdingi_mmap_lock_write(mapping)
Quoted 7 lines
Could you document that with a quick comment above this block, and also add a
lockdep_assertto the start of__unmap_hugepage_range()to make sure that the mapping is indeed locked for write?This seems like a fairly fragile area, and the locking requirements should not be left undocumented.
Fixed in v3
Signature · 1 line
Guillaume Morin <[email protected]>