Re: [PATCH v3 06/14] mm/sparse-vmemmap: refactor core of vmemmap_populate_basepages() to helper
From: Dan Williams <hidden>
Date: 2021-07-28 06:05:13
Also in:
linux-mm, nvdimm
On Wed, Jul 14, 2021 at 12:36 PM Joao Martins [off-list ref] wrote:
quoted hunk ↗ jump to hunk
In preparation for describing a memmap with compound pages, move the actual pte population logic into a separate function vmemmap_populate_address() and have vmemmap_populate_basepages() walk through all base pages it needs to populate. Signed-off-by: Joao Martins <redacted> --- mm/sparse-vmemmap.c | 44 ++++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-)diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c index 80d3ba30d345..76f4158f6301 100644 --- a/mm/sparse-vmemmap.c +++ b/mm/sparse-vmemmap.c@@ -570,33 +570,41 @@ pgd_t * __meminit vmemmap_pgd_populate(unsigned long addr, int node) return pgd; } -int __meminit vmemmap_populate_basepages(unsigned long start, unsigned long end, - int node, struct vmem_altmap *altmap) +static int __meminit vmemmap_populate_address(unsigned long addr, int node, + struct vmem_altmap *altmap) { - unsigned long addr = start; pgd_t *pgd; p4d_t *p4d; pud_t *pud; pmd_t *pmd; pte_t *pte; + pgd = vmemmap_pgd_populate(addr, node); + if (!pgd) + return -ENOMEM; + p4d = vmemmap_p4d_populate(pgd, addr, node); + if (!p4d) + return -ENOMEM; + pud = vmemmap_pud_populate(p4d, addr, node); + if (!pud) + return -ENOMEM; + pmd = vmemmap_pmd_populate(pud, addr, node); + if (!pmd) + return -ENOMEM; + pte = vmemmap_pte_populate(pmd, addr, node, altmap); + if (!pte) + return -ENOMEM; + vmemmap_verify(pte, node, addr, addr + PAGE_SIZE);
Missing a return here: mm/sparse-vmemmap.c:598:1: error: control reaches end of non-void function [-Werror=return-type] Yes, it's fixed up in a later patch, but might as well not leave the bisect breakage lying around, and the kbuild robot would gripe about this eventually as well.
+}
+
+int __meminit vmemmap_populate_basepages(unsigned long start, unsigned long end,
+ int node, struct vmem_altmap *altmap)
+{
+ unsigned long addr = start;
+
for (; addr < end; addr += PAGE_SIZE) {
- pgd = vmemmap_pgd_populate(addr, node);
- if (!pgd)
- return -ENOMEM;
- p4d = vmemmap_p4d_populate(pgd, addr, node);
- if (!p4d)
- return -ENOMEM;
- pud = vmemmap_pud_populate(p4d, addr, node);
- if (!pud)
- return -ENOMEM;
- pmd = vmemmap_pmd_populate(pud, addr, node);
- if (!pmd)
- return -ENOMEM;
- pte = vmemmap_pte_populate(pmd, addr, node, altmap);
- if (!pte)
+ if (vmemmap_populate_address(addr, node, altmap))
return -ENOMEM;
I'd prefer:
rc = vmemmap_populate_address(addr, node, altmap);
if (rc)
return rc;
...in case future refactoring adds different error codes to pass up.
- vmemmap_verify(pte, node, addr, addr + PAGE_SIZE);
}
return 0;
--
2.17.1