Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     linux-mm@kvack.org, linux-kernel@vger.kernel.org
Cc:     Muchun Song <songmuchun@bytedance.com>,
        Miaohe Lin <linmiaohe@huawei.com>,
        David Hildenbrand <david@redhat.com>,
        Michal Hocko <mhocko@suse.com>, Peter Xu <peterx@redhat.com>,
        Naoya Horiguchi <naoya.horiguchi@linux.dev>,
        "Aneesh Kumar K . V" <aneesh.kumar@linux.vnet.ibm.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        Davidlohr Bueso <dave@stgolabs.net>,
        Prakash Sangappa <prakash.sangappa@oracle.com>,
        James Houghton <jthoughton@google.com>,
        Mina Almasry <almasrymina@google.com>,
        Pasha Tatashin <pasha.tatashin@soleen.com>,
        Axel Rasmussen <axelrasmussen@google.com>,
        Ray Fucillo <Ray.Fucillo@intersystems.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Mike Kravetz <mike.kravetz@oracle.com>
Subject: [PATCH 1/8] hugetlbfs: revert use i_mmap_rwsem to address page fault/truncate race
Date:   Wed, 24 Aug 2022 10:57:50 -0700
Message-Id: <20220824175757.20590-2-mike.kravetz@oracle.com>
In-Reply-To: <20220824175757.20590-1-mike.kravetz@oracle.com>
References: <20220824175757.20590-1-mike.kravetz@oracle.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?L9b3H2bGyVYD8eVvDdQfOpF3Fww8E8IAdX1RZ9qNkd6BzBHrODncM51xYXYF?=
 =?us-ascii?Q?6+/TP3eTO2QwGDlrZYoY6p0ZSSYqPsLJklh+ARomiWXWlN2jfmUeck5u3b3A?=
 =?us-ascii?Q?A7vOmtI7sTyuKRAx/IQvyPAcCcYfJtJCLA3bQjnTvilpK7F30RCId+sDu0+R?=
 =?us-ascii?Q?lvhdMGyDp0Au5c062P/H04pdqen7YJnZUZpPCGcb/NSHEwcquiDo4Dl207+v?=
 =?us-ascii?Q?Wa9/aGR0hxoW94r2WxwUNoLnuF1xx8FAFeYYcVk36CDHcru0BgIPkVxW/7bR?=
 =?us-ascii?Q?2bCB9Mz5iCGutNyt2P0xsWSvd0/IkdtQGu9Uvvptgk7qJnXB/ndMcX6OqoYt?=
 =?us-ascii?Q?3nDaAbuHODiAtvcESn10l2hpDPLHIoT+bAMfOFLRug5Lgw9N+Xt+cDWkKW4m?=
 =?us-ascii?Q?i+fHWrTz+QA4J0ET0CYCxhHMl3ALDo85GZVWvwb2vlIxL6LKQjIqBXWSHNM6?=
 =?us-ascii?Q?CQtOLVPZb3EDCheX0iM1o+RHUUu/LuOFaoesCP3NculCQOB8qm5O6PJS7Rvx?=
 =?us-ascii?Q?OdzqZ77JTtiX4dCrZMfb5u19uecr0vTCfYyvl7y/gcqCMCzTCckoC/TGwGzZ?=
 =?us-ascii?Q?R5BKZMKW4dXdIm79KxwhIJrfC+sxBUJImFESCKoUOJ9rEs+QXawETvo2Qb9J?=
 =?us-ascii?Q?LwLn9560+XMytLqxrL3fQHLhQ2pudIFqHtrNu+qBIBWNFRM9gDEE/itet/2Y?=
 =?us-ascii?Q?BRj44GtZC/hEUTCDCyqjYDpKok3IFoMWxiEuhZxmBgL5ksPcs17kIQZ2iJq2?=
 =?us-ascii?Q?gzGmaSFz82prHyC2s/4qJNg5TgHg0SSeVVGVLKuA+p9UZbZWNfJC6ajZeGTW?=
 =?us-ascii?Q?2UAVI60+YLa91kDGS263J/IoWLqfLTPkkXxLctwv6GCWdy8nhzI9HDhCvnM7?=
 =?us-ascii?Q?UQNxcA+zgRwcynKnAW5uBAsUWLWXX6uGpzclBNQmMtLXKuZ/7ktdwZgMGKUA?=
 =?us-ascii?Q?4BJRyLE8QI4xnm5boKfUiHQpAcLN0HK8cjQeq5EwcuFkbde8NkepaSY/GlvP?=
 =?us-ascii?Q?kwzNYp3RxZ9y2h8/KLGtivGe09X+vCUfJqPTW15UfF3apwOruplAxImhs6iQ?=
 =?us-ascii?Q?1WBTvJU1MIEwMTBzHGn+PU6UCkQNJI69h9Xw/BfsUz1fXpCDLsfqP3j3R+70?=
 =?us-ascii?Q?KNAZ71YoCnq8e9KH+//srRM3mtkIdW9oi3JYJCB1ZTgIXatLIHpsZtJAnzWP?=
 =?us-ascii?Q?dV4Fnssvn63fmjAKg4Rz5o8aMiWRses0sEvb5J8cqfsICcDSl+xzJJne2hRi?=
 =?us-ascii?Q?kT2STilLTfMNmpG+3vXBRZFNWW/+x5l+a6FqrfkVGVisK/teFp+kUsStB8cw?=
 =?us-ascii?Q?VhYhUt8QJf/4i7fKBPSG4PkMt5mnnEE00ZRe7yB86JxSOB4MsCIM+y8NJQKL?=
 =?us-ascii?Q?k5jhSUE7xtLgVaeWTj9QnPTokPENVUc4hUWS1yENyDBgm/W252VKdxG9lGCj?=
 =?us-ascii?Q?7hd+3Zjd2GksgNlT9yTChZaCfbOTrKWxJJYXaQ17KDaCn4XVYJvpW4ur7Gi3?=
 =?us-ascii?Q?U+O0SqxEnXCn2St/XCyUnOazrCO172cBdzsX2wJuMwC0sqvJlEnDiLr18Ard?=
 =?us-ascii?Q?SrEddQtMQ+nOIN5w9p09T+wxb/4jEuLKCaVGvU8J8luf/6bQhFagtMUcaHo4?=
 =?us-ascii?Q?Kw=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 381512ad-1c9a-4526-1742-08da85fa3355
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4201.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Aug 2022 17:58:07.6504
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: t7+HNqcezM0cGkV1hBpk5h7rJrXs9E5wmDDvLpgerts4XZcfR3hfvYdYsj+HZAI749+rUfq7ZARo7xMFhG1DeA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR1001MB2251
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-08-24_11,2022-08-22_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0 phishscore=0
 mlxlogscore=999 adultscore=0 mlxscore=0 malwarescore=0 spamscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000
 definitions=main-2208240066
