Re: [PATCH v1 3/5] mm/shmem: make SGP_NOALLOC succeed on hole like SGP_READ

[Baolin Wang]

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.

> 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.

> - 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;
}

> 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.
>    *