[PATCH v5 0/2] Generalize vmemmap_populate_hugepages to sparc

[PATCH v5 0/2] Generalize vmemmap_populate_hugepages to sparc

[chengkaitao]

Change sparc's implementation of vmemmap_populate() using
vmemmap_populate_hugepages() to streamline the code. Another
benefit is that it allows us to eliminate the external declarations
of vmemmap_p?d_populate functions and convert them to static functions.

Since vmemmap_populate_hugepages may fallback to vmemmap_populate-
_basepages, which differs from sparc's original implementation.
During the v1 discussion with Mike Rapoport, sparc uses base pages
in the kernel page tables, so it should be able to use them in
vmemmap as well. Consequently, no additional special handling is
required.

Remove unnecessary whitespace from sparc.

Changes in v5:
- remove [PATCH v4 3/3] sparc: Remove unnecessary whitespace

Changes in v4:
- Add commit message
- Add verification that pmd actually maps a large page in the
vmemmap_check_pmd function

Changes in v3:
- Allow sparc to fallback to vmemmap_populate_basepages
- Convert vmemmap_p?d_populate() to static functions
- Split the v1 patch

Changes in v2:
- Revert the whitespace deletions
- Change vmemmap_false_pmd to vmemmap_pte_fallback_allowed

Link to V4:
https://lore.kernel.org/all/20251219115812.65855-1-pilgrimtao@gmail.com/
Link to V3:
https://lore.kernel.org/all/20251218174749.45965-1-pilgrimtao@gmail.com/
Link to V2:
https://lore.kernel.org/all/20251218130957.36892-1-pilgrimtao@gmail.com/
Link to V1:
https://lore.kernel.org/all/20251217120858.18713-1-pilgrimtao@gmail.com/

Chengkaitao (2):
  sparc: Use vmemmap_populate_hugepages for vmemmap_populate
  mm: Convert vmemmap_p?d_populate() to static functions

 arch/sparc/mm/init_64.c | 47 ++++++++++++++---------------------------
 include/linux/mm.h      |  7 ------
 mm/sparse-vmemmap.c     | 10 ++++-----
 3 files changed, 21 insertions(+), 43 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH v5 1/2] sparc: Use vmemmap_populate_hugepages for vmemmap_populate

[chengkaitao]

From: Chengkaitao <chengkaitao@kylinos.cn>

1. In the SPARC architecture, reimplemented vmemmap_populate using
vmemmap_populate_hugepages.
2. Allow the SPARC arch to fallback to vmemmap_populate_basepages(),
when vmemmap_alloc_block returns NULL.

Signed-off-by: Chengkaitao <chengkaitao@kylinos.cn>
Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/sparc/mm/init_64.c | 47 ++++++++++++++---------------------------
 1 file changed, 16 insertions(+), 31 deletions(-)

diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index df9f7c444c39..858eaa6615ea 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -2581,8 +2581,8 @@ unsigned long _PAGE_CACHE __read_mostly;
 EXPORT_SYMBOL(_PAGE_CACHE);
 
 #ifdef CONFIG_SPARSEMEM_VMEMMAP
-int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
-			       int node, struct vmem_altmap *altmap)
+void __meminit vmemmap_set_pmd(pmd_t *pmd, void *p, int node,
+			       unsigned long addr, unsigned long next)
 {
 	unsigned long pte_base;
 
@@ -2595,39 +2595,24 @@ int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
 
 	pte_base |= _PAGE_PMD_HUGE;
 
-	vstart = vstart & PMD_MASK;
-	vend = ALIGN(vend, PMD_SIZE);
-	for (; vstart < vend; vstart += PMD_SIZE) {
-		pgd_t *pgd = vmemmap_pgd_populate(vstart, node);
-		unsigned long pte;
-		p4d_t *p4d;
-		pud_t *pud;
-		pmd_t *pmd;
-
-		if (!pgd)
-			return -ENOMEM;
-
-		p4d = vmemmap_p4d_populate(pgd, vstart, node);
-		if (!p4d)
-			return -ENOMEM;
-
-		pud = vmemmap_pud_populate(p4d, vstart, node);
-		if (!pud)
-			return -ENOMEM;
+	pmd_val(*pmd) = pte_base | __pa(p);
+}
 
-		pmd = pmd_offset(pud, vstart);
-		pte = pmd_val(*pmd);
-		if (!(pte & _PAGE_VALID)) {
-			void *block = vmemmap_alloc_block(PMD_SIZE, node);
+int __meminit vmemmap_check_pmd(pmd_t *pmdp, int node,
+				unsigned long addr, unsigned long next)
+{
+	int large = pmd_leaf(*pmdp);
 
-			if (!block)
-				return -ENOMEM;
+	if (large)
+		vmemmap_verify((pte_t *)pmdp, node, addr, next);
 
-			pmd_val(*pmd) = pte_base | __pa(block);
-		}
-	}
+	return large;
+}
 
