| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAWJmAYmMtdE7nSJ8BWSnt2DObipqGi_KdXdMJrHfwc0ANBSPQ@mail.gmail.com>
Date: Wed, 28 Jan 2026 11:25:46 +0800
From: Chengkaitao <pilgrimtao@...il.com>
To: Andreas Larsson <andreas@...sler.com>
Cc: davem@...emloft.net, akpm@...ux-foundation.org, david@...nel.org,
lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com, vbabka@...e.cz,
rppt@...nel.org, surenb@...gle.com, mhocko@...e.com, kevin.brodsky@....com,
dave.hansen@...ux.intel.com, ziy@...dia.com, chengkaitao@...inos.cn,
willy@...radead.org, zhengqi.arch@...edance.com, sparclinux@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v5 1/2] sparc: Use vmemmap_populate_hugepages for vmemmap_populate
On Mon, Jan 26, 2026 at 10:50 PM Andreas Larsson <andreas@...sler.com> wrote:
>
> On 2026-01-11 08:44, chengkaitao wrote:
> > From: Chengkaitao <chengkaitao@...inos.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@...inos.cn>
> > Acked-by: Mike Rapoport (Microsoft) <rppt@...nel.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
Powered by blists - more mailing lists