Skip to content

Commit 87bf91d

Browse files
mjkravetztorvalds
authored andcommitted
hugetlbfs: Use i_mmap_rwsem to address page fault/truncate race
hugetlbfs page faults can race with truncate and hole punch operations. Current code in the page fault path attempts to handle this by 'backing out' operations if we encounter the race. One obvious omission in the current code is removing a page newly added to the page cache. This is pretty straight forward to address, but there is a more subtle and difficult issue of backing out hugetlb reservations. To handle this correctly, the 'reservation state' before page allocation needs to be noted so that it can be properly backed out. There are four distinct possibilities for reservation state: shared/reserved, shared/no-resv, private/reserved and private/no-resv. Backing out a reservation may require memory allocation which could fail so that needs to be taken into account as well. Instead of writing the required complicated code for this rare occurrence, just eliminate the race. i_mmap_rwsem is now held in read mode for the duration of page fault processing. Hold i_mmap_rwsem in write mode when modifying i_size. In this way, truncation can not proceed when page faults are being processed. In addition, i_size will not change during fault processing so a single check can be made to ensure faults are not beyond (proposed) end of file. Faults can still race with hole punch, but that race is handled by existing code and the use of hugetlb_fault_mutex. With this modification, checks for races with truncation in the page fault path can be simplified and removed. remove_inode_hugepages no longer needs to take hugetlb_fault_mutex in the case of truncation. Comments are expanded to explain reasoning behind locking. Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Cc: Andrea Arcangeli <aarcange@redhat.com> Cc: "Aneesh Kumar K . V" <aneesh.kumar@linux.vnet.ibm.com> Cc: Davidlohr Bueso <dave@stgolabs.net> Cc: Hugh Dickins <hughd@google.com> Cc: "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com> Cc: Michal Hocko <mhocko@kernel.org> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> Cc: Prakash Sangappa <prakash.sangappa@oracle.com> Link: http://lkml.kernel.org/r/20200316205756.146666-3-mike.kravetz@oracle.com Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent c0d0381 commit 87bf91d

File tree

2 files changed

+31
-20
lines changed

2 files changed

+31
-20
lines changed

fs/hugetlbfs/inode.c

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -393,10 +393,9 @@ hugetlb_vmdelete_list(struct rb_root_cached *root, pgoff_t start, pgoff_t end)
393393
* In this case, we first scan the range and release found pages.
394394
* After releasing pages, hugetlb_unreserve_pages cleans up region/reserv
395395
* maps and global counts. Page faults can not race with truncation
396-
* in this routine. hugetlb_no_page() prevents page faults in the
397-
* truncated range. It checks i_size before allocation, and again after
398-
* with the page table lock for the page held. The same lock must be
399-
* acquired to unmap a page.
396+
* in this routine. hugetlb_no_page() holds i_mmap_rwsem and prevents
397+
* page faults in the truncated range by checking i_size. i_size is
398+
* modified while holding i_mmap_rwsem.
400399
* hole punch is indicated if end is not LLONG_MAX
401400
* In the hole punch case we scan the range and release found pages.
402401
* Only when releasing a page is the associated region/reserv map
@@ -436,7 +435,15 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart,
436435

437436
index = page->index;
438437
hash = hugetlb_fault_mutex_hash(mapping, index);
439-
mutex_lock(&hugetlb_fault_mutex_table[hash]);
438+
if (!truncate_op) {
439+
/*
440+
* Only need to hold the fault mutex in the
441+
* hole punch case. This prevents races with
442+
* page faults. Races are not possible in the
443+
* case of truncation.
444+
*/
445+
mutex_lock(&hugetlb_fault_mutex_table[hash]);
446+
}
440447

441448
/*
442449
* If page is mapped, it was faulted in after being
@@ -479,7 +486,8 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart,
479486
}
480487

481488
unlock_page(page);
482-
mutex_unlock(&hugetlb_fault_mutex_table[hash]);
489+
if (!truncate_op)
490+
mutex_unlock(&hugetlb_fault_mutex_table[hash]);
483491
}
484492
huge_pagevec_release(&pvec);
485493
cond_resched();
@@ -517,8 +525,8 @@ static int hugetlb_vmtruncate(struct inode *inode, loff_t offset)
517525
BUG_ON(offset & ~huge_page_mask(h));
518526
pgoff = offset >> PAGE_SHIFT;
519527

520-
i_size_write(inode, offset);
521528
i_mmap_lock_write(mapping);
529+
i_size_write(inode, offset);
522530
if (!RB_EMPTY_ROOT(&mapping->i_mmap.rb_root))
523531
hugetlb_vmdelete_list(&mapping->i_mmap, pgoff, 0);
524532
i_mmap_unlock_write(mapping);
@@ -640,7 +648,11 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
640648
/* addr is the offset within the file (zero based) */
641649
addr = index * hpage_size;
642650

643-
/* mutex taken here, fault path and hole punch */
651+
/*
652+
* fault mutex taken here, protects against fault path
653+
* and hole punch. inode_lock previously taken protects
654+
* against truncation.
655+
*/
644656
hash = hugetlb_fault_mutex_hash(mapping, index);
645657
mutex_lock(&hugetlb_fault_mutex_table[hash]);
646658

mm/hugetlb.c

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3929,16 +3929,17 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm,
39293929
}
39303930

39313931
/*
3932-
* Use page lock to guard against racing truncation
3933-
* before we get page_table_lock.
3932+
* We can not race with truncation due to holding i_mmap_rwsem.
3933+
* i_size is modified when holding i_mmap_rwsem, so check here
3934+
* once for faults beyond end of file.
39343935
*/
3936+
size = i_size_read(mapping->host) >> huge_page_shift(h);
3937+
if (idx >= size)
3938+
goto out;
3939+
39353940
retry:
39363941
page = find_lock_page(mapping, idx);
39373942
if (!page) {
3938-
size = i_size_read(mapping->host) >> huge_page_shift(h);
3939-
if (idx >= size)
3940-
goto out;
3941-
39423943
/*
39433944
* Check for page in userfault range
39443945
*/
@@ -4044,10 +4045,6 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm,
40444045
}
40454046

40464047
ptl = huge_pte_lock(h, mm, ptep);
4047-
size = i_size_read(mapping->host) >> huge_page_shift(h);
4048-
if (idx >= size)
4049-
goto backout;
4050-
40514048
ret = 0;
40524049
if (!huge_pte_none(huge_ptep_get(ptep)))
40534050
goto backout;
@@ -4151,8 +4148,10 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
41514148

41524149
/*
41534150
* Acquire i_mmap_rwsem before calling huge_pte_alloc and hold
4154-
* until finished with ptep. This prevents huge_pmd_unshare from
4155-
* being called elsewhere and making the ptep no longer valid.
4151+
* until finished with ptep. This serves two purposes:
4152+
* 1) It prevents huge_pmd_unshare from being called elsewhere
4153+
* and making the ptep no longer valid.
4154+
* 2) It synchronizes us with i_size modifications during truncation.
41564155
*
41574156
* ptep could have already be assigned via huge_pte_offset. That
41584157
* is OK, as huge_pte_alloc will return the same value unless

0 commit comments

Comments
 (0)