[PATCH] arm64/mm: Remove add_huge_page_size()

[PATCH] arm64/mm: Remove add_huge_page_size()

[Gavin Shan]

The function add_huge_page_size(), wrapper of hugetlb_add_hstate(),
avoids to register duplicated huge page states for same size. However,
the same logic has been included in hugetlb_add_hstate(). So it seems
unnecessary to keep add_huge_page_size() and this just removes it.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 arch/arm64/mm/hugetlbpage.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index bbeb6a5a6ba6..ed7530413941 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -441,22 +441,14 @@ void huge_ptep_clear_flush(struct vm_area_struct *vma,
 	clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig);
 }
 
-static void __init add_huge_page_size(unsigned long size)
-{
-	if (size_to_hstate(size))
-		return;
-
-	hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
-}
-
 static int __init hugetlbpage_init(void)
 {
 #ifdef CONFIG_ARM64_4K_PAGES
-	add_huge_page_size(PUD_SIZE);
+	hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
 #endif
-	add_huge_page_size(CONT_PMD_SIZE);
-	add_huge_page_size(PMD_SIZE);
-	add_huge_page_size(CONT_PTE_SIZE);
+	hugetlb_add_hstate(CONT_PMD_SHIFT + PMD_SHIFT - PAGE_SHIFT);
+	hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
+	hugetlb_add_hstate(CONT_PTE_SHIFT);
 
 	return 0;
 }
@@ -473,7 +465,7 @@ static __init int setup_hugepagesz(char *opt)
 	case CONT_PMD_SIZE:
 	case PMD_SIZE:
 	case CONT_PTE_SIZE:
-		add_huge_page_size(ps);
+		hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
 		return 1;
 	}
 
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/mm: Remove add_huge_page_size()

[Anshuman Khandual]

On 05/06/2020 12:16 PM, Gavin Shan wrote:
> The function add_huge_page_size(), wrapper of hugetlb_add_hstate(),
> avoids to register duplicated huge page states for same size. However,
> the same logic has been included in hugetlb_add_hstate(). So it seems
> unnecessary to keep add_huge_page_size() and this just removes it.

Makes sense.

> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>  arch/arm64/mm/hugetlbpage.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index bbeb6a5a6ba6..ed7530413941 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -441,22 +441,14 @@ void huge_ptep_clear_flush(struct vm_area_struct *vma,
>  	clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig);
>  }
>  
> -static void __init add_huge_page_size(unsigned long size)
> -{
> -	if (size_to_hstate(size))
> -		return;
> -
> -	hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
> -}
> -
>  static int __init hugetlbpage_init(void)
>  {
>  #ifdef CONFIG_ARM64_4K_PAGES
> -	add_huge_page_size(PUD_SIZE);
> +	hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
>  #endif
> -	add_huge_page_size(CONT_PMD_SIZE);
> -	add_huge_page_size(PMD_SIZE);
> -	add_huge_page_size(CONT_PTE_SIZE);
> +	hugetlb_add_hstate(CONT_PMD_SHIFT + PMD_SHIFT - PAGE_SHIFT);
> +	hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
> +	hugetlb_add_hstate(CONT_PTE_SHIFT);

Should these page order values be converted into macros instead. Also
we should probably keep (CONT_PTE_SHIFT + PAGE_SHIFT - PAGE_SHIFT) as
is to make things more clear.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/mm: Remove add_huge_page_size()

[Will Deacon]

On Wed, May 06, 2020 at 12:36:43PM +0530, Anshuman Khandual wrote:
> 
> 
> On 05/06/2020 12:16 PM, Gavin Shan wrote:
> > The function add_huge_page_size(), wrapper of hugetlb_add_hstate(),
> > avoids to register duplicated huge page states for same size. However,
> > the same logic has been included in hugetlb_add_hstate(). So it seems
> > unnecessary to keep add_huge_page_size() and this just removes it.
> 
> Makes sense.
> 
> > 
> > Signed-off-by: Gavin Shan <gshan@redhat.com>
> > ---
> >  arch/arm64/mm/hugetlbpage.c | 18 +++++-------------
> >  1 file changed, 5 insertions(+), 13 deletions(-)
> > 
> > diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> > index bbeb6a5a6ba6..ed7530413941 100644
> > --- a/arch/arm64/mm/hugetlbpage.c
> > +++ b/arch/arm64/mm/hugetlbpage.c
> > @@ -441,22 +441,14 @@ void huge_ptep_clear_flush(struct vm_area_struct *vma,
> >  	clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig);
> >  }
> >  
> > -static void __init add_huge_page_size(unsigned long size)
> > -{
> > -	if (size_to_hstate(size))
> > -		return;
> > -
> > -	hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
> > -}
> > -
> >  static int __init hugetlbpage_init(void)
> >  {
> >  #ifdef CONFIG_ARM64_4K_PAGES
> > -	add_huge_page_size(PUD_SIZE);
> > +	hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
> >  #endif
> > -	add_huge_page_size(CONT_PMD_SIZE);
> > -	add_huge_page_size(PMD_SIZE);
> > -	add_huge_page_size(CONT_PTE_SIZE);
> > +	hugetlb_add_hstate(CONT_PMD_SHIFT + PMD_SHIFT - PAGE_SHIFT);
> > +	hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
> > +	hugetlb_add_hstate(CONT_PTE_SHIFT);

Something similar has already been done in linux-next.

> Should these page order values be converted into macros instead. Also
> we should probably keep (CONT_PTE_SHIFT + PAGE_SHIFT - PAGE_SHIFT) as
> is to make things more clear.

I think the real confusion stems from us not being consistent with your
*_SHIFT definitions on arm64. It's madness for CONT_PTE_SHIFT to be smaller
than PAGE_SHIFT imo, but it's just cosmetic I guess.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/mm: Remove add_huge_page_size()

[Gavin Shan]

On 5/6/20 5:19 PM, Will Deacon wrote:
> On Wed, May 06, 2020 at 12:36:43PM +0530, Anshuman Khandual wrote:
>>
>>
>> On 05/06/2020 12:16 PM, Gavin Shan wrote:
>>> The function add_huge_page_size(), wrapper of hugetlb_add_hstate(),
>>> avoids to register duplicated huge page states for same size. However,
>>> the same logic has been included in hugetlb_add_hstate(). So it seems
>>> unnecessary to keep add_huge_page_size() and this just removes it.
>>
>> Makes sense.
>>
>>>
>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>> ---
>>>   arch/arm64/mm/hugetlbpage.c | 18 +++++-------------
>>>   1 file changed, 5 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
>>> index bbeb6a5a6ba6..ed7530413941 100644
>>> --- a/arch/arm64/mm/hugetlbpage.c
>>> +++ b/arch/arm64/mm/hugetlbpage.c
>>> @@ -441,22 +441,14 @@ void huge_ptep_clear_flush(struct vm_area_struct *vma,
>>>   	clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig);
>>>   }
>>>   
>>> -static void __init add_huge_page_size(unsigned long size)
>>> -{
>>> -	if (size_to_hstate(size))
>>> -		return;
>>> -
>>> -	hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
>>> -}
>>> -
>>>   static int __init hugetlbpage_init(void)
>>>   {
>>>   #ifdef CONFIG_ARM64_4K_PAGES
>>> -	add_huge_page_size(PUD_SIZE);
>>> +	hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
>>>   #endif
>>> -	add_huge_page_size(CONT_PMD_SIZE);
>>> -	add_huge_page_size(PMD_SIZE);
>>> -	add_huge_page_size(CONT_PTE_SIZE);
>>> +	hugetlb_add_hstate(CONT_PMD_SHIFT + PMD_SHIFT - PAGE_SHIFT);
>>> +	hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
>>> +	hugetlb_add_hstate(CONT_PTE_SHIFT);
> 
> Something similar has already been done in linux-next.
> 

Thanks, Will. I didn't check linux-next before posting this patch.
Please ignore it then :)

>> Should these page order values be converted into macros instead. Also
>> we should probably keep (CONT_PTE_SHIFT + PAGE_SHIFT - PAGE_SHIFT) as
>> is to make things more clear.
> 
> I think the real confusion stems from us not being consistent with your
> *_SHIFT definitions on arm64. It's madness for CONT_PTE_SHIFT to be smaller
> than PAGE_SHIFT imo, but it's just cosmetic I guess.
> 

Yeah, Do you want me to post a patch, to fix it?

> Will
> 

Thanks,
Gavin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/mm: Remove add_huge_page_size()

[Will Deacon]

On Thu, May 07, 2020 at 10:15:59AM +1000, Gavin Shan wrote:
> On 5/6/20 5:19 PM, Will Deacon wrote:
> > On Wed, May 06, 2020 at 12:36:43PM +0530, Anshuman Khandual wrote:
> > > 
> > > 
> > > On 05/06/2020 12:16 PM, Gavin Shan wrote:
> > > > The function add_huge_page_size(), wrapper of hugetlb_add_hstate(),
> > > > avoids to register duplicated huge page states for same size. However,
> > > > the same logic has been included in hugetlb_add_hstate(). So it seems
> > > > unnecessary to keep add_huge_page_size() and this just removes it.
> > > 
> > > Makes sense.
> > > 
> > > > 
> > > > Signed-off-by: Gavin Shan <gshan@redhat.com>
> > > > ---
> > > >   arch/arm64/mm/hugetlbpage.c | 18 +++++-------------
> > > >   1 file changed, 5 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> > > > index bbeb6a5a6ba6..ed7530413941 100644
> > > > --- a/arch/arm64/mm/hugetlbpage.c
> > > > +++ b/arch/arm64/mm/hugetlbpage.c
> > > > @@ -441,22 +441,14 @@ void huge_ptep_clear_flush(struct vm_area_struct *vma,
> > > >   	clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig);
> > > >   }
> > > > -static void __init add_huge_page_size(unsigned long size)
> > > > -{
> > > > -	if (size_to_hstate(size))
> > > > -		return;
> > > > -
> > > > -	hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
> > > > -}
> > > > -
> > > >   static int __init hugetlbpage_init(void)
> > > >   {
> > > >   #ifdef CONFIG_ARM64_4K_PAGES
> > > > -	add_huge_page_size(PUD_SIZE);
> > > > +	hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
> > > >   #endif
> > > > -	add_huge_page_size(CONT_PMD_SIZE);
> > > > -	add_huge_page_size(PMD_SIZE);
> > > > -	add_huge_page_size(CONT_PTE_SIZE);
> > > > +	hugetlb_add_hstate(CONT_PMD_SHIFT + PMD_SHIFT - PAGE_SHIFT);
> > > > +	hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
> > > > +	hugetlb_add_hstate(CONT_PTE_SHIFT);
> > 
> > Something similar has already been done in linux-next.
> > 
> 
> Thanks, Will. I didn't check linux-next before posting this patch.
> Please ignore it then :)
> 
> > > Should these page order values be converted into macros instead. Also
> > > we should probably keep (CONT_PTE_SHIFT + PAGE_SHIFT - PAGE_SHIFT) as
> > > is to make things more clear.
> > 
> > I think the real confusion stems from us not being consistent with your
> > *_SHIFT definitions on arm64. It's madness for CONT_PTE_SHIFT to be smaller
> > than PAGE_SHIFT imo, but it's just cosmetic I guess.
> > 
> 
> Yeah, Do you want me to post a patch, to fix it?

Let's wait until 5.8 is out the door first, since we've already got a fair
amount of activity in this area and it would be a pity if we broke something
as a result of cleanup!

Cheers,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel