linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
@ 2025-11-07  9:59 Huacai Chen
  2025-11-07 11:21 ` Arnd Bergmann
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Huacai Chen @ 2025-11-07  9:59 UTC (permalink / raw)
  To: Huacai Chen, Andrew Morton
  Cc: Arnd Bergmann, Vishal Moola, Kevin Brodsky, Jan Kara, linux-mm,
	linux-arch, linux-kernel, Huacai Chen

__{pgd,p4d,pud,pmd,pte}_alloc_one_*() always allocate pages with GFP
flag GFP_PGTABLE_KERNEL/GFP_PGTABLE_USER. These two macros are defined
as follows:

 #define GFP_PGTABLE_KERNEL	(GFP_KERNEL | __GFP_ZERO)
 #define GFP_PGTABLE_USER	(GFP_PGTABLE_KERNEL | __GFP_ACCOUNT)

There is no __GFP_HIGHMEM in them, so we needn't to clear __GFP_HIGHMEM
explicitly.

Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
Resend because the lines begin with # was eaten by git.

 include/asm-generic/pgalloc.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h
index 3c8ec3bfea44..706e87b43b19 100644
--- a/include/asm-generic/pgalloc.h
+++ b/include/asm-generic/pgalloc.h
@@ -18,8 +18,7 @@
  */
 static inline pte_t *__pte_alloc_one_kernel_noprof(struct mm_struct *mm)
 {
-	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL &
-			~__GFP_HIGHMEM, 0);
+	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL, 0);
 
 	if (!ptdesc)
 		return NULL;
@@ -172,7 +171,6 @@ static inline pud_t *__pud_alloc_one_noprof(struct mm_struct *mm, unsigned long
 
 	if (mm == &init_mm)
 		gfp = GFP_PGTABLE_KERNEL;
-	gfp &= ~__GFP_HIGHMEM;
 
 	ptdesc = pagetable_alloc_noprof(gfp, 0);
 	if (!ptdesc)
@@ -226,7 +224,6 @@ static inline p4d_t *__p4d_alloc_one_noprof(struct mm_struct *mm, unsigned long
 
 	if (mm == &init_mm)
 		gfp = GFP_PGTABLE_KERNEL;
-	gfp &= ~__GFP_HIGHMEM;
 
 	ptdesc = pagetable_alloc_noprof(gfp, 0);
 	if (!ptdesc)
@@ -270,7 +267,6 @@ static inline pgd_t *__pgd_alloc_noprof(struct mm_struct *mm, unsigned int order
 
 	if (mm == &init_mm)
 		gfp = GFP_PGTABLE_KERNEL;
-	gfp &= ~__GFP_HIGHMEM;
 
 	ptdesc = pagetable_alloc_noprof(gfp, order);
 	if (!ptdesc)
-- 
2.47.3



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
  2025-11-07  9:59 [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM Huacai Chen
@ 2025-11-07 11:21 ` Arnd Bergmann
  2025-11-07 16:51   ` Vishal Moola (Oracle)
  2025-11-07 11:44 ` Lance Yang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2025-11-07 11:21 UTC (permalink / raw)
  To: Huacai Chen, Huacai Chen, Andrew Morton
  Cc: Vishal Moola, Kevin Brodsky, Jan Kara, linux-mm, Linux-Arch,
	linux-kernel

On Fri, Nov 7, 2025, at 10:59, Huacai Chen wrote:
> __{pgd,p4d,pud,pmd,pte}_alloc_one_*() always allocate pages with GFP
> flag GFP_PGTABLE_KERNEL/GFP_PGTABLE_USER. These two macros are defined
> as follows:
>
>  #define GFP_PGTABLE_KERNEL	(GFP_KERNEL | __GFP_ZERO)
>  #define GFP_PGTABLE_USER	(GFP_PGTABLE_KERNEL | __GFP_ACCOUNT)
>
> There is no __GFP_HIGHMEM in them, so we needn't to clear __GFP_HIGHMEM
> explicitly.
>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> Resend because the lines begin with # was eaten by git.

Thanks for your patch, this is an area I've also started
looking at, with the intention to reduce the references
to __GFO_HIGHMEM to the minimum we need for supporting the
remaining platforms that need to use highmem somewhere.

