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
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
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
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
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
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
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
© 2016 - 2025 Red Hat, Inc.