diff mbox series

[RFC,v3,1/3] mm: Factor out the pagetable pages account into new helper function

Message ID 9c527d4d2eb1f457306e575ce16c6acdd8141e02.1656586863.git.baolin.wang@linux.alibaba.com (mailing list archive)
State New
Headers show
Series Add PUD and kernel PTE level pagetable account | expand

Commit Message

Baolin Wang June 30, 2022, 11:11 a.m. UTC
Factor out the pagetable pages account into new helper functions to avoid
duplicated code. Meanwhile these helper functions also will be used to
account pagetable pages which do not need split pagetale lock.

Meanwhile convert to use mod_lruvec_page_state() in case of non-order-0
page table allocation.

Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 include/linux/mm.h | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Mike Rapoport June 30, 2022, 2:11 p.m. UTC | #1
On Thu, Jun 30, 2022 at 07:11:14PM +0800, Baolin Wang wrote:
> Factor out the pagetable pages account into new helper functions to avoid
> duplicated code. Meanwhile these helper functions also will be used to
> account pagetable pages which do not need split pagetale lock.
> 
> Meanwhile convert to use mod_lruvec_page_state() in case of non-order-0
> page table allocation.

These are *very* rare. I think only parisc may have non-order-0 pmd and pud
tables.
With that, I'd suggest making use of compound_nr() build time opt-in.
 
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
>  include/linux/mm.h | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a2270e3..3be6d2c 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2353,20 +2353,30 @@ static inline void pgtable_init(void)
>  	pgtable_cache_init();
>  }
>  
> +static inline void pgtable_page_inc(struct page *page)
> +{
> +	__SetPageTable(page);
> +	mod_lruvec_page_state(page, NR_PAGETABLE, compound_nr(page));
> +}
> +
> +static inline void pgtable_page_dec(struct page *page)
> +{
> +	__ClearPageTable(page);
> +	mod_lruvec_page_state(page, NR_PAGETABLE, -compound_nr(page));
> +}
> +
>  static inline bool pgtable_pte_page_ctor(struct page *page)
>  {
>  	if (!ptlock_init(page))
>  		return false;
> -	__SetPageTable(page);
> -	inc_lruvec_page_state(page, NR_PAGETABLE);
> +	pgtable_page_inc(page);
>  	return true;
>  }
>  
>  static inline void pgtable_pte_page_dtor(struct page *page)
>  {
>  	ptlock_free(page);
> -	__ClearPageTable(page);
> -	dec_lruvec_page_state(page, NR_PAGETABLE);
> +	pgtable_page_dec(page);
>  }
>  
>  #define pte_offset_map_lock(mm, pmd, address, ptlp)	\
> @@ -2452,16 +2462,14 @@ static inline bool pgtable_pmd_page_ctor(struct page *page)
>  {
>  	if (!pmd_ptlock_init(page))
>  		return false;
> -	__SetPageTable(page);
> -	inc_lruvec_page_state(page, NR_PAGETABLE);
> +	pgtable_page_inc(page);
>  	return true;
>  }
>  
>  static inline void pgtable_pmd_page_dtor(struct page *page)
>  {
>  	pmd_ptlock_free(page);
> -	__ClearPageTable(page);
> -	dec_lruvec_page_state(page, NR_PAGETABLE);
> +	pgtable_page_dec(page);
>  }
>  
>  /*
> -- 
> 1.8.3.1
>
Baolin Wang July 1, 2022, 8 a.m. UTC | #2
On 6/30/2022 10:11 PM, Mike Rapoport wrote:
> On Thu, Jun 30, 2022 at 07:11:14PM +0800, Baolin Wang wrote:
>> Factor out the pagetable pages account into new helper functions to avoid
>> duplicated code. Meanwhile these helper functions also will be used to
>> account pagetable pages which do not need split pagetale lock.
>>
>> Meanwhile convert to use mod_lruvec_page_state() in case of non-order-0
>> page table allocation.
> 
> These are *very* rare. I think only parisc may have non-order-0 pmd and pud
> tables.

s390 also has non-order-0 page table allocation, but they both do not 
use the generic page table allocation now.

> With that, I'd suggest making use of compound_nr() build time opt-in.

After more thinking, I'd prefer to change back to use 
inc_lruvec_page_state()/dec_lruvec_page_state(), since now no 
architecures will need non-order-0 page table allocation.

After this patchset, I plan to convert parisc and s390 to use generic 
pagetable allocation, then I will add non-order-0 page table allocation 
support. Like Matthew suggested, maybe I need change the API to pass the 
number of pages.

>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>> ---
>>   include/linux/mm.h | 24 ++++++++++++++++--------
>>   1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index a2270e3..3be6d2c 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -2353,20 +2353,30 @@ static inline void pgtable_init(void)
>>   	pgtable_cache_init();
>>   }
>>   
>> +static inline void pgtable_page_inc(struct page *page)
>> +{
>> +	__SetPageTable(page);
>> +	mod_lruvec_page_state(page, NR_PAGETABLE, compound_nr(page));
>> +}
>> +
>> +static inline void pgtable_page_dec(struct page *page)
>> +{
>> +	__ClearPageTable(page);
>> +	mod_lruvec_page_state(page, NR_PAGETABLE, -compound_nr(page));
>> +}
>> +
>>   static inline bool pgtable_pte_page_ctor(struct page *page)
>>   {
>>   	if (!ptlock_init(page))
>>   		return false;
>> -	__SetPageTable(page);
>> -	inc_lruvec_page_state(page, NR_PAGETABLE);
>> +	pgtable_page_inc(page);
>>   	return true;
>>   }
>>   
>>   static inline void pgtable_pte_page_dtor(struct page *page)
>>   {
>>   	ptlock_free(page);
>> -	__ClearPageTable(page);
>> -	dec_lruvec_page_state(page, NR_PAGETABLE);
>> +	pgtable_page_dec(page);
>>   }
>>   
>>   #define pte_offset_map_lock(mm, pmd, address, ptlp)	\
>> @@ -2452,16 +2462,14 @@ static inline bool pgtable_pmd_page_ctor(struct page *page)
>>   {
>>   	if (!pmd_ptlock_init(page))
>>   		return false;
>> -	__SetPageTable(page);
>> -	inc_lruvec_page_state(page, NR_PAGETABLE);
>> +	pgtable_page_inc(page);
>>   	return true;
>>   }
>>   
>>   static inline void pgtable_pmd_page_dtor(struct page *page)
>>   {
>>   	pmd_ptlock_free(page);
>> -	__ClearPageTable(page);
>> -	dec_lruvec_page_state(page, NR_PAGETABLE);
>> +	pgtable_page_dec(page);
>>   }
>>   
>>   /*
>> -- 
>> 1.8.3.1
>>
>
Mike Rapoport July 2, 2022, 10:15 a.m. UTC | #3
On Fri, Jul 01, 2022 at 04:00:59PM +0800, Baolin Wang wrote:
> 
> 
> On 6/30/2022 10:11 PM, Mike Rapoport wrote:
> > On Thu, Jun 30, 2022 at 07:11:14PM +0800, Baolin Wang wrote:
> > > Factor out the pagetable pages account into new helper functions to avoid
> > > duplicated code. Meanwhile these helper functions also will be used to
> > > account pagetable pages which do not need split pagetale lock.
> > > 
> > > Meanwhile convert to use mod_lruvec_page_state() in case of non-order-0
> > > page table allocation.
> > 
> > These are *very* rare. I think only parisc may have non-order-0 pmd and pud
> > tables.
> 
> s390 also has non-order-0 page table allocation, but they both do not use
> the generic page table allocation now.
> 
> > With that, I'd suggest making use of compound_nr() build time opt-in.
> 
> After more thinking, I'd prefer to change back to use
> inc_lruvec_page_state()/dec_lruvec_page_state(), since now no architecures
> will need non-order-0 page table allocation.
> 
> After this patchset, I plan to convert parisc and s390 to use generic
> pagetable allocation, then I will add non-order-0 page table allocation
> support. Like Matthew suggested, maybe I need change the API to pass the
> number of pages.

I think it would be simpler to add proper accounting to s390 and parisc
versions than make them use the generic allocation functions. Moreover, API
change to support these cases feels like unnecessary churn to me.
 
> > > Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > > ---
Baolin Wang July 3, 2022, 1:48 p.m. UTC | #4
On 7/2/2022 6:15 PM, Mike Rapoport wrote:
> On Fri, Jul 01, 2022 at 04:00:59PM +0800, Baolin Wang wrote:
>>
>>
>> On 6/30/2022 10:11 PM, Mike Rapoport wrote:
>>> On Thu, Jun 30, 2022 at 07:11:14PM +0800, Baolin Wang wrote:
>>>> Factor out the pagetable pages account into new helper functions to avoid
>>>> duplicated code. Meanwhile these helper functions also will be used to
>>>> account pagetable pages which do not need split pagetale lock.
>>>>
>>>> Meanwhile convert to use mod_lruvec_page_state() in case of non-order-0
>>>> page table allocation.
>>>
>>> These are *very* rare. I think only parisc may have non-order-0 pmd and pud
>>> tables.
>>
>> s390 also has non-order-0 page table allocation, but they both do not use
>> the generic page table allocation now.
>>
>>> With that, I'd suggest making use of compound_nr() build time opt-in.
>>
>> After more thinking, I'd prefer to change back to use
>> inc_lruvec_page_state()/dec_lruvec_page_state(), since now no architecures
>> will need non-order-0 page table allocation.
>>
>> After this patchset, I plan to convert parisc and s390 to use generic
>> pagetable allocation, then I will add non-order-0 page table allocation
>> support. Like Matthew suggested, maybe I need change the API to pass the
>> number of pages.
> 
> I think it would be simpler to add proper accounting to s390 and parisc
> versions than make them use the generic allocation functions. Moreover, API
> change to support these cases feels like unnecessary churn to me.

Got it. Thanks.
diff mbox series

Patch

diff --git a/include/linux/mm.h b/include/linux/mm.h
index a2270e3..3be6d2c 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2353,20 +2353,30 @@  static inline void pgtable_init(void)
 	pgtable_cache_init();
 }
 
+static inline void pgtable_page_inc(struct page *page)
+{
+	__SetPageTable(page);
+	mod_lruvec_page_state(page, NR_PAGETABLE, compound_nr(page));
+}
+
+static inline void pgtable_page_dec(struct page *page)
+{
+	__ClearPageTable(page);
+	mod_lruvec_page_state(page, NR_PAGETABLE, -compound_nr(page));
+}
+
 static inline bool pgtable_pte_page_ctor(struct page *page)
 {
 	if (!ptlock_init(page))
 		return false;
-	__SetPageTable(page);
-	inc_lruvec_page_state(page, NR_PAGETABLE);
+	pgtable_page_inc(page);
 	return true;
 }
 
 static inline void pgtable_pte_page_dtor(struct page *page)
 {
 	ptlock_free(page);
-	__ClearPageTable(page);
-	dec_lruvec_page_state(page, NR_PAGETABLE);
+	pgtable_page_dec(page);
 }
 
 #define pte_offset_map_lock(mm, pmd, address, ptlp)	\
@@ -2452,16 +2462,14 @@  static inline bool pgtable_pmd_page_ctor(struct page *page)
 {
 	if (!pmd_ptlock_init(page))
 		return false;
-	__SetPageTable(page);
-	inc_lruvec_page_state(page, NR_PAGETABLE);
+	pgtable_page_inc(page);
 	return true;
 }
 
 static inline void pgtable_pmd_page_dtor(struct page *page)
 {
 	pmd_ptlock_free(page);
-	__ClearPageTable(page);
-	dec_lruvec_page_state(page, NR_PAGETABLE);
+	pgtable_page_dec(page);
 }
 
 /*