-	return 0;
+int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
+			       int node, struct vmem_altmap *altmap)
+{
+	return vmemmap_populate_hugepages(vstart, vend, node, altmap);
 }
 #endif /* CONFIG_SPARSEMEM_VMEMMAP */
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v5 1/2] sparc: Use vmemmap_populate_hugepages for vmemmap_populate

[Andreas Larsson]

On 2026-01-11 08:44, chengkaitao wrote:
> From: Chengkaitao <chengkaitao@kylinos.cn>
> 
> 1. In the SPARC architecture, reimplemented vmemmap_populate using
> vmemmap_populate_hugepages.
> 2. Allow the SPARC arch to fallback to vmemmap_populate_basepages(),
> when vmemmap_alloc_block returns NULL.

This patch seems to potentially make more functional changes than what
the descriptions gives impression of.

Given the amount of changes this seems to introduce, more on that below,
I'd like to see more description on the changes and why they can be done
than this.

Nit: use active language, "reimplement", not "reimplemented".


> Signed-off-by: Chengkaitao <chengkaitao@kylinos.cn>
> Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>  arch/sparc/mm/init_64.c | 47 ++++++++++++++---------------------------
>  1 file changed, 16 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
> index df9f7c444c39..858eaa6615ea 100644
> --- a/arch/sparc/mm/init_64.c
> +++ b/arch/sparc/mm/init_64.c
> @@ -2581,8 +2581,8 @@ unsigned long _PAGE_CACHE __read_mostly;
>  EXPORT_SYMBOL(_PAGE_CACHE);
>  
>  #ifdef CONFIG_SPARSEMEM_VMEMMAP
> -int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
> -			       int node, struct vmem_altmap *altmap)
> +void __meminit vmemmap_set_pmd(pmd_t *pmd, void *p, int node,
> +			       unsigned long addr, unsigned long next)
>  {
>  	unsigned long pte_base;
>  
> @@ -2595,39 +2595,24 @@ int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
>  
>  	pte_base |= _PAGE_PMD_HUGE;
>  
> -	vstart = vstart & PMD_MASK;
> -	vend = ALIGN(vend, PMD_SIZE);

It seems that this patch removes alignment of both start and end. Is
this a functional change in practice or are these always aligned for
some other reason?


> -	for (; vstart < vend; vstart += PMD_SIZE) {
> -		pgd_t *pgd = vmemmap_pgd_populate(vstart, node);
> -		unsigned long pte;
> -		p4d_t *p4d;
> -		pud_t *pud;
> -		pmd_t *pmd;
> -
> -		if (!pgd)
> -			return -ENOMEM;
> -
> -		p4d = vmemmap_p4d_populate(pgd, vstart, node);
> -		if (!p4d)
> -			return -ENOMEM;
> -
> -		pud = vmemmap_pud_populate(p4d, vstart, node);
> -		if (!pud)
> -			return -ENOMEM;
> +	pmd_val(*pmd) = pte_base | __pa(p);
> +}
>  
> -		pmd = pmd_offset(pud, vstart);
> -		pte = pmd_val(*pmd);
> -		if (!(pte & _PAGE_VALID)) {

It is not the same thing, but is this equivalent to if
(pmd_none(pmdp_get(pmd))) at this point?


> -			void *block = vmemmap_alloc_block(PMD_SIZE, node);
> +int __meminit vmemmap_check_pmd(pmd_t *pmdp, int node,
> +				unsigned long addr, unsigned long next)
> +{
> +	int large = pmd_leaf(*pmdp);
>  
> -			if (!block)
> -				return -ENOMEM;
> +	if (large)
> +		vmemmap_verify((pte_t *)pmdp, node, addr, next);
>  
> -			pmd_val(*pmd) = pte_base | __pa(block);
> -		}
> -	}
> +	return large;
> +}
>  
> -	return 0;
> +int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
> +			       int node, struct vmem_altmap *altmap)
> +{
> +	return vmemmap_populate_hugepages(vstart, vend, node, altmap);
>  }
>  #endif /* CONFIG_SPARSEMEM_VMEMMAP */
>  


