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

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

[zhangqilong]

> 
> On Tue, Jul 29, 2025 at 09:49:41PM +0800, Zhang Qilong wrote:
> > By default, THP are usually enabled. Mapping /dev/zero with a size
> 
> Err... we can't rely on this.

OK, I will update this description in next version.  

> 
> As per below comments on code, I'd update this to say something about
> fallback if it's not.
> 
> > larger than 2MB could achieve performance gains by allocating aligned
> > address. 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>
> 
> This looks ok to me because there's a precedent for using
> thp_get_unmapped_area() directly as a file_operations-
> >get_unmapped_area e.g. in ext4.
> 
> We also simply (amusingly, or perhaps not hugely amusingly, rather
> 'uniquely') establish an anonymous mapping on f_op->mmap via
> mmap_zero() using vma_set_anonymous(), so we can rely on the standard
> anon page memory faulting logic to sort out the actual allocation/mapping of
> the huge page via:
> 
> __handle_mm_fault() -> create_huge_pmd() ->
> do_huge_pmd_anonymous_page() etc.
> 
> So everything should 'just work', and fallback if not permitted.
> 
> So in general seems fine.
> 
> > ---
> >  drivers/char/mem.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/char/mem.c b/drivers/char/mem.c index
> > 48839958b0b1..c57327ca9dd6 100644
> > --- a/drivers/char/mem.c
> > +++ b/drivers/char/mem.c
> > @@ -515,10 +515,12 @@ static int mmap_zero(struct file *file, struct
> > vm_area_struct *vma)  static unsigned long
> get_unmapped_area_zero(struct file *file,
> >  				unsigned long addr, unsigned long len,
> >  				unsigned long pgoff, unsigned long flags)
> {  #ifdef CONFIG_MMU
> > +	unsigned long ret;
> > +
> >  	if (flags & MAP_SHARED) {
> >  		/*
> >  		 * mmap_zero() will call shmem_zero_setup() to create a file,
> >  		 * so use shmem's get_unmapped_area in case it can be
> huge;
> >  		 * and pass NULL for file as in mmap.c's
> get_unmapped_area(), @@
> > -526,10 +528,13 @@ static unsigned long get_unmapped_area_zero(struct
> file *file,
> >  		 */
> >  		return shmem_get_unmapped_area(NULL, addr, len, pgoff,
> flags);
> >  	}
> >
> >  	/* Otherwise flags & MAP_PRIVATE: with no shmem object beneath
> it */
> 
> Let's add a comment here like:
> 
> 	/*
> 	 * Attempt to map aligned to huge page size if possible, otherwise
> we
>          * fall back to system page size mappings. If THP is not enabled, this
>          * returns NULL and we always fallback.
> 	 */
> 
> I think it'd be sensible to have an #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> here, because thp_get_unmapped_area() does the fallback for you, and
> then otherwise we'd be trying it twice which is weird.
> 
> E.g.:
> 
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> 	return thp_get_unmapped_area(file, addr, len, pgoff, flags); #else
> 	return mm_get_unmapped_area(current->mm, file, addr, len, pgoff,
> flags); #endif
> 

Trying it twice is realy unnecessary. This looks clearer and better, I will refer
to your suggestion in patch V2. Thanks a lot for your and helpful advice.

> > +	ret = thp_get_unmapped_area(file, addr, len, pgoff, flags);
> > +	if (ret)
> > +		return ret;
> >  	return mm_get_unmapped_area(current->mm, file, addr, len, pgoff,
> > flags);  #else
> >  	return -ENOSYS;
> >  #endif
> >  }
> > --
> > 2.43.0
> >
> 
> In _theory_ we should do the thing in mmap() where we check the size is
> PMD-aligned (see __get_unmapped_area()), but I don't think anybody's
> mapping a bunch of /dev/zero mappings next to each other or using them in
> any way where that'd matter... So yeah let's not :)

I agree with your thought. Do not make check here.

Thanks.
Zhang

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

[kernel test robot]

Hi Zhang,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus soc/for-next linus/master v6.16 next-20250729]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zhang-Qilong/dev-zero-try-to-align-PMD_SIZE-for-private-mapping/20250729-215253
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20250729134942.900517-1-zhangqilong3%40huawei.com
patch subject: [PATCH] /dev/zero: try to align PMD_SIZE for private mapping
config: alpha-allnoconfig (https://download.01.org/0day-ci/archive/20250730/202507301149.ILLCBOyz-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250730/202507301149.ILLCBOyz-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507301149.ILLCBOyz-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/uapi/linux/posix_types.h:5,
                    from include/uapi/linux/types.h:14,
                    from include/linux/types.h:6,
                    from include/linux/kasan-checks.h:5,
                    from include/asm-generic/rwonce.h:26,
                    from arch/alpha/include/asm/rwonce.h:33,
                    from include/linux/compiler.h:390,
                    from include/linux/export.h:5,
                    from include/linux/linkage.h:7,
                    from arch/alpha/include/asm/bug.h:5,
                    from include/linux/bug.h:5,
                    from include/linux/mmdebug.h:5,
                    from include/linux/mm.h:6,
                    from drivers/char/mem.c:12:
   drivers/char/mem.c: In function 'get_unmapped_area_zero':
>> include/linux/stddef.h:8:14: error: called object is not a function or function pointer
       8 | #define NULL ((void *)0)
         |              ^
   include/linux/huge_mm.h:536:33: note: in expansion of macro 'NULL'
     536 | #define thp_get_unmapped_area   NULL
         |                                 ^~~~
   drivers/char/mem.c:533:15: note: in expansion of macro 'thp_get_unmapped_area'
     533 |         ret = thp_get_unmapped_area(file, addr, len, pgoff, flags);
         |               ^~~~~~~~~~~~~~~~~~~~~


vim +8 include/linux/stddef.h

^1da177e4c3f41 Linus Torvalds   2005-04-16  6  
^1da177e4c3f41 Linus Torvalds   2005-04-16  7  #undef NULL
^1da177e4c3f41 Linus Torvalds   2005-04-16 @8  #define NULL ((void *)0)
6e218287432472 Richard Knutsson 2006-09-30  9  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Lorenzo Stoakes]

