[PATCH v2 1/5] mm: introduce num_pages_contiguous()

lizhe.67@bytedance.com posted 5 patches 3 months ago
There is a newer version of this series
[PATCH v2 1/5] mm: introduce num_pages_contiguous()
Posted by lizhe.67@bytedance.com 3 months ago
From: Li Zhe <lizhe.67@bytedance.com>

Function num_pages_contiguous() determine the number of contiguous
pages starting from the first page in the given array of page pointers.
VFIO will utilize this interface to accelerate the VFIO DMA map process.

Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
---
 include/linux/mm.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0ef2ba0c667a..1d26203d1ced 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -205,6 +205,26 @@ extern unsigned long sysctl_admin_reserve_kbytes;
 #define folio_page_idx(folio, p)	((p) - &(folio)->page)
 #endif
 
+/*
+ * num_pages_contiguous() - determine the number of contiguous pages
+ * starting from the first page.
+ *
+ * @pages: an array of page pointers
+ * @nr_pages: length of the array
+ */
+static inline unsigned long num_pages_contiguous(struct page **pages,
+						 unsigned long nr_pages)
+{
+	struct page *first_page = pages[0];
+	unsigned long i;
+
+	for (i = 1; i < nr_pages; i++)
+		if (pages[i] != nth_page(first_page, i))
+			break;
+
+	return i;
+}
+
 /* to align the pointer to the (next) page boundary */
 #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE)
 
-- 
2.20.1
Re: [PATCH v2 1/5] mm: introduce num_pages_contiguous()
Posted by kernel test robot 3 months ago
Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on awilliam-vfio/next]
[also build test ERROR on awilliam-vfio/for-linus akpm-mm/mm-everything linus/master v6.16-rc4 next-20250704]
[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/lizhe-67-bytedance-com/mm-introduce-num_pages_contiguous/20250704-142948
base:   https://github.com/awilliam/linux-vfio.git next
patch link:    https://lore.kernel.org/r/20250704062602.33500-2-lizhe.67%40bytedance.com
patch subject: [PATCH v2 1/5] mm: introduce num_pages_contiguous()
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20250705/202507050529.EoMuEtd8-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250705/202507050529.EoMuEtd8-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/202507050529.EoMuEtd8-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from arch/sh/include/asm/page.h:160,
                    from arch/sh/include/asm/thread_info.h:13,
                    from include/linux/thread_info.h:60,
                    from include/asm-generic/preempt.h:5,
                    from ./arch/sh/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:79,
                    from include/linux/spinlock.h:56,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/mm.h:7,
                    from arch/sh/kernel/asm-offsets.c:14:
   include/linux/mm.h: In function 'num_pages_contiguous':
>> include/asm-generic/memory_model.h:48:21: error: implicit declaration of function 'page_to_section'; did you mean 'present_section'? [-Wimplicit-function-declaration]
      48 |         int __sec = page_to_section(__pg);                      \
         |                     ^~~~~~~~~~~~~~~
   include/asm-generic/memory_model.h:53:32: note: in definition of macro '__pfn_to_page'
      53 | ({      unsigned long __pfn = (pfn);                    \
         |                                ^~~
   include/asm-generic/memory_model.h:65:21: note: in expansion of macro '__page_to_pfn'
      65 | #define page_to_pfn __page_to_pfn
         |                     ^~~~~~~~~~~~~
   include/linux/mm.h:200:38: note: in expansion of macro 'page_to_pfn'
     200 | #define nth_page(page,n) pfn_to_page(page_to_pfn((page)) + (n))
         |                                      ^~~~~~~~~~~
   include/linux/mm.h:221:33: note: in expansion of macro 'nth_page'
     221 |                 if (pages[i] != nth_page(first_page, i))
         |                                 ^~~~~~~~
   include/linux/mm.h: At top level:
>> include/linux/mm.h:2002:29: error: conflicting types for 'page_to_section'; have 'long unsigned int(const struct page *)'
    2002 | static inline unsigned long page_to_section(const struct page *page)
         |                             ^~~~~~~~~~~~~~~
   include/asm-generic/memory_model.h:48:21: note: previous implicit declaration of 'page_to_section' with type 'int()'
      48 |         int __sec = page_to_section(__pg);                      \
         |                     ^~~~~~~~~~~~~~~
   include/asm-generic/memory_model.h:53:32: note: in definition of macro '__pfn_to_page'
      53 | ({      unsigned long __pfn = (pfn);                    \
         |                                ^~~
   include/asm-generic/memory_model.h:65:21: note: in expansion of macro '__page_to_pfn'
      65 | #define page_to_pfn __page_to_pfn
         |                     ^~~~~~~~~~~~~
   include/linux/mm.h:200:38: note: in expansion of macro 'page_to_pfn'
     200 | #define nth_page(page,n) pfn_to_page(page_to_pfn((page)) + (n))
         |                                      ^~~~~~~~~~~
   include/linux/mm.h:221:33: note: in expansion of macro 'nth_page'
     221 |                 if (pages[i] != nth_page(first_page, i))
         |                                 ^~~~~~~~
   make[3]: *** [scripts/Makefile.build:98: arch/sh/kernel/asm-offsets.s] Error 1
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1274: prepare0] Error 2
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +2002 include/linux/mm.h

bf4e8902ee5080 Daniel Kiper      2011-05-24  2001  
aa462abe8aaf21 Ian Campbell      2011-08-17 @2002  static inline unsigned long page_to_section(const struct page *page)
d41dee369bff3b Andy Whitcroft    2005-06-23  2003  {
d41dee369bff3b Andy Whitcroft    2005-06-23  2004  	return (page->flags >> SECTIONS_PGSHIFT) & SECTIONS_MASK;
d41dee369bff3b Andy Whitcroft    2005-06-23  2005  }
308c05e35e3517 Christoph Lameter 2008-04-28  2006  #endif
d41dee369bff3b Andy Whitcroft    2005-06-23  2007  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v2 1/5] mm: introduce num_pages_contiguous()
Posted by lizhe.67@bytedance.com 3 months ago
On Sat, 5 Jul 2025 05:19:34 +0800, lkp@intel.com wrote:
 
> All errors (new ones prefixed by >>):
> 
>    In file included from arch/sh/include/asm/page.h:160,
>                     from arch/sh/include/asm/thread_info.h:13,
>                     from include/linux/thread_info.h:60,
>                     from include/asm-generic/preempt.h:5,
>                     from ./arch/sh/include/generated/asm/preempt.h:1,
>                     from include/linux/preempt.h:79,
>                     from include/linux/spinlock.h:56,
>                     from include/linux/mmzone.h:8,
>                     from include/linux/gfp.h:7,
>                     from include/linux/mm.h:7,
>                     from arch/sh/kernel/asm-offsets.c:14:
>    include/linux/mm.h: In function 'num_pages_contiguous':
> >> include/asm-generic/memory_model.h:48:21: error: implicit declaration of function 'page_to_section'; did you mean 'present_section'? [-Wimplicit-function-declaration]
>       48 |         int __sec = page_to_section(__pg);                      \
>          |                     ^~~~~~~~~~~~~~~
>    include/asm-generic/memory_model.h:53:32: note: in definition of macro '__pfn_to_page'
>       53 | ({      unsigned long __pfn = (pfn);                    \
>          |                                ^~~
>    include/asm-generic/memory_model.h:65:21: note: in expansion of macro '__page_to_pfn'
>       65 | #define page_to_pfn __page_to_pfn
>          |                     ^~~~~~~~~~~~~
>    include/linux/mm.h:200:38: note: in expansion of macro 'page_to_pfn'
>      200 | #define nth_page(page,n) pfn_to_page(page_to_pfn((page)) + (n))
>          |                                      ^~~~~~~~~~~
>    include/linux/mm.h:221:33: note: in expansion of macro 'nth_page'
>      221 |                 if (pages[i] != nth_page(first_page, i))
>          |                                 ^~~~~~~~
>    include/linux/mm.h: At top level:
> >> include/linux/mm.h:2002:29: error: conflicting types for 'page_to_section'; have 'long unsigned int(const struct page *)'
>     2002 | static inline unsigned long page_to_section(const struct page *page)
>          |                             ^~~~~~~~~~~~~~~
>    include/asm-generic/memory_model.h:48:21: note: previous implicit declaration of 'page_to_section' with type 'int()'
>       48 |         int __sec = page_to_section(__pg);                      \
>          |                     ^~~~~~~~~~~~~~~
>    include/asm-generic/memory_model.h:53:32: note: in definition of macro '__pfn_to_page'
>       53 | ({      unsigned long __pfn = (pfn);                    \
>          |                                ^~~
>    include/asm-generic/memory_model.h:65:21: note: in expansion of macro '__page_to_pfn'
>       65 | #define page_to_pfn __page_to_pfn
>          |                     ^~~~~~~~~~~~~
>    include/linux/mm.h:200:38: note: in expansion of macro 'page_to_pfn'
>      200 | #define nth_page(page,n) pfn_to_page(page_to_pfn((page)) + (n))
>          |                                      ^~~~~~~~~~~
>    include/linux/mm.h:221:33: note: in expansion of macro 'nth_page'
>      221 |                 if (pages[i] != nth_page(first_page, i))
>          |                                 ^~~~~~~~
>    make[3]: *** [scripts/Makefile.build:98: arch/sh/kernel/asm-offsets.s] Error 1
>    make[3]: Target 'prepare' not remade because of errors.
>    make[2]: *** [Makefile:1274: prepare0] Error 2
>    make[2]: Target 'prepare' not remade because of errors.
>    make[1]: *** [Makefile:248: __sub-make] Error 2
>    make[1]: Target 'prepare' not remade because of errors.
>    make: *** [Makefile:248: __sub-make] Error 2
>    make: Target 'prepare' not remade because of errors.
> 
> 
> vim +2002 include/linux/mm.h
> 
> bf4e8902ee5080 Daniel Kiper      2011-05-24  2001  
> aa462abe8aaf21 Ian Campbell      2011-08-17 @2002  static inline unsigned long page_to_section(const struct page *page)
> d41dee369bff3b Andy Whitcroft    2005-06-23  2003  {
> d41dee369bff3b Andy Whitcroft    2005-06-23  2004  	return (page->flags >> SECTIONS_PGSHIFT) & SECTIONS_MASK;
> d41dee369bff3b Andy Whitcroft    2005-06-23  2005  }
> 308c05e35e3517 Christoph Lameter 2008-04-28  2006  #endif
> d41dee369bff3b Andy Whitcroft    2005-06-23  2007  

Thank you. Let's move function num_pages_contiguous() below the
definition of page_to_section() to resolve this compilation error.

Thanks,
Zhe
Re: [PATCH v2 1/5] mm: introduce num_pages_contiguous()
Posted by Jason Gunthorpe 3 months ago
On Fri, Jul 04, 2025 at 02:25:58PM +0800, lizhe.67@bytedance.com wrote:
> From: Li Zhe <lizhe.67@bytedance.com>
> 
> Function num_pages_contiguous() determine the number of contiguous
> pages starting from the first page in the given array of page pointers.
> VFIO will utilize this interface to accelerate the VFIO DMA map process.
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
> ---
>  include/linux/mm.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0ef2ba0c667a..1d26203d1ced 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -205,6 +205,26 @@ extern unsigned long sysctl_admin_reserve_kbytes;
>  #define folio_page_idx(folio, p)	((p) - &(folio)->page)
>  #endif
>  
> +/*
> + * num_pages_contiguous() - determine the number of contiguous pages
> + * starting from the first page.
> + *
> + * @pages: an array of page pointers
> + * @nr_pages: length of the array
> + */
> +static inline unsigned long num_pages_contiguous(struct page **pages,
> +						 unsigned long nr_pages)

Both longs should be size_t I think

> +{
> +	struct page *first_page = pages[0];
> +	unsigned long i;

Size_t

> +
> +	for (i = 1; i < nr_pages; i++)
> +		if (pages[i] != nth_page(first_page, i))
> +			break;

It seems OK. So the reasoning here is this is faster on
CONFIG_SPARSEMEM_VMEMMAP/nonsparse and about the same on sparse mem?
(or we don't care?)

Jason
Re: [PATCH v2 1/5] mm: introduce num_pages_contiguous()
Posted by lizhe.67@bytedance.com 3 months ago
On Fri, 4 Jul 2025 14:10:15 -0300, jgg@ziepe.ca wrote:

> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 0ef2ba0c667a..1d26203d1ced 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -205,6 +205,26 @@ extern unsigned long sysctl_admin_reserve_kbytes;
> >  #define folio_page_idx(folio, p)	((p) - &(folio)->page)
> >  #endif
> >  
> > +/*
> > + * num_pages_contiguous() - determine the number of contiguous pages
> > + * starting from the first page.
> > + *
> > + * @pages: an array of page pointers
> > + * @nr_pages: length of the array
> > + */
> > +static inline unsigned long num_pages_contiguous(struct page **pages,
> > +						 unsigned long nr_pages)
> 
> Both longs should be size_t I think

Yes, size_t is a better choice.

> > +{
> > +	struct page *first_page = pages[0];
> > +	unsigned long i;
> 
> Size_t
> 
> > +
> > +	for (i = 1; i < nr_pages; i++)
> > +		if (pages[i] != nth_page(first_page, i))
> > +			break;
> 
> It seems OK. So the reasoning here is this is faster on
> CONFIG_SPARSEMEM_VMEMMAP/nonsparse

Yes.

> and about the same on sparse mem?
> (or we don't care?)

Regarding sparse memory, I'm not entirely certain. From my
understanding, VFIO is predominantly utilized in virtualization
scenarios, which typically have sufficient kernel resources. This
implies that CONFIG_SPARSEMEM_VMEMMAP is generally set to "y" in
such cases. Therefore, we need not overly concern ourselves with
this particular scenario. Of course, David has also proposed
optimization solutions for sparse memory scenarios[1]. If anyone
later complains about performance in this context, I would be happy
to assist with further optimization efforts. Currently, I only have
a x86_64 machine, on which CONFIG_SPARSEMEM_VMEMMAP is forcibly
enabled. Attempting to compile with CONFIG_SPARSEMEM &&
!CONFIG_SPARSEMEM_VMEMMAP results in compilation errors, preventing
me from conducting the relevant performance tests.

Thanks,
Zhe

[1]: https://lore.kernel.org/all/c1144447-6b67-48d3-b37c-5f1ca6a9b4a7@redhat.com/#t
Re: [PATCH v2 1/5] mm: introduce num_pages_contiguous()
Posted by David Hildenbrand 3 months ago
On 04.07.25 08:25, lizhe.67@bytedance.com wrote:
> From: Li Zhe <lizhe.67@bytedance.com>
> 
> Function num_pages_contiguous() determine the number of contiguous
> pages starting from the first page in the given array of page pointers.
> VFIO will utilize this interface to accelerate the VFIO DMA map process.
> 
> Suggested-by: David Hildenbrand <david@redhat.com>

I think Jason suggested having this as a helper as well.

> Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
> ---
>   include/linux/mm.h | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0ef2ba0c667a..1d26203d1ced 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -205,6 +205,26 @@ extern unsigned long sysctl_admin_reserve_kbytes;
>   #define folio_page_idx(folio, p)	((p) - &(folio)->page)
>   #endif
>   
> +/*
> + * num_pages_contiguous() - determine the number of contiguous pages
> + * starting from the first page.


Maybe clarify here here:

"Pages are contiguous if they represent contiguous PFNs. Depending on 
the memory model, this can mean that the addresses of the "struct page"s 
are not contiguous."


> + *
> + * @pages: an array of page pointers
> + * @nr_pages: length of the array
> + */
> +static inline unsigned long num_pages_contiguous(struct page **pages,
> +						 unsigned long nr_pages)
> +{
> +	struct page *first_page = pages[0];
> +	unsigned long i;
> +
> +	for (i = 1; i < nr_pages; i++)
> +		if (pages[i] != nth_page(first_page, i))

if (pages[i] != nth_page(pages[0], i))

Should be clear as well, so no need for the temporary "first_page" variable.

Apart from that LGTM.

-- 
Cheers,

David / dhildenb
Re: [PATCH v2 1/5] mm: introduce num_pages_contiguous()
Posted by lizhe.67@bytedance.com 3 months ago
On Fri, 4 Jul 2025 09:56:23 +0200, david@redhat.com wrote:

> On 04.07.25 08:25, lizhe.67@bytedance.com wrote:
> > From: Li Zhe <lizhe.67@bytedance.com>
> > 
> > Function num_pages_contiguous() determine the number of contiguous
> > pages starting from the first page in the given array of page pointers.
> > VFIO will utilize this interface to accelerate the VFIO DMA map process.
> > 
> > Suggested-by: David Hildenbrand <david@redhat.com>
> 
> I think Jason suggested having this as a helper as well.

Yes, thank you for the reminder. Jason needs to be added here.

Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>

> > Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
> > ---
> >   include/linux/mm.h | 20 ++++++++++++++++++++
> >   1 file changed, 20 insertions(+)
> > 
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 0ef2ba0c667a..1d26203d1ced 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -205,6 +205,26 @@ extern unsigned long sysctl_admin_reserve_kbytes;
> >   #define folio_page_idx(folio, p)	((p) - &(folio)->page)
> >   #endif
> >   
> > +/*
> > + * num_pages_contiguous() - determine the number of contiguous pages
> > + * starting from the first page.
> 
> Maybe clarify here here:
> 
> "Pages are contiguous if they represent contiguous PFNs. Depending on 
> the memory model, this can mean that the addresses of the "struct page"s 
> are not contiguous."

Thank you. I will include this clarification in the comment.

> > + *
> > + * @pages: an array of page pointers
> > + * @nr_pages: length of the array
> > + */
> > +static inline unsigned long num_pages_contiguous(struct page **pages,
> > +						 unsigned long nr_pages)
> > +{
> > +	struct page *first_page = pages[0];
> > +	unsigned long i;
> > +
> > +	for (i = 1; i < nr_pages; i++)
> > +		if (pages[i] != nth_page(first_page, i))
> 
> if (pages[i] != nth_page(pages[0], i))
> 
> Should be clear as well, so no need for the temporary "first_page" variable.

Thank you. Doing so makes the function appear much clearer.

> Apart from that LGTM.

Thank you for your review!

Thanks,
Zhe