I'm not sure what the reason is for your patch, I assume
this is meant purely as a cleanup, correct? Are you looking
at a wider set of related cleanups, or did you just notice
this one instance?

Note that for the moment, the 32-bit arm __pte_alloc_one() function
still passes __GFP_HIGHMEM when CONFIG_HIGHPTE is set, though
I would like to remove that code path. Unless we remove
that at the same time, this should probably be explained in your
patch description.

      Arnd


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
  2025-11-07  9:59 [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM Huacai Chen
  2025-11-07 11:21 ` Arnd Bergmann
@ 2025-11-07 11:44 ` Lance Yang
  2025-11-07 12:50   ` Arnd Bergmann
  2025-11-07 16:58 ` Vishal Moola (Oracle)
  2025-11-09  7:44 ` Mike Rapoport
  3 siblings, 1 reply; 12+ messages in thread
From: Lance Yang @ 2025-11-07 11:44 UTC (permalink / raw)
  To: chenhuacai
  Cc: akpm, arnd, chenhuacai, jack, kevin.brodsky, linux-arch,
	linux-kernel, linux-mm, david, lorenzo.stoakes, vishal.moola,
	Lance Yang

From: Lance Yang <lance.yang@linux.dev>


On Fri,  7 Nov 2025 17:59:22 +0800, Huacai Chen wrote:
> __{pgd,p4d,pud,pmd,pte}_alloc_one_*() always allocate pages with GFP
> flag GFP_PGTABLE_KERNEL/GFP_PGTABLE_USER. These two macros are defined
> as follows:
> 
>  #define GFP_PGTABLE_KERNEL	(GFP_KERNEL | __GFP_ZERO)
>  #define GFP_PGTABLE_USER	(GFP_PGTABLE_KERNEL | __GFP_ACCOUNT)
> 
> There is no __GFP_HIGHMEM in them, so we needn't to clear __GFP_HIGHMEM
> explicitly.

Nice cleanup!

> 
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> Resend because the lines begin with # was eaten by git.
> 
>  include/asm-generic/pgalloc.h | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h
> index 3c8ec3bfea44..706e87b43b19 100644
> --- a/include/asm-generic/pgalloc.h
> +++ b/include/asm-generic/pgalloc.h
> @@ -18,8 +18,7 @@
>   */
>  static inline pte_t *__pte_alloc_one_kernel_noprof(struct mm_struct *mm)
>  {
> -	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL &
> -			~__GFP_HIGHMEM, 0);
> +	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL, 0);

I looked into the history and it seems you are right. This defensive pattern
was likely introduced by Vishal Moola in commit c787ae5[1].

After this cleanup, would it make sense to add a BUILD_BUG_ON() somewhere
to check that __GFP_HIGHMEM is not present in GFP_PGTABLE_KERNEL and
GFP_PGTABLE_USER? This would prevent any future regression ;)

Just a thought ...

[1] https://github.com/torvalds/linux/commit/c787ae5b391496f4f63bc942c18eb9fdee05741f

Cheers,
Lance

>  
>  	if (!ptdesc)
>  		return NULL;
> @@ -172,7 +171,6 @@ static inline pud_t *__pud_alloc_one_noprof(struct mm_struct *mm, unsigned long
>  
>  	if (mm == &init_mm)
>  		gfp = GFP_PGTABLE_KERNEL;
> -	gfp &= ~__GFP_HIGHMEM;
>  
>  	ptdesc = pagetable_alloc_noprof(gfp, 0);
>  	if (!ptdesc)
> @@ -226,7 +224,6 @@ static inline p4d_t *__p4d_alloc_one_noprof(struct mm_struct *mm, unsigned long
>  
>  	if (mm == &init_mm)
>  		gfp = GFP_PGTABLE_KERNEL;
> -	gfp &= ~__GFP_HIGHMEM;
>  
>  	ptdesc = pagetable_alloc_noprof(gfp, 0);
>  	if (!ptdesc)
> @@ -270,7 +267,6 @@ static inline pgd_t *__pgd_alloc_noprof(struct mm_struct *mm, unsigned int order
>  
>  	if (mm == &init_mm)
>  		gfp = GFP_PGTABLE_KERNEL;
> -	gfp &= ~__GFP_HIGHMEM;
>  
>  	ptdesc = pagetable_alloc_noprof(gfp, order);
>  	if (!ptdesc)


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
  2025-11-07 11:44 ` Lance Yang
@ 2025-11-07 12:50   ` Arnd Bergmann
  2025-11-07 14:17     ` Lance Yang
  2025-11-07 16:34     ` Vishal Moola (Oracle)
  0 siblings, 2 replies; 12+ messages in thread
From: Arnd Bergmann @ 2025-11-07 12:50 UTC (permalink / raw)
  To: Lance Yang, Huacai Chen
  Cc: Andrew Morton, Huacai Chen, Jan Kara, Kevin Brodsky, Linux-Arch,
	linux-kernel, linux-mm, david, Lorenzo Stoakes, vishal.moola,
	Lance Yang

On Fri, Nov 7, 2025, at 12:44, Lance Yang wrote:
> From: Lance Yang <lance.yang@linux.dev>
> On Fri,  7 Nov 2025 17:59:22 +0800, Huacai Chen wrote:
>> 
>>   */
>>  static inline pte_t *__pte_alloc_one_kernel_noprof(struct mm_struct *mm)
>>  {
>> -	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL &
>> -			~__GFP_HIGHMEM, 0);
>> +	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL, 0);
>
> I looked into the history and it seems you are right. This defensive pattern
> was likely introduced by Vishal Moola in commit c787ae5[1].

Right, so not even so long ago, so we need to make sure we agree
on a direction and don't send opposite patches in the name of
cleanups.

> After this cleanup, would it make sense to add a BUILD_BUG_ON() somewhere
> to check that __GFP_HIGHMEM is not present in GFP_PGTABLE_KERNEL and
> GFP_PGTABLE_USER? This would prevent any future regression ;)
>
> Just a thought ...

I think we can go either way here, but I'd tend towards not
adding more checks but instead removing any mention of __GFP_HIGHMEM
that we can show is either pointless or can be avoided, with
the goal of having only a small number of actual highmem
allocations remaining in places we do care about (normal
page cache, zram, possibly huge pages).

      Arnd


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
  2025-11-07 12:50   ` Arnd Bergmann
@ 2025-11-07 14:17     ` Lance Yang
  2025-11-07 16:34     ` Vishal Moola (Oracle)
  1 sibling, 0 replies; 12+ messages in thread
From: Lance Yang @ 2025-11-07 14:17 UTC (permalink / raw)
  To: Arnd Bergmann, Huacai Chen
  Cc: Andrew Morton, Huacai Chen, Jan Kara, Kevin Brodsky, Linux-Arch,
	linux-kernel, linux-mm, david, Lance Yang, Lorenzo Stoakes,
	vishal.moola



On 2025/11/7 20:50, Arnd Bergmann wrote:
> On Fri, Nov 7, 2025, at 12:44, Lance Yang wrote:
>> From: Lance Yang <lance.yang@linux.dev>
>> On Fri,  7 Nov 2025 17:59:22 +0800, Huacai Chen wrote:
>>>
>>>    */
>>>   static inline pte_t *__pte_alloc_one_kernel_noprof(struct mm_struct *mm)
>>>   {
>>> -	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL &
>>> -			~__GFP_HIGHMEM, 0);
>>> +	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL, 0);
>>
>> I looked into the history and it seems you are right. This defensive pattern
>> was likely introduced by Vishal Moola in commit c787ae5[1].
> 
> Right, so not even so long ago, so we need to make sure we agree
> on a direction and don't send opposite patches in the name of
> cleanups.

Yes, better to get on the same page now than to have conflicting
cleanups down the line ;)

> 
>> After this cleanup, would it make sense to add a BUILD_BUG_ON() somewhere
>> to check that __GFP_HIGHMEM is not present in GFP_PGTABLE_KERNEL and
>> GFP_PGTABLE_USER? This would prevent any future regression ;)
>>
>> Just a thought ...
> 
> I think we can go either way here, but I'd tend towards not
> adding more checks but instead removing any mention of __GFP_HIGHMEM
> that we can show is either pointless or can be avoided, with

Makes sense to me :)

> the goal of having only a small number of actual highmem
> allocations remaining in places we do care about (normal
> page cache, zram, possibly huge pages).

Right! That's the ideal end state. Making the code cleaner and
the intention clearer ;p

Cheers,
Lance


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
  2025-11-07 12:50   ` Arnd Bergmann
  2025-11-07 14:17     ` Lance Yang
@ 2025-11-07 16:34     ` Vishal Moola (Oracle)
  1 sibling, 0 replies; 12+ messages in thread
From: Vishal Moola (Oracle) @ 2025-11-07 16:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Lance Yang, Huacai Chen, Andrew Morton, Huacai Chen, Jan Kara,
	Kevin Brodsky, Linux-Arch, linux-kernel, linux-mm, david,
	Lorenzo Stoakes, Lance Yang

On Fri, Nov 07, 2025 at 01:50:06PM +0100, Arnd Bergmann wrote:
> On Fri, Nov 7, 2025, at 12:44, Lance Yang wrote:
> > From: Lance Yang <lance.yang@linux.dev>
> > On Fri,  7 Nov 2025 17:59:22 +0800, Huacai Chen wrote:
> >> 
> >>   */
> >>  static inline pte_t *__pte_alloc_one_kernel_noprof(struct mm_struct *mm)
> >>  {
> >> -	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL &
> >> -			~__GFP_HIGHMEM, 0);
> >> +	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL, 0);
> >
> > I looked into the history and it seems you are right. This defensive pattern
> > was likely introduced by Vishal Moola in commit c787ae5[1].
> 
> Right, so not even so long ago, so we need to make sure we agree
> on a direction and don't send opposite patches in the name of
> cleanups.

I took a look, this patch is the direction we want to go :).

> > After this cleanup, would it make sense to add a BUILD_BUG_ON() somewhere
> > to check that __GFP_HIGHMEM is not present in GFP_PGTABLE_KERNEL and
> > GFP_PGTABLE_USER? This would prevent any future regression ;)
> >
> > Just a thought ...

In this case, I don't think the extra check is necessary. This is a
remnant of defensively converting callers to the ptdesc api from
get_free_pages() variants (which masks off GFP_HIGHMEM internally).
I doubt we'll ever be changing those macros to support highmem.

> I think we can go either way here, but I'd tend towards not
> adding more checks but instead removing any mention of __GFP_HIGHMEM
> that we can show is either pointless or can be avoided, with
> the goal of having only a small number of actual highmem
> allocations remaining in places we do care about (normal
> page cache, zram, possibly huge pages).

+1

I'm not familiar with which architectures use highmem or not, so
theres likely more cases like this patch that are remnants of the
ptdesc conversion.

git grep "pagetable_alloc.*GFP_HIGHMEM" shows at least 5 references
inline that can definitely be removed. I'll go ahead and clean those up,
but I'll leave the rest to people more familiar with the architectures.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
  2025-11-07 11:21 ` Arnd Bergmann