This change introduces using vmemmap_alloc_block_buf() instead of
vmemmap_alloc_block() seems to introduce two new behaviours that was not
in use for sparc64 before:

1) Using altmap_alloc_block_buf() for a non-null altmap, that was not
   used before. Also the fallback to vmemmap_populate_basepages() passes
   on altmap.

2) Trying sparse_buffer_alloc() before vmemmap_alloc_block(), which was
   not done before.

Neither the commit message nor the cover letter touches upon this. Could
you elaborate here?


Given all the (at least seeming) functional changes could you share how
you tested this change?

Cheers,
Andreas

Re: [PATCH v5 1/2] sparc: Use vmemmap_populate_hugepages for vmemmap_populate

[Andrew Morton]

On Mon, 26 Jan 2026 15:50:34 +0100 Andreas Larsson <andreas@gaisler.com> wrote:

> On 2026-01-11 08:44, chengkaitao wrote:
> > From: Chengkaitao <chengkaitao@kylinos.cn>
> > 
> > 1. In the SPARC architecture, reimplemented vmemmap_populate using
> > vmemmap_populate_hugepages.
> > 2. Allow the SPARC arch to fallback to vmemmap_populate_basepages(),
> > when vmemmap_alloc_block returns NULL.
> 
> This patch seems to potentially make more functional changes than what
> the descriptions gives impression of.
> 
> Given the amount of changes this seems to introduce, more on that below,
> I'd like to see more description on the changes and why they can be done
> than this.
> 
> Nit: use active language, "reimplement", not "reimplemented".

Thanks, I'll drop the v5 version of this patchset.

Re: [PATCH v5 1/2] sparc: Use vmemmap_populate_hugepages for vmemmap_populate

[Chengkaitao]

On Mon, Jan 26, 2026 at 10:50 PM Andreas Larsson <andreas@gaisler.com> wrote:
>
> On 2026-01-11 08:44, chengkaitao wrote:
> > From: Chengkaitao <chengkaitao@kylinos.cn>
> >
> > 1. In the SPARC architecture, reimplemented vmemmap_populate using
> > vmemmap_populate_hugepages.
> > 2. Allow the SPARC arch to fallback to vmemmap_populate_basepages(),
> > when vmemmap_alloc_block returns NULL.
>
> This patch seems to potentially make more functional changes than what
> the descriptions gives impression of.
>
> Given the amount of changes this seems to introduce, more on that below,
> I'd like to see more description on the changes and why they can be done
> than this.
>
> Nit: use active language, "reimplement", not "reimplemented".
>
>
> > Signed-off-by: Chengkaitao <chengkaitao@kylinos.cn>
> > Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > ---
> >  arch/sparc/mm/init_64.c | 47 ++++++++++++++---------------------------
> >  1 file changed, 16 insertions(+), 31 deletions(-)
> >
> > diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
> > index df9f7c444c39..858eaa6615ea 100644
> > --- a/arch/sparc/mm/init_64.c
> > +++ b/arch/sparc/mm/init_64.c
> > @@ -2581,8 +2581,8 @@ unsigned long _PAGE_CACHE __read_mostly;
> >  EXPORT_SYMBOL(_PAGE_CACHE);
> >
> >  #ifdef CONFIG_SPARSEMEM_VMEMMAP
> > -int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
> > -                            int node, struct vmem_altmap *altmap)
> > +void __meminit vmemmap_set_pmd(pmd_t *pmd, void *p, int node,
> > +                            unsigned long addr, unsigned long next)
> >  {
> >       unsigned long pte_base;
> >
> > @@ -2595,39 +2595,24 @@ int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
> >
> >       pte_base |= _PAGE_PMD_HUGE;
> >
> > -     vstart = vstart & PMD_MASK;
> > -     vend = ALIGN(vend, PMD_SIZE);
>
> It seems that this patch removes alignment of both start and end. Is
> this a functional change in practice or are these always aligned for
> some other reason?
>
Whether vstart and vend are aligned with PMD_SIZE doesn't seem to
affect the behavior pattern or output of vmemmap_populate_hugepages.
The vmemmap_populate_hugepages function performs necessary alignment
processing internally, such as pmd_addr_end and pmd/pte_index?

> > -     for (; vstart < vend; vstart += PMD_SIZE) {
> > -             pgd_t *pgd = vmemmap_pgd_populate(vstart, node);
> > -             unsigned long pte;
> > -             p4d_t *p4d;
> > -             pud_t *pud;
> > -             pmd_t *pmd;
> > -
> > -             if (!pgd)
> > -                     return -ENOMEM;
> > -
> > -             p4d = vmemmap_p4d_populate(pgd, vstart, node);
> > -             if (!p4d)
> > -                     return -ENOMEM;
> > -
> > -             pud = vmemmap_pud_populate(p4d, vstart, node);
> > -             if (!pud)
> > -                     return -ENOMEM;
> > +     pmd_val(*pmd) = pte_base | __pa(p);
> > +}
> >
> > -             pmd = pmd_offset(pud, vstart);
> > -             pte = pmd_val(*pmd);
> > -             if (!(pte & _PAGE_VALID)) {
>
> It is not the same thing, but is this equivalent to if
> (pmd_none(pmdp_get(pmd))) at this point?
>
For PMD entries, there shouldn't be cases where pmd_none and
_PAGE_VALID exhibit inconsistent behavior. I've observed that
pmd_none is widely used in the SPARC architecture.

> > -                     void *block = vmemmap_alloc_block(PMD_SIZE, node);
> > +int __meminit vmemmap_check_pmd(pmd_t *pmdp, int node,
> > +                             unsigned long addr, unsigned long next)
> > +{
> > +     int large = pmd_leaf(*pmdp);
> >
> > -                     if (!block)
> > -                             return -ENOMEM;
> > +     if (large)
> > +             vmemmap_verify((pte_t *)pmdp, node, addr, next);
> >
> > -                     pmd_val(*pmd) = pte_base | __pa(block);
> > -             }
> > -     }
> > +     return large;
> > +}
> >
> > -     return 0;
> > +int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
> > +                            int node, struct vmem_altmap *altmap)
> > +{
> > +     return vmemmap_populate_hugepages(vstart, vend, node, altmap);
> >  }
> >  #endif /* CONFIG_SPARSEMEM_VMEMMAP */
> >
>
>
> This change introduces using vmemmap_alloc_block_buf() instead of
> vmemmap_alloc_block() seems to introduce two new behaviours that was not
> in use for sparc64 before:
>
> 1) Using altmap_alloc_block_buf() for a non-null altmap, that was not
>    used before. Also the fallback to vmemmap_populate_basepages() passes
>    on altmap.

If altmap validation isn't required, I can retain the original code
logic by setting altmap to NULL.

> 2) Trying sparse_buffer_alloc() before vmemmap_alloc_block(), which was
>    not done before.

In SPARC, sparse_init() is called to initialize the sparsemap_buf.
If the SPARC architecture doesn't support using sparse_buffer_alloc,
we can remove the sparse_init() call path.

> Neither the commit message nor the cover letter touches upon this. Could
> you elaborate here?
>
> Given all the (at least seeming) functional changes could you share how
> you tested this change?

My original intention was to help architectures adopt more generic
kernel APIs to reduce maintenance costs. However, due to my lack of
physical SPARC devices, I couldn't perform comprehensive testing,
I've only verified compilation correctness based on code analysis.
I sincerely apologize for this limitation. If you have access to
physical SPARC hardware, could you kindly help with testing?

-- 
Cheers,
Chengkaitao

Re: [PATCH v5 1/2] sparc: Use vmemmap_populate_hugepages for vmemmap_populate

[Andreas Larsson]

On 2026-01-28 04:25, Chengkaitao wrote:
> On Mon, Jan 26, 2026 at 10:50 PM Andreas Larsson <andreas@gaisler.com> wrote:
>>
>> On 2026-01-11 08:44, chengkaitao wrote:
>>> From: Chengkaitao <chengkaitao@kylinos.cn>
>>>
>>> 1. In the SPARC architecture, reimplemented vmemmap_populate using
>>> vmemmap_populate_hugepages.
>>> 2. Allow the SPARC arch to fallback to vmemmap_populate_basepages(),
>>> when vmemmap_alloc_block returns NULL.
>>
>> This patch seems to potentially make more functional changes than what
>> the descriptions gives impression of.
>>
>> Given the amount of changes this seems to introduce, more on that below,
>> I'd like to see more description on the changes and why they can be done
>> than this.
>>
>> Nit: use active language, "reimplement", not "reimplemented".
>>
>>
>>> Signed-off-by: Chengkaitao <chengkaitao@kylinos.cn>
>>> Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>>> ---
>>>  arch/sparc/mm/init_64.c | 47 ++++++++++++++---------------------------
>>>  1 file changed, 16 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
>>> index df9f7c444c39..858eaa6615ea 100644
>>> --- a/arch/sparc/mm/init_64.c
>>> +++ b/arch/sparc/mm/init_64.c
>>> @@ -2581,8 +2581,8 @@ unsigned long _PAGE_CACHE __read_mostly;
>>>  EXPORT_SYMBOL(_PAGE_CACHE);
>>>
>>>  #ifdef CONFIG_SPARSEMEM_VMEMMAP
>>> -int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
>>> -                            int node, struct vmem_altmap *altmap)
>>> +void __meminit vmemmap_set_pmd(pmd_t *pmd, void *p, int node,
>>> +                            unsigned long addr, unsigned long next)
>>>  {
>>>       unsigned long pte_base;
>>>
>>> @@ -2595,39 +2595,24 @@ int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
>>>
>>>       pte_base |= _PAGE_PMD_HUGE;
>>>
>>> -     vstart = vstart & PMD_MASK;
>>> -     vend = ALIGN(vend, PMD_SIZE);
>>
>> It seems that this patch removes alignment of both start and end. Is
>> this a functional change in practice or are these always aligned for
>> some other reason?
>>
> Whether vstart and vend are aligned with PMD_SIZE doesn't seem to
> affect the behavior pattern or output of vmemmap_populate_hugepages.
> The vmemmap_populate_hugepages function performs necessary alignment
> processing internally, such as pmd_addr_end and pmd/pte_index?
> 
>>> -     for (; vstart < vend; vstart += PMD_SIZE) {
>>> -             pgd_t *pgd = vmemmap_pgd_populate(vstart, node);
>>> -             unsigned long pte;
>>> -             p4d_t *p4d;
>>> -             pud_t *pud;
>>> -             pmd_t *pmd;
>>> -
>>> -             if (!pgd)
>>> -                     return -ENOMEM;
>>> -
>>> -             p4d = vmemmap_p4d_populate(pgd, vstart, node);
>>> -             if (!p4d)
>>> -                     return -ENOMEM;
>>> -
>>> -             pud = vmemmap_pud_populate(p4d, vstart, node);
>>> -             if (!pud)
>>> -                     return -ENOMEM;
>>> +     pmd_val(*pmd) = pte_base | __pa(p);
>>> +}
>>>
>>> -             pmd = pmd_offset(pud, vstart);
>>> -             pte = pmd_val(*pmd);
>>> -             if (!(pte & _PAGE_VALID)) {
>>
>> It is not the same thing, but is this equivalent to if
>> (pmd_none(pmdp_get(pmd))) at this point?
>>
> For PMD entries, there shouldn't be cases where pmd_none and
> _PAGE_VALID exhibit inconsistent behavior. I've observed that
> pmd_none is widely used in the SPARC architecture.
> 
>>> -                     void *block = vmemmap_alloc_block(PMD_SIZE, node);
>>> +int __meminit vmemmap_check_pmd(pmd_t *pmdp, int node,
>>> +                             unsigned long addr, unsigned long next)
>>> +{
>>> +     int large = pmd_leaf(*pmdp);
>>>
>>> -                     if (!block)
>>> -                             return -ENOMEM;
>>> +     if (large)
>>> +             vmemmap_verify((pte_t *)pmdp, node, addr, next);
>>>
>>> -                     pmd_val(*pmd) = pte_base | __pa(block);
>>> -             }
>>> -     }
>>> +     return large;
>>> +}
>>>
>>> -     return 0;
>>> +int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
>>> +                            int node, struct vmem_altmap *altmap)
>>> +{
>>> +     return vmemmap_populate_hugepages(vstart, vend, node, altmap);
>>>  }
>>>  #endif /* CONFIG_SPARSEMEM_VMEMMAP */
>>>
>>
>>
>> This change introduces using vmemmap_alloc_block_buf() instead of
>> vmemmap_alloc_block() seems to introduce two new behaviours that was not
>> in use for sparc64 before:
>>
>> 1) Using altmap_alloc_block_buf() for a non-null altmap, that was not
>>    used before. Also the fallback to vmemmap_populate_basepages() passes
>>    on altmap.
> 
> If altmap validation isn't required, I can retain the original code
> logic by setting altmap to NULL.
> 
>> 2) Trying sparse_buffer_alloc() before vmemmap_alloc_block(), which was
>>    not done before.
> 
> In SPARC, sparse_init() is called to initialize the sparsemap_buf.
> If the SPARC architecture doesn't support using sparse_buffer_alloc,
> we can remove the sparse_init() call path.

Thank you for the details.

>> Neither the commit message nor the cover letter touches upon this. Could
>> you elaborate here?
>>
>> Given all the (at least seeming) functional changes could you share how
>> you tested this change?
> 
> My original intention was to help architectures adopt more generic
> kernel APIs to reduce maintenance costs. However, due to my lack of
> physical SPARC devices, I couldn't perform comprehensive testing,
> I've only verified compilation correctness based on code analysis.
> I sincerely apologize for this limitation. If you have access to
> physical SPARC hardware, could you kindly help with testing?

Yes, I have tested v6 on SPARC hardware.

Cheers,
Andreas

Re: [PATCH v5 1/2] sparc: Use vmemmap_populate_hugepages for vmemmap_populate

[Chengkaitao]

On Mon, Jan 26, 2026 at 10:50 PM Andreas Larsson <andreas@gaisler.com> wrote:
>
> On 2026-01-11 08:44, chengkaitao wrote:
> > From: Chengkaitao <chengkaitao@kylinos.cn>
> >
> > 1. In the SPARC architecture, reimplemented vmemmap_populate using
> > vmemmap_populate_hugepages.
> > 2. Allow the SPARC arch to fallback to vmemmap_populate_basepages(),
> > when vmemmap_alloc_block returns NULL.
>
> This patch seems to potentially make more functional changes than what
> the descriptions gives impression of.
>
> Given the amount of changes this seems to introduce, more on that below,
> I'd like to see more description on the changes and why they can be done
> than this.

Hi Andreas Larsson,
Regarding my previous responses to the questions raised in this
email thread, I have not yet received your reply. I would like
to provide additional clarifications on these points here:

> Nit: use active language, "reimplement", not "reimplemented".

This issue has been fixed in version v6.
https://lore.kernel.org/all/20260201063532.44807-2-pilgrimtao@gmail.com/

> > Signed-off-by: Chengkaitao <chengkaitao@kylinos.cn>
> > Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > ---
> >  arch/sparc/mm/init_64.c | 47 ++++++++++++++---------------------------
> >  1 file changed, 16 insertions(+), 31 deletions(-)
> >
> > diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
> > index df9f7c444c39..858eaa6615ea 100644
> > --- a/arch/sparc/mm/init_64.c
> > +++ b/arch/sparc/mm/init_64.c
> > @@ -2581,8 +2581,8 @@ unsigned long _PAGE_CACHE __read_mostly;
> >  EXPORT_SYMBOL(_PAGE_CACHE);
> >
> >  #ifdef CONFIG_SPARSEMEM_VMEMMAP
> > -int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
> > -                            int node, struct vmem_altmap *altmap)
> > +void __meminit vmemmap_set_pmd(pmd_t *pmd, void *p, int node,
> > +                            unsigned long addr, unsigned long next)
> >  {
> >       unsigned long pte_base;
> >
> > @@ -2595,39 +2595,24 @@ int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
> >
> >       pte_base |= _PAGE_PMD_HUGE;
> >
> > -     vstart = vstart & PMD_MASK;
> > -     vend = ALIGN(vend, PMD_SIZE);
>
> It seems that this patch removes alignment of both start and end. Is
> this a functional change in practice or are these always aligned for
> some other reason?

In the implementation of vmemmap_populate_hugepages, the start
value remains unaligned when entering the for-loop for the first
time. However, there is no need to apply the "start & PMD_MASK"
operation in the vmemmap_*_populate series of functions. In
iterations after the first one, pmd_addr_end() will align the
start value; since start is already aligned, there is no longer
a need to align end.

In fact, in the original code, the presence of
"vstart = vstart & PMD_MASK" made "vend = ALIGN(vend, PMD_SIZE)"
redundant as well.

> > -     for (; vstart < vend; vstart += PMD_SIZE) {
> > -             pgd_t *pgd = vmemmap_pgd_populate(vstart, node);
> > -             unsigned long pte;
> > -             p4d_t *p4d;
> > -             pud_t *pud;
> > -             pmd_t *pmd;
> > -
> > -             if (!pgd)
> > -                     return -ENOMEM;
> > -
> > -             p4d = vmemmap_p4d_populate(pgd, vstart, node);
> > -             if (!p4d)
> > -                     return -ENOMEM;
> > -
> > -             pud = vmemmap_pud_populate(p4d, vstart, node);
> > -             if (!pud)
> > -                     return -ENOMEM;
> > +     pmd_val(*pmd) = pte_base | __pa(p);
> > +}
> >
> > -             pmd = pmd_offset(pud, vstart);
> > -             pte = pmd_val(*pmd);
> > -             if (!(pte & _PAGE_VALID)) {
>
> It is not the same thing, but is this equivalent to if
> (pmd_none(pmdp_get(pmd))) at this point?

Since the vmemmap_*_populate functions all use vmemmap_alloc_block_zero
when allocating page tables, (pmd_none(pmdp_get(pmd))) can be used as a
replacement at this point.

> > -                     void *block = vmemmap_alloc_block(PMD_SIZE, node);
> > +int __meminit vmemmap_check_pmd(pmd_t *pmdp, int node,
> > +                             unsigned long addr, unsigned long next)
> > +{
> > +     int large = pmd_leaf(*pmdp);
> >
> > -                     if (!block)
> > -                             return -ENOMEM;
> > +     if (large)
> > +             vmemmap_verify((pte_t *)pmdp, node, addr, next);
> >
> > -                     pmd_val(*pmd) = pte_base | __pa(block);
> > -             }
> > -     }
> > +     return large;
> > +}
> >
> > -     return 0;
> > +int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
> > +                            int node, struct vmem_altmap *altmap)
> > +{
> > +     return vmemmap_populate_hugepages(vstart, vend, node, altmap);
> >  }
> >  #endif /* CONFIG_SPARSEMEM_VMEMMAP */
> >
>
>
> This change introduces using vmemmap_alloc_block_buf() instead of
> vmemmap_alloc_block() seems to introduce two new behaviours that was not
> in use for sparc64 before:
>
> 1) Using altmap_alloc_block_buf() for a non-null altmap, that was not
>    used before. Also the fallback to vmemmap_populate_basepages() passes
>    on altmap.

This issue has been fixed in version v6.
https://lore.kernel.org/all/20260201063532.44807-2-pilgrimtao@gmail.com/

> 2) Trying sparse_buffer_alloc() before vmemmap_alloc_block(), which was
>    not done before.

The sparse_buffer_alloc function here is usable.
Both sparse_buffer_alloc() and vmemmap_alloc_block() essentially
call memmap_alloc(), the only difference is that sparse_buffer_alloc
performs the allocation in advance.

> Neither the commit message nor the cover letter touches upon this. Could
> you elaborate here?
>
> Given all the (at least seeming) functional changes could you share how
> you tested this change?
>
> Cheers,
> Andreas
>

Is there still a possibility for this patch to be merged into the
mainline? Should I continue modifying and iterating on it? Could
you help with testing it?

Regardless of your answer, I look forward to your reply.

-- 
Yours,
Chengkaitao

[PATCH v5 2/2] mm: Convert vmemmap_p?d_populate() to static functions

[chengkaitao]

From: Chengkaitao <chengkaitao@kylinos.cn>

Since the vmemmap_p?d_populate functions are unused outside the mm
subsystem, we can remove their external declarations and convert
them to static functions.

Signed-off-by: Chengkaitao <chengkaitao@kylinos.cn>
Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 include/linux/mm.h  |  7 -------
 mm/sparse-vmemmap.c | 10 +++++-----
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6f959d8ca4b4..322fe72b6ebd 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4234,13 +4234,6 @@ unsigned long section_map_size(void);
 struct page * __populate_section_memmap(unsigned long pfn,
 		unsigned long nr_pages, int nid, struct vmem_altmap *altmap,
 		struct dev_pagemap *pgmap);
-pgd_t *vmemmap_pgd_populate(unsigned long addr, int node);
-p4d_t *vmemmap_p4d_populate(pgd_t *pgd, unsigned long addr, int node);
-pud_t *vmemmap_pud_populate(p4d_t *p4d, unsigned long addr, int node);
-pmd_t *vmemmap_pmd_populate(pud_t *pud, unsigned long addr, int node);
-pte_t *vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, int node,
-			    struct vmem_altmap *altmap, unsigned long ptpfn,
-			    unsigned long flags);
 void *vmemmap_alloc_block(unsigned long size, int node);
 struct vmem_altmap;
 void *vmemmap_alloc_block_buf(unsigned long size, int node,
diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
index 37522d6cb398..67f9a08c2376 100644
--- a/mm/sparse-vmemmap.c
+++ b/mm/sparse-vmemmap.c
@@ -151,7 +151,7 @@ void __meminit vmemmap_verify(pte_t *pte, int node,
 			start, end - 1);
 }
 
-pte_t * __meminit vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, int node,
+static pte_t * __meminit vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, int node,
 				       struct vmem_altmap *altmap,
 				       unsigned long ptpfn, unsigned long flags)
 {
@@ -195,7 +195,7 @@ static void * __meminit vmemmap_alloc_block_zero(unsigned long size, int node)
 	return p;
 }
 
-pmd_t * __meminit vmemmap_pmd_populate(pud_t *pud, unsigned long addr, int node)
+static pmd_t * __meminit vmemmap_pmd_populate(pud_t *pud, unsigned long addr, int node)
 {
 	pmd_t *pmd = pmd_offset(pud, addr);
 	if (pmd_none(*pmd)) {
@@ -208,7 +208,7 @@ pmd_t * __meminit vmemmap_pmd_populate(pud_t *pud, unsigned long addr, int node)
 	return pmd;
 }
 
-pud_t * __meminit vmemmap_pud_populate(p4d_t *p4d, unsigned long addr, int node)
+static pud_t * __meminit vmemmap_pud_populate(p4d_t *p4d, unsigned long addr, int node)
 {
 	pud_t *pud = pud_offset(p4d, addr);
 	if (pud_none(*pud)) {
@@ -221,7 +221,7 @@ pud_t * __meminit vmemmap_pud_populate(p4d_t *p4d, unsigned long addr, int node)
 	return pud;
 }
 
-p4d_t * __meminit vmemmap_p4d_populate(pgd_t *pgd, unsigned long addr, int node)
+static p4d_t * __meminit vmemmap_p4d_populate(pgd_t *pgd, unsigned long addr, int node)
 {
 	p4d_t *p4d = p4d_offset(pgd, addr);
 	if (p4d_none(*p4d)) {
@@ -234,7 +234,7 @@ p4d_t * __meminit vmemmap_p4d_populate(pgd_t *pgd, unsigned long addr, int node)
 	return p4d;
 }
 
-pgd_t * __meminit vmemmap_pgd_populate(unsigned long addr, int node)
+static pgd_t * __meminit vmemmap_pgd_populate(unsigned long addr, int node)
 {
 	pgd_t *pgd = pgd_offset_k(addr);
 	if (pgd_none(*pgd)) {
-- 
2.50.1 (Apple Git-155)