On Tue, Jul 29, 2025 at 09:49:41PM +0800, Zhang Qilong wrote:
> By default, THP are usually enabled. Mapping /dev/zero with a size

Err... we can't rely on this.

As per below comments on code, I'd update this to say something about fallback
if it's not.

> larger than 2MB could achieve performance gains by allocating aligned
> address. 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>

This looks ok to me because there's a precedent for using
thp_get_unmapped_area() directly as a file_operations->get_unmapped_area e.g. in
ext4.

We also simply (amusingly, or perhaps not hugely amusingly, rather 'uniquely')
establish an anonymous mapping on f_op->mmap via mmap_zero() using
vma_set_anonymous(), so we can rely on the standard anon page memory faulting
logic to sort out the actual allocation/mapping of the huge page via:

__handle_mm_fault() -> create_huge_pmd() -> do_huge_pmd_anonymous_page() etc.

So everything should 'just work', and fallback if not permitted.

So in general seems fine.

> ---
>  drivers/char/mem.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 48839958b0b1..c57327ca9dd6 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -515,10 +515,12 @@ static int mmap_zero(struct file *file, struct vm_area_struct *vma)
>  static unsigned long get_unmapped_area_zero(struct file *file,
>  				unsigned long addr, unsigned long len,
>  				unsigned long pgoff, unsigned long flags)
>  {
>  #ifdef CONFIG_MMU
> +	unsigned long ret;
> +
>  	if (flags & MAP_SHARED) {
>  		/*
>  		 * mmap_zero() will call shmem_zero_setup() to create a file,
>  		 * so use shmem's get_unmapped_area in case it can be huge;
>  		 * and pass NULL for file as in mmap.c's get_unmapped_area(),
> @@ -526,10 +528,13 @@ static unsigned long get_unmapped_area_zero(struct file *file,
>  		 */
>  		return shmem_get_unmapped_area(NULL, addr, len, pgoff, flags);
>  	}
>
>  	/* Otherwise flags & MAP_PRIVATE: with no shmem object beneath it */

Let's add a comment here like:

	/*
	 * Attempt to map aligned to huge page size if possible, otherwise we
         * fall back to system page size mappings. If THP is not enabled, this
         * returns NULL and we always fallback.
	 */

I think it'd be sensible to have an #ifdef CONFIG_TRANSPARENT_HUGEPAGE here,
because thp_get_unmapped_area() does the fallback for you, and then otherwise
we'd be trying it twice which is weird.

E.g.:

#ifdef CONFIG_TRANSPARENT_HUGEPAGE
	return thp_get_unmapped_area(file, addr, len, pgoff, flags);
#else
	return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
#endif

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

In _theory_ we should do the thing in mmap() where we check the size is
PMD-aligned (see __get_unmapped_area()), but I don't think anybody's mapping a
bunch of /dev/zero mappings next to each other or using them in any way where
that'd matter... So yeah let's not :)

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

[David Hildenbrand]

On 29.07.25 15:49, Zhang Qilong wrote:
> By default, THP are usually enabled. Mapping /dev/zero with a size
> larger than 2MB could achieve performance gains by allocating aligned
> address. 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>
> ---
>   drivers/char/mem.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 48839958b0b1..c57327ca9dd6 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -515,10 +515,12 @@ static int mmap_zero(struct file *file, struct vm_area_struct *vma)
>   static unsigned long get_unmapped_area_zero(struct file *file,
>   				unsigned long addr, unsigned long len,
>   				unsigned long pgoff, unsigned long flags)
>   {
>   #ifdef CONFIG_MMU
> +	unsigned long ret;

Can we call that "addr" like we do in __get_unmapped_area()?

Nothing else jumped at me ... so I assume this is fine?

CCing mmap-man Lorenzo.


-- 
Cheers,

David / dhildenb

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

[Zhang Qilong]

By default, THP are usually enabled. Mapping /dev/zero with a size
larger than 2MB could achieve performance gains by allocating aligned
address. 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>
---
 drivers/char/mem.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 48839958b0b1..c57327ca9dd6 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -515,10 +515,12 @@ static int mmap_zero(struct file *file, struct vm_area_struct *vma)
 static unsigned long get_unmapped_area_zero(struct file *file,
 				unsigned long addr, unsigned long len,
 				unsigned long pgoff, unsigned long flags)
 {
 #ifdef CONFIG_MMU
+	unsigned long ret;
+
 	if (flags & MAP_SHARED) {
 		/*
 		 * mmap_zero() will call shmem_zero_setup() to create a file,
 		 * so use shmem's get_unmapped_area in case it can be huge;
 		 * and pass NULL for file as in mmap.c's get_unmapped_area(),
@@ -526,10 +528,13 @@ static unsigned long get_unmapped_area_zero(struct file *file,
 		 */
 		return shmem_get_unmapped_area(NULL, addr, len, pgoff, flags);
 	}
 
 	/* Otherwise flags & MAP_PRIVATE: with no shmem object beneath it */
+	ret = thp_get_unmapped_area(file, addr, len, pgoff, flags);
+	if (ret)
+		return ret;
 	return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
 #else
 	return -ENOSYS;
 #endif
 }
-- 
2.43.0