diff mbox series

[3/9] mm: Call free_transhuge_folio() directly from destroy_large_folio()

Message ID 20230815032645.1393700-4-willy@infradead.org (mailing list archive)
State New
Headers show
Series Remove _folio_dtor and _folio_order | expand

Commit Message

Matthew Wilcox Aug. 15, 2023, 3:26 a.m. UTC 
Indirect calls are expensive, thanks to Spectre.  Convert this one to
a direct call, and pass a folio instead of the head page for type safety.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/huge_mm.h | 2 +-
 mm/huge_memory.c        | 5 ++---
 mm/page_alloc.c         | 8 +++++---
 3 files changed, 8 insertions(+), 7 deletions(-)

Comments

kernel test robot Aug. 15, 2023, 6:13 a.m. UTC | #1 
Hi Matthew,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master v6.5-rc6 next-20230809]
[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/Matthew-Wilcox-Oracle/io_uring-Stop-calling-free_compound_page/20230815-112847
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20230815032645.1393700-4-willy%40infradead.org
patch subject: [PATCH 3/9] mm: Call free_transhuge_folio() directly from destroy_large_folio()
config: m68k-randconfig-r006-20230815 (https://download.01.org/0day-ci/archive/20230815/202308151346.AP0ryW4Y-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230815/202308151346.AP0ryW4Y-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/202308151346.AP0ryW4Y-lkp@intel.com/

All errors (new ones prefixed by >>):

   mm/page_alloc.c: In function 'destroy_large_folio':
>> mm/page_alloc.c:615:17: error: implicit declaration of function 'free_transhuge_folio' [-Werror=implicit-function-declaration]
     615 |                 free_transhuge_folio(folio);
         |                 ^~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/free_transhuge_folio +615 mm/page_alloc.c

   604	
   605	void destroy_large_folio(struct folio *folio)
   606	{
   607		enum compound_dtor_id dtor = folio->_folio_dtor;
   608	
   609		if (folio_test_hugetlb(folio)) {
   610			free_huge_page(folio);
   611			return;
   612		}
   613	
   614		if (folio_test_transhuge(folio) && dtor == TRANSHUGE_PAGE_DTOR) {
 > 615			free_transhuge_folio(folio);
   616			return;
   617		}
   618	
   619		VM_BUG_ON_FOLIO(dtor >= NR_COMPOUND_DTORS, folio);
   620		compound_page_dtors[dtor](&folio->page);
   621	}
   622
David Hildenbrand Aug. 15, 2023, 7:40 a.m. UTC | #2 
On 15.08.23 05:26, Matthew Wilcox (Oracle) wrote:
> Indirect calls are expensive, thanks to Spectre.  Convert this one to
> a direct call, and pass a folio instead of the head page for type safety.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   include/linux/huge_mm.h | 2 +-
>   mm/huge_memory.c        | 5 ++---
>   mm/page_alloc.c         | 8 +++++---
>   3 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 20284387b841..24aee49a581a 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -144,7 +144,7 @@ unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr,
>   		unsigned long len, unsigned long pgoff, unsigned long flags);
>   
>   void prep_transhuge_page(struct page *page);
> -void free_transhuge_page(struct page *page);
> +void free_transhuge_folio(struct folio *folio);
>   
>   bool can_split_folio(struct folio *folio, int *pextra_pins);
>   int split_huge_page_to_list(struct page *page, struct list_head *list);
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 8480728fa220..516fe3c26ef3 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2779,9 +2779,8 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>   	return ret;
>   }
>   
> -void free_transhuge_page(struct page *page)
> +void free_transhuge_folio(struct folio *folio)
>   {
> -	struct folio *folio = (struct folio *)page;
>   	struct deferred_split *ds_queue = get_deferred_split_queue(folio);
>   	unsigned long flags;
>   
> @@ -2798,7 +2797,7 @@ void free_transhuge_page(struct page *page)
>   		}
>   		spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
>   	}
> -	free_compound_page(page);
> +	free_compound_page(&folio->page);
>   }
>   
>   void deferred_split_folio(struct folio *folio)
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 1f67d4968590..feb2e95cf021 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -287,9 +287,6 @@ const char * const migratetype_names[MIGRATE_TYPES] = {
>   static compound_page_dtor * const compound_page_dtors[NR_COMPOUND_DTORS] = {
>   	[NULL_COMPOUND_DTOR] = NULL,
>   	[COMPOUND_PAGE_DTOR] = free_compound_page,
> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -	[TRANSHUGE_PAGE_DTOR] = free_transhuge_page,
> -#endif
>   };
>   
>   int min_free_kbytes = 1024;
> @@ -624,6 +621,11 @@ void destroy_large_folio(struct folio *folio)
>   		return;
>   	}
>   
> +	if (folio_test_transhuge(folio) && dtor == TRANSHUGE_PAGE_DTOR) {
> +		free_transhuge_folio(folio);

I really wonder if folio_test_transhuge() should be written similar to 
folio_test_hugetlb() instead, such that the dtor check is implicit.

Any good reasons not to do that?
kernel test robot Aug. 15, 2023, 8:09 a.m. UTC | #3 
Hi Matthew,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master v6.5-rc6 next-20230809]
[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/Matthew-Wilcox-Oracle/io_uring-Stop-calling-free_compound_page/20230815-112847
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20230815032645.1393700-4-willy%40infradead.org
patch subject: [PATCH 3/9] mm: Call free_transhuge_folio() directly from destroy_large_folio()
config: arm-randconfig-r046-20230815 (https://download.01.org/0day-ci/archive/20230815/202308151509.ErdX8wsj-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230815/202308151509.ErdX8wsj-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/202308151509.ErdX8wsj-lkp@intel.com/

All errors (new ones prefixed by >>):

>> mm/page_alloc.c:615:3: error: call to undeclared function 'free_transhuge_folio'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     615 |                 free_transhuge_folio(folio);
         |                 ^
   1 error generated.


vim +/free_transhuge_folio +615 mm/page_alloc.c

   604	
   605	void destroy_large_folio(struct folio *folio)
   606	{
   607		enum compound_dtor_id dtor = folio->_folio_dtor;
   608	
   609		if (folio_test_hugetlb(folio)) {
   610			free_huge_page(folio);
   611			return;
   612		}
   613	
   614		if (folio_test_transhuge(folio) && dtor == TRANSHUGE_PAGE_DTOR) {
 > 615			free_transhuge_folio(folio);
   616			return;
   617		}
   618	
   619		VM_BUG_ON_FOLIO(dtor >= NR_COMPOUND_DTORS, folio);
   620		compound_page_dtors[dtor](&folio->page);
   621	}
   622
Matthew Wilcox Aug. 15, 2023, 2:06 p.m. UTC | #4 
On Tue, Aug 15, 2023 at 09:40:58AM +0200, David Hildenbrand wrote:
> > @@ -624,6 +621,11 @@ void destroy_large_folio(struct folio *folio)
> >   		return;
> >   	}
> > +	if (folio_test_transhuge(folio) && dtor == TRANSHUGE_PAGE_DTOR) {
> > +		free_transhuge_folio(folio);
> 
> I really wonder if folio_test_transhuge() should be written similar to
> folio_test_hugetlb() instead, such that the dtor check is implicit.
> 
> Any good reasons not to do that?

Actually, we should probably delete folio_test_transhuge().  I'll tack
a patch onto the end of this series to do that.  I want to avoid a
reference to free_transhuge_folio() if !CONFIG_TRANSPARENT_HUGEPAGE and
folio_test_transhuge() accomplishes that by way of TESTPAGEFLAG_FALSE
in page-flags.  But so does folio_test_deferred_list() which is what
we're getting to by the end of this series.

So I'll leave this patch alone for now (other than fixing up the
buildbot errors).
diff mbox series

Patch

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 20284387b841..24aee49a581a 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -144,7 +144,7 @@  unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr,
 		unsigned long len, unsigned long pgoff, unsigned long flags);
 
 void prep_transhuge_page(struct page *page);
