This reverts 87bf91d39bb52b688fb411d668fbe7df278b29ae

Commit 87bf91d39bb5 depends on i_mmap_rwsem being taken during hugetlb
fault processing.  Commit c0d0381ade79 added code to take i_mmap_rwsem
in read mode during fault processing.  However, this was observed to
increase fault processing time by aprox 33%.  To address this, i_mmap_rwsem
will only be taken during fault processing when necessary.  As a result,
i_mmap_rwsem can not be used to synchronize fault and truncate.  In a
subsequent commit, code will be added to detect the race and back out
operations.

Reported-by: kernel test robot <rong.a.c...@intel.com>
Signed-off-by: Mike Kravetz <mike.krav...@oracle.com>
---
 fs/hugetlbfs/inode.c | 28 ++++++++--------------------
 mm/hugetlb.c         | 23 ++++++++++++-----------
 2 files changed, 20 insertions(+), 31 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index ef5313f9c78f..b4bb82815dd4 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -444,9 +444,10 @@ hugetlb_vmdelete_list(struct rb_root_cached *root, pgoff_t 
start, pgoff_t end)
  *     In this case, we first scan the range and release found pages.
  *     After releasing pages, hugetlb_unreserve_pages cleans up region/reserv
  *     maps and global counts.  Page faults can not race with truncation
- *     in this routine.  hugetlb_no_page() holds i_mmap_rwsem and prevents
- *     page faults in the truncated range by checking i_size.  i_size is
- *     modified while holding i_mmap_rwsem.
+ *     in this routine.  hugetlb_no_page() prevents page faults in the
+ *     truncated range.  It checks i_size before allocation, and again after
+ *     with the page table lock for the page held.  The same lock must be
+ *     acquired to unmap a page.
  * hole punch is indicated if end is not LLONG_MAX
  *     In the hole punch case we scan the range and release found pages.
  *     Only when releasing a page is the associated region/reserv map
@@ -486,15 +487,7 @@ static void remove_inode_hugepages(struct inode *inode, 
loff_t lstart,
 
                        index = page->index;
                        hash = hugetlb_fault_mutex_hash(mapping, index);
-                       if (!truncate_op) {
-                               /*
-                                * Only need to hold the fault mutex in the
-                                * hole punch case.  This prevents races with
-                                * page faults.  Races are not possible in the
-                                * case of truncation.
-                                */
-                               mutex_lock(&hugetlb_fault_mutex_table[hash]);
-                       }
+                       mutex_lock(&hugetlb_fault_mutex_table[hash]);
 
                        /*
                         * If page is mapped, it was faulted in after being
@@ -537,8 +530,7 @@ static void remove_inode_hugepages(struct inode *inode, 
loff_t lstart,
                        }
 
                        unlock_page(page);
-                       if (!truncate_op)
-                               mutex_unlock(&hugetlb_fault_mutex_table[hash]);
+                       mutex_unlock(&hugetlb_fault_mutex_table[hash]);
                }
                huge_pagevec_release(&pvec);
                cond_resched();
@@ -576,8 +568,8 @@ static int hugetlb_vmtruncate(struct inode *inode, loff_t 
offset)
        BUG_ON(offset & ~huge_page_mask(h));
        pgoff = offset >> PAGE_SHIFT;
 
-       i_mmap_lock_write(mapping);
        i_size_write(inode, offset);
+       i_mmap_lock_write(mapping);
        if (!RB_EMPTY_ROOT(&mapping->i_mmap.rb_root))
                hugetlb_vmdelete_list(&mapping->i_mmap, pgoff, 0);
        i_mmap_unlock_write(mapping);
@@ -699,11 +691,7 @@ static long hugetlbfs_fallocate(struct file *file, int 
mode, loff_t offset,
                /* addr is the offset within the file (zero based) */
                addr = index * hpage_size;
 
-               /*
-                * fault mutex taken here, protects against fault path
-                * and hole punch.  inode_lock previously taken protects
-                * against truncation.
-                */
+               /* mutex taken here, fault path and hole punch */
                hash = hugetlb_fault_mutex_hash(mapping, index);
                mutex_lock(&hugetlb_fault_mutex_table[hash]);
 
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 57ece74e3aae..5349beda3658 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4322,17 +4322,16 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm,
        }
 
        /*
-        * We can not race with truncation due to holding i_mmap_rwsem.
-        * i_size is modified when holding i_mmap_rwsem, so check here
-        * once for faults beyond end of file.
+        * Use page lock to guard against racing truncation
+        * before we get page_table_lock.
         */
-       size = i_size_read(mapping->host) >> huge_page_shift(h);
-       if (idx >= size)
-               goto out;
-
 retry:
        page = find_lock_page(mapping, idx);
        if (!page) {
+               size = i_size_read(mapping->host) >> huge_page_shift(h);
+               if (idx >= size)
+                       goto out;
+
                /*
                 * Check for page in userfault range
                 */
@@ -4438,6 +4437,10 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm,
        }
 
        ptl = huge_pte_lock(h, mm, ptep);
+       size = i_size_read(mapping->host) >> huge_page_shift(h);
+       if (idx >= size)
+               goto backout;
+
        ret = 0;
        if (!huge_pte_none(huge_ptep_get(ptep)))
                goto backout;
@@ -4541,10 +4544,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct 
vm_area_struct *vma,
 
        /*
         * Acquire i_mmap_rwsem before calling huge_pte_alloc and hold
-        * until finished with ptep.  This serves two purposes:
-        * 1) It prevents huge_pmd_unshare from being called elsewhere
-        *    and making the ptep no longer valid.
-        * 2) It synchronizes us with i_size modifications during truncation.
+        * until finished with ptep.  This prevents huge_pmd_unshare from
+        * being called elsewhere and making the ptep no longer valid.
         *
         * ptep could have already be assigned via huge_pte_offset.  That
         * is OK, as huge_pte_alloc will return the same value unless
-- 
2.25.4

Reply via email to