[PATCH resend v2] /dev/zero: try to align PMD_SIZE for private mapping

Zhang Qilong posted 1 patch 2 months, 1 week ago
There is a newer version of this series
drivers/char/mem.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
[PATCH resend v2] /dev/zero: try to align PMD_SIZE for private mapping
Posted by Zhang Qilong 2 months, 1 week ago
Attempt to map aligned to huge page size for private mapping which
could achieve performance gains, the mprot_tw4m in libMicro average
execution time on arm64:
  - Test case:        mprot_tw4m
  - Before the patch:   22 us
  - After the patch:    17 us

Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
---
v2:
- Add comments on code suggested by Lorenzo
- Use IS_ENABLED to check THP config

 drivers/char/mem.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 48839958b0b1..c27cc89bd02d 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -525,11 +525,18 @@ static unsigned long get_unmapped_area_zero(struct file *file,
 		 * so as not to confuse shmem with our handle on "/dev/zero".
 		 */
 		return shmem_get_unmapped_area(NULL, addr, len, pgoff, flags);
 	}
 
-	/* Otherwise flags & MAP_PRIVATE: with no shmem object beneath it */
+	/*
+	 * Otherwise flags & MAP_PRIVATE: with no shmem object beneath it,
+	 * attempt to map aligned to huge page size if possible, otherwise we
+	 * fall back to system page size mappings.
+	 */
+	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
+		return thp_get_unmapped_area(file, addr, len, pgoff, flags);
+
 	return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
 #else
 	return -ENOSYS;
 #endif
 }
-- 
2.43.0
Re: [PATCH resend v2] /dev/zero: try to align PMD_SIZE for private mapping
Posted by Lorenzo Stoakes 2 months, 1 week ago
On Wed, Jul 30, 2025 at 05:19:05PM +0800, Zhang Qilong wrote:
> Attempt to map aligned to huge page size for private mapping which
> could achieve performance gains, the mprot_tw4m in libMicro average
> execution time on arm64:
>   - Test case:        mprot_tw4m
>   - Before the patch:   22 us
>   - After the patch:    17 us
>
> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
> ---
> v2:
> - Add comments on code suggested by Lorenzo
> - Use IS_ENABLED to check THP config
>
>  drivers/char/mem.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 48839958b0b1..c27cc89bd02d 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -525,11 +525,18 @@ static unsigned long get_unmapped_area_zero(struct file *file,
>  		 * so as not to confuse shmem with our handle on "/dev/zero".
>  		 */
>  		return shmem_get_unmapped_area(NULL, addr, len, pgoff, flags);
>  	}
>
> -	/* Otherwise flags & MAP_PRIVATE: with no shmem object beneath it */
> +	/*
> +	 * Otherwise flags & MAP_PRIVATE: with no shmem object beneath it,
> +	 * attempt to map aligned to huge page size if possible, otherwise we
> +	 * fall back to system page size mappings.
> +	 */

Nit, but put a space after this as it aplies to both blocks.

> +	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> +		return thp_get_unmapped_area(file, addr, len, pgoff, flags);

As mentioned by Matthew, and as requested originally, please make this an #ifdef
/ #else.

You might then want to factor out the #ifdef CONFIG_MMU (gross) at a higher
level.

I'm sick of us bending over backwards for museum piece (or should be) nommu but
anyway.

Something like

#ifdef CONFIG_MMU
static unsigned long get_unmapped_area_zero(struct file *file,
				unsigned long addr, unsigned long len,
				unsigned long pgoff, unsigned long flags)
{
	return -ENOSYS;
}
#else
...


It's ugly but unfortunately I think necessary.

> +
>  	return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
>  #else
>  	return -ENOSYS;
>  #endif
>  }
> --
> 2.43.0
>

Thanks, Lorenzo
Re: [PATCH resend v2] /dev/zero: try to align PMD_SIZE for private mapping
Posted by Matthew Wilcox 2 months, 1 week ago
On Wed, Jul 30, 2025 at 05:19:05PM +0800, Zhang Qilong wrote:
> +	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> +		return thp_get_unmapped_area(file, addr, len, pgoff, flags);
> +
>  	return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);

I'm sure the build bots will give us a compiler error.
If CONFIG_TRANSPARENT_HUGEPAGE is not enabled, we get:
include/linux/huge_mm.h:#define thp_get_unmapped_area   NULL

and we chose that so that various filesystems can unconditionally set
their .get_unmapped_area method to it.

Which means the cpp will turn this into:

	if (0)
		return NULL(file, addr, len, pgoff, flags);

and the compiler will say:

error: implicit declaration of function ‘NULL’ [-Wimplicit-function-declaration]

Re: [PATCH resend v2] /dev/zero: try to align PMD_SIZE for private mapping
Posted by Lorenzo Stoakes 2 months, 1 week ago
On Wed, Jul 30, 2025 at 01:49:34PM +0100, Matthew Wilcox wrote:
> On Wed, Jul 30, 2025 at 05:19:05PM +0800, Zhang Qilong wrote:
> > +	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> > +		return thp_get_unmapped_area(file, addr, len, pgoff, flags);
> > +
> >  	return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
>
> I'm sure the build bots will give us a compiler error.
> If CONFIG_TRANSPARENT_HUGEPAGE is not enabled, we get:
> include/linux/huge_mm.h:#define thp_get_unmapped_area   NULL
>
> and we chose that so that various filesystems can unconditionally set
> their .get_unmapped_area method to it.
>
> Which means the cpp will turn this into:
>
> 	if (0)
> 		return NULL(file, addr, len, pgoff, flags);
>
> and the compiler will say:
>
> error: implicit declaration of function ‘NULL’ [-Wimplicit-function-declaration]
>

Yeah I did ask explicitly for #ifdef here :)
Re: [PATCH resend v2] /dev/zero: try to align PMD_SIZE for private mapping
Posted by David Hildenbrand 2 months, 1 week ago
On 30.07.25 11:19, Zhang Qilong wrote:
> Attempt to map aligned to huge page size for private mapping which
> could achieve performance gains, the mprot_tw4m in libMicro average
> execution time on arm64:
>    - Test case:        mprot_tw4m
>    - Before the patch:   22 us
>    - After the patch:    17 us
> 
> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
> ---
> v2:
> - Add comments on code suggested by Lorenzo
> - Use IS_ENABLED to check THP config
> 
>   drivers/char/mem.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 48839958b0b1..c27cc89bd02d 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -525,11 +525,18 @@ static unsigned long get_unmapped_area_zero(struct file *file,
>   		 * so as not to confuse shmem with our handle on "/dev/zero".
>   		 */
>   		return shmem_get_unmapped_area(NULL, addr, len, pgoff, flags);
>   	}
>   
> -	/* Otherwise flags & MAP_PRIVATE: with no shmem object beneath it */
> +	/*
> +	 * Otherwise flags & MAP_PRIVATE: with no shmem object beneath it,
> +	 * attempt to map aligned to huge page size if possible, otherwise we
> +	 * fall back to system page size mappings.
> +	 */
> +	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> +		return thp_get_unmapped_area(file, addr, len, pgoff, flags);
> +
>   	return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
>   #else
>   	return -ENOSYS;
>   #endif
>   }

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb