include/asm-generic/pgalloc.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
__{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.3On 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.
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 ..."?
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
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.
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)
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
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.
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
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
+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.
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.
© 2016 - 2025 Red Hat, Inc.