[PATCH 6.3.y] mm/hugetlb: revert use of page_cache_next_miss()

Sidhartha Kumar posted 1 patch 2 years, 7 months ago
fs/hugetlbfs/inode.c |  8 +++-----
mm/hugetlb.c         | 11 +++++------
2 files changed, 8 insertions(+), 11 deletions(-)
[PATCH 6.3.y] mm/hugetlb: revert use of page_cache_next_miss()
Posted by Sidhartha Kumar 2 years, 7 months ago
commit fd4aed8d985a3236d0877ff6d0c80ad39d4ce81a upstream

Ackerley Tng reported an issue with hugetlbfs fallocate as noted in the
Closes tag.  The issue showed up after the conversion of hugetlb page
cache lookup code to use page_cache_next_miss.  User visible effects are:

- hugetlbfs fallocate incorrectly returns -EEXIST if pages are presnet
  in the file.
- hugetlb pages will not be included in core dumps if they need to be
  brought in via GUP.
- userfaultfd UFFDIO_COPY will not notice pages already present in the
  cache.  It may try to allocate a new page and potentially return
  ENOMEM as opposed to EEXIST.

Revert the use page_cache_next_miss() in hugetlb code.

The upstream fix[2] cannot be used used directly as the return value for
filemap_get_folio() has been changed between 6.3 and upstream.

Closes: https://lore.kernel.org/linux-mm/cover.1683069252.git.ackerleytng@google.com
Fixes: d0ce0e47b323 ("mm/hugetlb: convert hugetlb fault paths to use alloc_hugetlb_folio()")
Cc: <stable@vger.kernel.org> #v6.3
Reported-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>

[1] https://lore.kernel.org/linux-mm/cover.1683069252.git.ackerleytng@google.com/
[2] https://lore.kernel.org/lkml/20230621230255.GD4155@monkey/
---

 fs/hugetlbfs/inode.c |  8 +++-----
 mm/hugetlb.c         | 11 +++++------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 9062da6da5675..586767afb4cdb 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -821,7 +821,6 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
 		 */
 		struct folio *folio;
 		unsigned long addr;
-		bool present;
 
 		cond_resched();
 
@@ -845,10 +844,9 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
 		mutex_lock(&hugetlb_fault_mutex_table[hash]);
 
 		/* See if already present in mapping to avoid alloc/free */
-		rcu_read_lock();
-		present = page_cache_next_miss(mapping, index, 1) != index;
-		rcu_read_unlock();
-		if (present) {
+		folio = filemap_get_folio(mapping, index);
+		if (folio) {
+			folio_put(folio);
 			mutex_unlock(&hugetlb_fault_mutex_table[hash]);
 			hugetlb_drop_vma_policy(&pseudo_vma);
 			continue;
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 245038a9fe4ea..29ab27d2a3ef5 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5666,13 +5666,12 @@ static bool hugetlbfs_pagecache_present(struct hstate *h,
 {
 	struct address_space *mapping = vma->vm_file->f_mapping;
 	pgoff_t idx = vma_hugecache_offset(h, vma, address);
-	bool present;
-
-	rcu_read_lock();
-	present = page_cache_next_miss(mapping, idx, 1) != idx;
-	rcu_read_unlock();
+	struct folio *folio;
 
-	return present;
+	folio = filemap_get_folio(mapping, idx);
+	if (folio)
+		folio_put(folio);
+	return folio != NULL;
 }
 
 int hugetlb_add_to_page_cache(struct folio *folio, struct address_space *mapping,
-- 
2.40.1
Re: [PATCH 6.3.y] mm/hugetlb: revert use of page_cache_next_miss()
Posted by Greg KH 2 years, 7 months ago
On Thu, Jun 29, 2023 at 05:18:17PM -0400, Sidhartha Kumar wrote:
> commit fd4aed8d985a3236d0877ff6d0c80ad39d4ce81a upstream
> 
> Ackerley Tng reported an issue with hugetlbfs fallocate as noted in the
> Closes tag.  The issue showed up after the conversion of hugetlb page
> cache lookup code to use page_cache_next_miss.  User visible effects are:
> 
> - hugetlbfs fallocate incorrectly returns -EEXIST if pages are presnet
>   in the file.
> - hugetlb pages will not be included in core dumps if they need to be
>   brought in via GUP.
> - userfaultfd UFFDIO_COPY will not notice pages already present in the
>   cache.  It may try to allocate a new page and potentially return
>   ENOMEM as opposed to EEXIST.
> 
> Revert the use page_cache_next_miss() in hugetlb code.
> 
> The upstream fix[2] cannot be used used directly as the return value for
> filemap_get_folio() has been changed between 6.3 and upstream.
> 
> Closes: https://lore.kernel.org/linux-mm/cover.1683069252.git.ackerleytng@google.com
> Fixes: d0ce0e47b323 ("mm/hugetlb: convert hugetlb fault paths to use alloc_hugetlb_folio()")
> Cc: <stable@vger.kernel.org> #v6.3
> Reported-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
> 
> [1] https://lore.kernel.org/linux-mm/cover.1683069252.git.ackerleytng@google.com/
> [2] https://lore.kernel.org/lkml/20230621230255.GD4155@monkey/
> ---
> 
>  fs/hugetlbfs/inode.c |  8 +++-----
>  mm/hugetlb.c         | 11 +++++------
>  2 files changed, 8 insertions(+), 11 deletions(-)

Now queued up, thanks.

greg k-h