@ 2025-11-07 16:51   ` Vishal Moola (Oracle)
  2025-11-09  7:43     ` Mike Rapoport
  0 siblings, 1 reply; 12+ messages in thread
From: Vishal Moola (Oracle) @ 2025-11-07 16:51 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Huacai Chen, Huacai Chen, Andrew Morton, Kevin Brodsky, Jan Kara,
	linux-mm, Linux-Arch, linux-kernel, Mike Rapoport

+Cc: Mike

On Fri, Nov 07, 2025 at 12:21:38PM +0100, Arnd Bergmann wrote:
> On Fri, Nov 7, 2025, at 10:59, Huacai Chen wrote:
> > __{pgd,p4d,pud,pmd,pte}_alloc_one_*() always allocate pages with GFP
> > flag GFP_PGTABLE_KERNEL/GFP_PGTABLE_USER. These two macros are defined
> > as follows:
> >
> >  #define GFP_PGTABLE_KERNEL	(GFP_KERNEL | __GFP_ZERO)
> >  #define GFP_PGTABLE_USER	(GFP_PGTABLE_KERNEL | __GFP_ACCOUNT)
> >
> > There is no __GFP_HIGHMEM in them, so we needn't to clear __GFP_HIGHMEM
> > explicitly.
> >
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > Resend because the lines begin with # was eaten by git.
> 
> Thanks for your patch, this is an area I've also started
> looking at, with the intention to reduce the references
> to __GFO_HIGHMEM to the minimum we need for supporting the
> remaining platforms that need to use highmem somewhere.

