| Message ID | 20250407165435.2567898-1-fvdl@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/cma: report base address of single range correctly | expand |
On Mon, 7 Apr 2025 at 18:54, Frank van der Linden <fvdl@google.com> wrote: > The cma_declare_contiguous_nid code was refactored by > commit c009da4258f9 ("mm, cma: support multiple contiguous > ranges, if requested"), so that it could use an internal > function to attempt a single range area first, and then > try a multi-range one. > > However, that meant that the actual base address used for > the !fixed case (base == 0) wasn't available one level up > to be printed in the informational message, and it would > always end up printing a base address of 0 in the boot > message. > > Make the internal function take a phys_addr_t pointer to > the base address, so that the value is available to the > caller. > > Fixes: c009da4258f9 ("mm, cma: support multiple contiguous ranges, if requested") > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > Closes: https://lore.kernel.org/linux-mm/CAMuHMdVWviQ7O9yBFE3f=ev0eVb1CnsQvR6SKtEROBbM6z7g3w@mail.gmail.com/ > Signed-off-by: Frank van der Linden <fvdl@google.com> Thanks, this fixes the issue! Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/mm/cma.c > +++ b/mm/cma.c > @@ -724,8 +724,10 @@ static int __init __cma_declare_contiguous_nid(phys_addr_t base, > ret = cma_init_reserved_mem(base, size, order_per_bit, name, res_cma); > if (ret) > memblock_phys_free(base, size); > - > - (*res_cma)->nid = nid; > + else { > + (*res_cma)->nid = nid; > + *basep = base; > + } Please add curly braces to the other branch, too. Or perhaps restructure as: if (ret) { memblock_phys_free(base, size); return ret; } (*res_cma)->nid = nid; *basep = base; return 0; > > return ret; > } Gr{oetje,eeting}s, Geert
On Tue, Apr 8, 2025 at 1:06 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Mon, 7 Apr 2025 at 18:54, Frank van der Linden <fvdl@google.com> wrote: > > The cma_declare_contiguous_nid code was refactored by > > commit c009da4258f9 ("mm, cma: support multiple contiguous > > ranges, if requested"), so that it could use an internal > > function to attempt a single range area first, and then > > try a multi-range one. > > > > However, that meant that the actual base address used for > > the !fixed case (base == 0) wasn't available one level up > > to be printed in the informational message, and it would > > always end up printing a base address of 0 in the boot > > message. > > > > Make the internal function take a phys_addr_t pointer to > > the base address, so that the value is available to the > > caller. > > > > Fixes: c009da4258f9 ("mm, cma: support multiple contiguous ranges, if requested") > > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > > Closes: https://lore.kernel.org/linux-mm/CAMuHMdVWviQ7O9yBFE3f=ev0eVb1CnsQvR6SKtEROBbM6z7g3w@mail.gmail.com/ > > Signed-off-by: Frank van der Linden <fvdl@google.com> > > Thanks, this fixes the issue! > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- a/mm/cma.c > > +++ b/mm/cma.c > > @@ -724,8 +724,10 @@ static int __init __cma_declare_contiguous_nid(phys_addr_t base, > > ret = cma_init_reserved_mem(base, size, order_per_bit, name, res_cma); > > if (ret) > > memblock_phys_free(base, size); > > - > > - (*res_cma)->nid = nid; > > + else { > > + (*res_cma)->nid = nid; > > + *basep = base; > > + } > > Please add curly braces to the other branch, too. > > Or perhaps restructure as: > > if (ret) { > memblock_phys_free(base, size); > return ret; > } > > (*res_cma)->nid = nid; > *basep = base; > > return 0; > > > > > return ret; > > } > > Gr{oetje,eeting}s, > > Geert > Thanks for testing - I sent a v2 with the style nit fixed. Thanks again, - Frank
diff --git a/mm/cma.c b/mm/cma.c index b06d5fe73399..46aceeae4c50 100644 --- a/mm/cma.c +++ b/mm/cma.c @@ -35,7 +35,7 @@ struct cma cma_areas[MAX_CMA_AREAS]; unsigned int cma_area_count; -static int __init __cma_declare_contiguous_nid(phys_addr_t base, +static int __init __cma_declare_contiguous_nid(phys_addr_t *basep, phys_addr_t size, phys_addr_t limit, phys_addr_t alignment, unsigned int order_per_bit, bool fixed, const char *name, struct cma **res_cma, @@ -370,7 +370,7 @@ int __init cma_declare_contiguous_multi(phys_addr_t total_size, phys_addr_t align, unsigned int order_per_bit, const char *name, struct cma **res_cma, int nid) { - phys_addr_t start, end; + phys_addr_t start = 0, end; phys_addr_t size, sizesum, sizeleft; struct cma_init_memrange *mrp, *mlp, *failed; struct cma_memrange *cmrp; @@ -384,7 +384,7 @@ int __init cma_declare_contiguous_multi(phys_addr_t total_size, /* * First, try it the normal way, producing just one range. */ - ret = __cma_declare_contiguous_nid(0, total_size, 0, align, + ret = __cma_declare_contiguous_nid(&start, total_size, 0, align, order_per_bit, false, name, res_cma, nid); if (ret != -ENOMEM) goto out; @@ -580,7 +580,7 @@ int __init cma_declare_contiguous_nid(phys_addr_t base, { int ret; - ret = __cma_declare_contiguous_nid(base, size, limit, alignment, + ret = __cma_declare_contiguous_nid(&base, size, limit, alignment, order_per_bit, fixed, name, res_cma, nid); if (ret != 0) pr_err("Failed to reserve %ld MiB\n", @@ -592,14 +592,14 @@ int __init cma_declare_contiguous_nid(phys_addr_t base, return ret; } -static int __init __cma_declare_contiguous_nid(phys_addr_t base, +static int __init __cma_declare_contiguous_nid(phys_addr_t *basep, phys_addr_t size, phys_addr_t limit, phys_addr_t alignment, unsigned int order_per_bit, bool fixed, const char *name, struct cma **res_cma, int nid) { phys_addr_t memblock_end = memblock_end_of_DRAM(); - phys_addr_t highmem_start; + phys_addr_t highmem_start, base = *basep; int ret; /* @@ -724,8 +724,10 @@ static int __init __cma_declare_contiguous_nid(phys_addr_t base, ret = cma_init_reserved_mem(base, size, order_per_bit, name, res_cma); if (ret) memblock_phys_free(base, size); - - (*res_cma)->nid = nid; + else { + (*res_cma)->nid = nid; + *basep = base; + } return ret; }
The cma_declare_contiguous_nid code was refactored by commit c009da4258f9 ("mm, cma: support multiple contiguous ranges, if requested"), so that it could use an internal function to attempt a single range area first, and then try a multi-range one. However, that meant that the actual base address used for the !fixed case (base == 0) wasn't available one level up to be printed in the informational message, and it would always end up printing a base address of 0 in the boot message. Make the internal function take a phys_addr_t pointer to the base address, so that the value is available to the caller. Fixes: c009da4258f9 ("mm, cma: support multiple contiguous ranges, if requested") Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Closes: https://lore.kernel.org/linux-mm/CAMuHMdVWviQ7O9yBFE3f=ev0eVb1CnsQvR6SKtEROBbM6z7g3w@mail.gmail.com/ Signed-off-by: Frank van der Linden <fvdl@google.com> --- mm/cma.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)