-void free_transhuge_page(struct page *page);
+void free_transhuge_folio(struct folio *folio);
 
 bool can_split_folio(struct folio *folio, int *pextra_pins);
 int split_huge_page_to_list(struct page *page, struct list_head *list);
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 8480728fa220..516fe3c26ef3 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2779,9 +2779,8 @@  int split_huge_page_to_list(struct page *page, struct list_head *list)
 	return ret;
 }
 
-void free_transhuge_page(struct page *page)
+void free_transhuge_folio(struct folio *folio)
 {
-	struct folio *folio = (struct folio *)page;
 	struct deferred_split *ds_queue = get_deferred_split_queue(folio);
 	unsigned long flags;
 
@@ -2798,7 +2797,7 @@  void free_transhuge_page(struct page *page)
 		}
 		spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
 	}
-	free_compound_page(page);
+	free_compound_page(&folio->page);
 }
 
 void deferred_split_folio(struct folio *folio)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1f67d4968590..feb2e95cf021 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -287,9 +287,6 @@  const char * const migratetype_names[MIGRATE_TYPES] = {
 static compound_page_dtor * const compound_page_dtors[NR_COMPOUND_DTORS] = {
 	[NULL_COMPOUND_DTOR] = NULL,
 	[COMPOUND_PAGE_DTOR] = free_compound_page,
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-	[TRANSHUGE_PAGE_DTOR] = free_transhuge_page,
-#endif
 };
 
 int min_free_kbytes = 1024;
@@ -624,6 +621,11 @@  void destroy_large_folio(struct folio *folio)
 		return;
 	}
 
+	if (folio_test_transhuge(folio) && dtor == TRANSHUGE_PAGE_DTOR) {
+		free_transhuge_folio(folio);
+		return;
+	}
+
 	VM_BUG_ON_FOLIO(dtor >= NR_COMPOUND_DTORS, folio);
 	compound_page_dtors[dtor](&folio->page);
 }