hugetlb: Acquire the i_mmap_lock before walking the prio_tree to unmap a page V2
Subsystems (2)
PATCH 1/1 hugetlb: Acquire the i_mmap_lock before walking the prio_tree to unmap a page V2
Changelog since V1
o Delete stupid comment from the description
When the owner of a mapping fails COW because a child process is holding a reference, the children VMAs are walked and the page is unmapped. The i_mmap_lock is taken for the unmapping of the page but not the walking of the prio_tree. In theory, that tree could be changing if the lock is not held. This patch takes the i_mmap_lock properly for the duration of the prio_tree walk.
--- mm/hugetlb.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)
mm/hugetlb.c +8 -1
@@ -1906,6 +1906,12 @@ static int unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma, + (vma->vm_pgoff >>PAGE_SHIFT ); mapping = (struct address_space *)page_private (page); +/* +* Take the mapping lock for the duration of the table walk. As +* this mapping should be shared between all the VMAs, +* __unmap_hugepage_range() is called as the lock is already held +*/ +spin_lock (&mapping->i_mmap_lock); vma_prio_tree_foreach(iter_vma, &iter, &mapping->i_mmap, pgoff, pgoff) {/* Do not unmap the current VMA */ if (iter_vma == vma)
@@ -1919,10 +1925,11 @@ static int unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,* from the time of fork. This would look like data corruption */ if (!is_vma_resv_set (iter_vma,HPAGE_RESV_OWNER )) -unmap_hugepage_range (iter_vma, +__unmap_hugepage_range (iter_vma, address, address +huge_page_size (h), page); } +spin_unlock (&mapping->i_mmap_lock);return 1 ; }
Quoted 10 lines · Mel Gorman
Changelog since V1
o Delete stupid comment from the descriptionWhen the owner of a mapping fails COW because a child process is holding a reference, the children VMAs are walked and the page is unmapped. The
i_mmap_lockis taken for the unmapping of the page but not the walking of theprio_tree. In theory, that tree could be changing if the lock is not held. This patch takes thei_mmap_lockproperly for the duration of theprio_treewalk.
Quoted 2 lines
[[email protected]: Spotted the problem in the first place]Signed-off-by Mel Gorman <[email protected]>
(and Andrew has already put this version into mmotm, thanks)
Quoted 34 lines
--- mm/hugetlb.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)mm/hugetlb.c +8
-1@@ -1906,6 +1906,12 @@ static int unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma, + (vma->vm_pgoff >>PAGE_SHIFT ); mapping = (struct address_space *)page_private (page); +/* +* Take the mapping lock for the duration of the table walk. As +* this mapping should be shared between all the VMAs, +* __unmap_hugepage_range() is called as the lock is already held +*/ +spin_lock (&mapping->i_mmap_lock); vma_prio_tree_foreach(iter_vma, &iter, &mapping->i_mmap, pgoff, pgoff) {/* Do not unmap the current VMA */ if (iter_vma == vma)@@ -1919,10 +1925,11 @@ static int unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,* from the time of fork. This would look like data corruption */ if (!is_vma_resv_set (iter_vma,HPAGE_RESV_OWNER )) -unmap_hugepage_range (iter_vma, +__unmap_hugepage_range (iter_vma, address, address +huge_page_size (h), page); } +spin_unlock (&mapping->i_mmap_lock);return 1 ; }