X-Proofpoint-ORIG-GUID: J7FDiQgmc6_cczxmbC4jycd0woQ4ctum
X-Proofpoint-GUID: J7FDiQgmc6_cczxmbC4jycd0woQ4ctum
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Commit c0d0381ade79 ("hugetlbfs: use i_mmap_rwsem for more pmd sharing
synchronization") added code to take i_mmap_rwsem in read mode for the
duration of fault processing.  The use of i_mmap_rwsem to prevent
fault/truncate races depends on this.  However, this has been shown to
cause performance/scaling issues.  As a result, that code will be
reverted.  Since the use i_mmap_rwsem to address page fault/truncate races
depends on this, it must also be reverted.

In a subsequent patch, code will be added to detect the fault/truncate
race and back out operations as required.

Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
 fs/hugetlbfs/inode.c | 30 +++++++++---------------------
 mm/hugetlb.c         | 23 ++++++++++++-----------
 2 files changed, 21 insertions(+), 32 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index f7a5b5124d8a..a32031e751d1 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -419,9 +419,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/reserve
  *	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/reserve map
@@ -451,16 +452,8 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart,
 			u32 hash = 0;
 
 			index = folio->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.
-				 */
-				hash = hugetlb_fault_mutex_hash(mapping, index);
-				mutex_lock(&hugetlb_fault_mutex_table[hash]);
-			}
+			hash = hugetlb_fault_mutex_hash(mapping, index);
+			mutex_lock(&hugetlb_fault_mutex_table[hash]);
 
 			/*
 			 * If folio is mapped, it was faulted in after being
@@ -504,8 +497,7 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart,
 			}
 
 			folio_unlock(folio);
-			if (!truncate_op)
-				mutex_unlock(&hugetlb_fault_mutex_table[hash]);
+			mutex_unlock(&hugetlb_fault_mutex_table[hash]);
 		}
 		folio_batch_release(&fbatch);
 		cond_resched();
@@ -543,8 +535,8 @@ static void 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,
 				      ZAP_FLAG_DROP_MARKER);
@@ -703,11 +695,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 9a72499486c1..70bc7f867bc0 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5575,18 +5575,17 @@ 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:
 	new_page = false;
 	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 */
 		if (userfaultfd_missing(vma)) {
 			ret = hugetlb_handle_userfault(vma, mapping, idx,
@@ -5677,6 +5676,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 pte changed from under us, retry */
 	if (!pte_same(huge_ptep_get(ptep), old_pte))
@@ -5785,10 +5788,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.37.1