Re: [PATCH v1 3/5] mm/shmem: make SGP_NOALLOC succeed on hole like SGP_READ
From: Chi Zhiling
Date: Mon May 25 2026 - 04:16:54 EST
On 5/25/26 3:10 PM, Baolin Wang wrote:
On 5/20/26 6:15 PM, Chi Zhiling wrote:
From: Chi Zhiling <chizhiling@xxxxxxxxxx>
Change SGP_NOALLOC to return 0 with NULL folio on hole, matching
SGP_READ behavior. This simplifies the sgp_type handling by unifying
hole semantics across these types.
Previously, SGP_NOALLOC returned -ENOENT on hole, while SGP_READ
returned 0. This inconsistency required special handling in callers
like khugepaged and userfaultfd.
This patch doesn't seem to be a performance optimization, and I'm not convinced.
Hi, baolin
This is not an optimization patch, and it is not related to this patch series either.
After this change:
- khugepaged: behavior unchanged (checks both error and NULL folio)
But this adds an extra check to khugepaged, and I'm not sure it's worth it.
You are right, I will drop this patch in v2.
- userfaultfd: behavior unchanged (both -ENOENT and NULL are converted
to -EFAULT before returning to userspace)
No, this will break userfaultfd, cause userfaultfd currently returns an error code directly in this case (please update to the latest codebase.). See:
static struct folio *shmem_get_folio_noalloc(struct inode *inode, pgoff_t pgoff)
{
struct folio *folio;
int err;
err = shmem_get_folio(inode, pgoff, 0, &folio, SGP_NOALLOC);
if (err)
return ERR_PTR(err);
return folio;
}
The return value of shmem_get_folio_noalloc() will change, but it seems this does not affect the logic in mfill_atomic_pte_continue().
```
folio = ops->get_folio_noalloc(inode, pgoff);
/* Our caller expects us to return -EFAULT if we failed to find folio */
if (IS_ERR_OR_NULL(folio))
return -EFAULT;
```
No matter whether get_folio_noalloc() returns an error or NULL, the error code is converted to -EFAULT here, and mfill_atomic_pte_continue() is currently the only caller of get_folio_noalloc().
Did I miss something?
Thanks,
Signed-off-by: Chi Zhiling <chizhiling@xxxxxxxxxx>
---
include/linux/shmem_fs.h | 2 +-
mm/khugepaged.c | 2 +-
mm/shmem.c | 9 +++------
3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
index 93a0ba872ebe..d461713c095b 100644
--- a/include/linux/shmem_fs.h
+++ b/include/linux/shmem_fs.h
@@ -165,7 +165,7 @@ extern unsigned long shmem_partial_swap_usage(struct address_space *mapping,
/* Flag allocation requirements to shmem_get_folio */
enum sgp_type {
SGP_READ, /* don't exceed i_size, don't allocate page */
- SGP_NOALLOC, /* similar, but fail on hole or use fallocated page */
+ SGP_NOALLOC, /* like SGP_READ, but accept fallocated page */
SGP_CACHE, /* don't exceed i_size, may allocate page */
SGP_WRITE, /* may exceed i_size, may allocate !Uptodate page */
SGP_FALLOC, /* like SGP_WRITE, but make existing page Uptodate */
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index b8452dbdb043..3309d1c094df 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1950,7 +1950,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
xas_unlock_irq(&xas);
/* swap in or instantiate fallocated page */
if (shmem_get_folio(mapping->host, index, 0,
- &folio, SGP_NOALLOC)) {
+ &folio, SGP_NOALLOC) || !folio) {
result = SCAN_FAIL;
goto xa_unlocked;
}
diff --git a/mm/shmem.c b/mm/shmem.c
index 3b5dc21b323c..458853c506ea 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2524,14 +2524,11 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index,
}
/*
- * SGP_READ: succeed on hole, with NULL folio, letting caller zero.
- * SGP_NOALLOC: fail on hole, with NULL folio, letting caller fail.
+ * SGP_READ/SGP_NOALLOC: succeed on hole, with NULL folio.
*/
*foliop = NULL;
- if (sgp == SGP_READ)
+ if (sgp <= SGP_NOALLOC)
return 0;
- if (sgp == SGP_NOALLOC)
- return -ENOENT;
/*
* Fast cache lookup and swap lookup did not find it: allocate.
@@ -2657,7 +2654,7 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index,
*
* When no folio is found, the behavior depends on @sgp:
* - for SGP_READ, *@foliop is %NULL and 0 is returned
- * - for SGP_NOALLOC, *@foliop is %NULL and -ENOENT is returned
+ * - for SGP_NOALLOC, *@foliop is %NULL and 0 is returned
* - for all other flags a new folio is allocated, inserted into the
* page cache and returned locked in @foliop.
*