Yay! Thanks for doing that, I like less highmem :)

> I'm not sure what the reason is for your patch, I assume
> this is meant purely as a cleanup, correct? Are you looking
> at a wider set of related cleanups, or did you just notice
> this one instance?
> 
> Note that for the moment, the 32-bit arm __pte_alloc_one() function
> still passes __GFP_HIGHMEM when CONFIG_HIGHPTE is set, though
> I would like to remove that code path. Unless we remove
> that at the same time, this should probably be explained in your
> patch description.

Skimming the functions, __pte_alloc_one_kernel() doesn't get passed in
a gfp, while __pte_alloc_one() does. IOW I __pte_alloc_one_kernel()
cares about architecture gfp, while the latter does care - so they are
2 very different cases.

Might be helpful to explain, although I don't think it matters much.

I've cc-ed Mike, he might have more useful opinions these functions.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
  2025-11-07  9:59 [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM Huacai Chen
  2025-11-07 11:21 ` Arnd Bergmann
  2025-11-07 11:44 ` Lance Yang
@ 2025-11-07 16:58 ` Vishal Moola (Oracle)
  2025-11-08  8:34   ` Huacai Chen
  2025-11-09  7:44 ` Mike Rapoport
  3 siblings, 1 reply; 12+ messages in thread
From: Vishal Moola (Oracle) @ 2025-11-07 16:58 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Huacai Chen, Andrew Morton, Arnd Bergmann, Kevin Brodsky,
	Jan Kara, linux-mm, linux-arch, linux-kernel

On Fri, Nov 07, 2025 at 05:59:22PM +0800, Huacai Chen wrote:
> __{pgd,p4d,pud,pmd,pte}_alloc_one_*() always allocate pages with GFP
> flag GFP_PGTABLE_KERNEL/GFP_PGTABLE_USER. These two macros are defined
> as follows:
> 
>  #define GFP_PGTABLE_KERNEL	(GFP_KERNEL | __GFP_ZERO)
>  #define GFP_PGTABLE_USER	(GFP_PGTABLE_KERNEL | __GFP_ACCOUNT)
> 
> There is no __GFP_HIGHMEM in them, so we needn't to clear __GFP_HIGHMEM
> explicitly.
> 
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---

I'm not really sure what "Refine ... about HIGHMEM" is supposed to mean.
Might it be clearer to title this something like "Remove unnecessary
highmem in ..."?


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
  2025-11-07 16:58 ` Vishal Moola (Oracle)
@ 2025-11-08  8:34   ` Huacai Chen
  2025-11-08 16:47     ` Andrew Morton
  0 siblings, 1 reply; 12+ messages in thread
From: Huacai Chen @ 2025-11-08  8:34 UTC (permalink / raw)
  To: Vishal Moola (Oracle)
  Cc: Huacai Chen, Andrew Morton, Arnd Bergmann, Kevin Brodsky,
	Jan Kara, linux-mm, linux-arch, linux-kernel

Hi, Vishal,

On Sat, Nov 8, 2025 at 12:59 AM Vishal Moola (Oracle)
<vishal.moola@gmail.com> wrote:
>
> On Fri, Nov 07, 2025 at 05:59:22PM +0800, Huacai Chen wrote:
> > __{pgd,p4d,pud,pmd,pte}_alloc_one_*() always allocate pages with GFP
> > flag GFP_PGTABLE_KERNEL/GFP_PGTABLE_USER. These two macros are defined
> > as follows:
> >
> >  #define GFP_PGTABLE_KERNEL   (GFP_KERNEL | __GFP_ZERO)
> >  #define GFP_PGTABLE_USER     (GFP_PGTABLE_KERNEL | __GFP_ACCOUNT)
> >
> > There is no __GFP_HIGHMEM in them, so we needn't to clear __GFP_HIGHMEM
> > explicitly.
> >
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
>
> I'm not really sure what "Refine ... about HIGHMEM" is supposed to mean.
> Might it be clearer to title this something like "Remove unnecessary
> highmem in ..."?
Yes, that is better, but Andrew has picked this patch, should I resend
a new version?

Huacai


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
  2025-11-08  8:34   ` Huacai Chen
@ 2025-11-08 16:47     ` Andrew Morton
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Morton @ 2025-11-08 16:47 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Vishal Moola (Oracle), Huacai Chen, Arnd Bergmann, Kevin Brodsky,
	Jan Kara, linux-mm, linux-arch, linux-kernel

On Sat, 8 Nov 2025 16:34:20 +0800 Huacai Chen <chenhuacai@kernel.org> wrote:

> Hi, Vishal,
> 
> On Sat, Nov 8, 2025 at 12:59 AM Vishal Moola (Oracle)
> <vishal.moola@gmail.com> wrote:
> >
> > On Fri, Nov 07, 2025 at 05:59:22PM +0800, Huacai Chen wrote:
> > > __{pgd,p4d,pud,pmd,pte}_alloc_one_*() always allocate pages with GFP
> > > flag GFP_PGTABLE_KERNEL/GFP_PGTABLE_USER. These two macros are defined
> > > as follows:
> > >
> > >  #define GFP_PGTABLE_KERNEL   (GFP_KERNEL | __GFP_ZERO)
> > >  #define GFP_PGTABLE_USER     (GFP_PGTABLE_KERNEL | __GFP_ACCOUNT)
> > >
> > > There is no __GFP_HIGHMEM in them, so we needn't to clear __GFP_HIGHMEM
> > > explicitly.
> > >
> > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > > ---
> >
> > I'm not really sure what "Refine ... about HIGHMEM" is supposed to mean.
> > Might it be clearer to title this something like "Remove unnecessary
> > highmem in ..."?
> Yes, that is better, but Andrew has picked this patch, should I resend
> a new version?

Please just send along a v2 in the usual fashion.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
  2025-11-07 16:51   ` Vishal Moola (Oracle)
@ 2025-11-09  7:43     ` Mike Rapoport
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Rapoport @ 2025-11-09  7:43 UTC (permalink / raw)
  To: Vishal Moola (Oracle)
  Cc: Arnd Bergmann, Huacai Chen, Huacai Chen, Andrew Morton,
	Kevin Brodsky, Jan Kara, linux-mm, Linux-Arch, linux-kernel

On Fri, Nov 07, 2025 at 08:51:38AM -0800, Vishal Moola (Oracle) wrote:
> +Cc: Mike
> 
> On Fri, Nov 07, 2025 at 12:21:38PM +0100, Arnd Bergmann wrote:
> > On Fri, Nov 7, 2025, at 10:59, Huacai Chen wrote:
> > > __{pgd,p4d,pud,pmd,pte}_alloc_one_*() always allocate pages with GFP
> > > flag GFP_PGTABLE_KERNEL/GFP_PGTABLE_USER. These two macros are defined
> > > as follows:
> > >
> > >  #define GFP_PGTABLE_KERNEL	(GFP_KERNEL | __GFP_ZERO)
> > >  #define GFP_PGTABLE_USER	(GFP_PGTABLE_KERNEL | __GFP_ACCOUNT)
> > >
> > > There is no __GFP_HIGHMEM in them, so we needn't to clear __GFP_HIGHMEM
> > > explicitly.
> > >
> > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > > ---
> > > Resend because the lines begin with # was eaten by git.
> > 
> > Thanks for your patch, this is an area I've also started
> > looking at, with the intention to reduce the references
> > to __GFO_HIGHMEM to the minimum we need for supporting the
> > remaining platforms that need to use highmem somewhere.
> 
> Yay! Thanks for doing that, I like less highmem :)
> 
> > I'm not sure what the reason is for your patch, I assume
> > this is meant purely as a cleanup, correct? Are you looking
> > at a wider set of related cleanups, or did you just notice
> > this one instance?
> > 
> > Note that for the moment, the 32-bit arm __pte_alloc_one() function
> > still passes __GFP_HIGHMEM when CONFIG_HIGHPTE is set, though
> > I would like to remove that code path. Unless we remove
> > that at the same time, this should probably be explained in your
> > patch description.
> 
> Skimming the functions, __pte_alloc_one_kernel() doesn't get passed in
> a gfp, while __pte_alloc_one() does. IOW I __pte_alloc_one_kernel()
> cares about architecture gfp, while the latter does care - so they are
> 2 very different cases.

__pte_alloc_one() has gfp parameter to accommodate CONFIG_HIGHPTE that x86
used to have until quite recently and arm still has.
 
> Might be helpful to explain, although I don't think it matters much.
> 
> I've cc-ed Mike, he might have more useful opinions these functions.
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM
  2025-11-07  9:59 [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM Huacai Chen
                   ` (2 preceding siblings ...)
  2025-11-07 16:58 ` Vishal Moola (Oracle)
@ 2025-11-09  7:44 ` Mike Rapoport
  3 siblings, 0 replies; 12+ messages in thread
From: Mike Rapoport @ 2025-11-09  7:44 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Huacai Chen, Andrew Morton, Arnd Bergmann, Vishal Moola,
	Kevin Brodsky, Jan Kara, linux-mm, linux-arch, linux-kernel

On Fri, Nov 07, 2025 at 05:59:22PM +0800, Huacai Chen wrote:
> __{pgd,p4d,pud,pmd,pte}_alloc_one_*() always allocate pages with GFP
> flag GFP_PGTABLE_KERNEL/GFP_PGTABLE_USER. These two macros are defined
> as follows:
> 
>  #define GFP_PGTABLE_KERNEL	(GFP_KERNEL | __GFP_ZERO)
>  #define GFP_PGTABLE_USER	(GFP_PGTABLE_KERNEL | __GFP_ACCOUNT)
> 
> There is no __GFP_HIGHMEM in them, so we needn't to clear __GFP_HIGHMEM
> explicitly.
> 
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>

Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
> Resend because the lines begin with # was eaten by git.
> 
>  include/asm-generic/pgalloc.h | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h
> index 3c8ec3bfea44..706e87b43b19 100644
> --- a/include/asm-generic/pgalloc.h
> +++ b/include/asm-generic/pgalloc.h
> @@ -18,8 +18,7 @@
>   */
>  static inline pte_t *__pte_alloc_one_kernel_noprof(struct mm_struct *mm)
>  {
> -	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL &
> -			~__GFP_HIGHMEM, 0);
> +	struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL, 0);
>  
>  	if (!ptdesc)
>  		return NULL;
> @@ -172,7 +171,6 @@ static inline pud_t *__pud_alloc_one_noprof(struct mm_struct *mm, unsigned long
>  
>  	if (mm == &init_mm)
>  		gfp = GFP_PGTABLE_KERNEL;
> -	gfp &= ~__GFP_HIGHMEM;
>  
>  	ptdesc = pagetable_alloc_noprof(gfp, 0);
>  	if (!ptdesc)
> @@ -226,7 +224,6 @@ static inline p4d_t *__p4d_alloc_one_noprof(struct mm_struct *mm, unsigned long
>  
>  	if (mm == &init_mm)
>  		gfp = GFP_PGTABLE_KERNEL;
> -	gfp &= ~__GFP_HIGHMEM;
>  
>  	ptdesc = pagetable_alloc_noprof(gfp, 0);
>  	if (!ptdesc)
> @@ -270,7 +267,6 @@ static inline pgd_t *__pgd_alloc_noprof(struct mm_struct *mm, unsigned int order
>  
>  	if (mm == &init_mm)
>  		gfp = GFP_PGTABLE_KERNEL;
> -	gfp &= ~__GFP_HIGHMEM;
>  
>  	ptdesc = pagetable_alloc_noprof(gfp, order);
>  	if (!ptdesc)
> -- 
> 2.47.3
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-11-09  7:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-07  9:59 [PATCH Resend] mm: Refine __{pgd,p4d,pud,pmd,pte}_alloc_one_*() about HIGHMEM Huacai Chen
2025-11-07 11:21 ` Arnd Bergmann
2025-11-07 16:51   ` Vishal Moola (Oracle)
2025-11-09  7:43     ` Mike Rapoport
2025-11-07 11:44 ` Lance Yang
2025-11-07 12:50   ` Arnd Bergmann
2025-11-07 14:17     ` Lance Yang
2025-11-07 16:34     ` Vishal Moola (Oracle)
2025-11-07 16:58 ` Vishal Moola (Oracle)
2025-11-08  8:34   ` Huacai Chen
2025-11-08 16:47     ` Andrew Morton
2025-11-09  7:44 ` Mike Rapoport

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).