[PATCH] mm/cma: report base address of single range correctly

[PATCH] mm/cma: report base address of single range correctly

[Frank van der Linden]

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(-)

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;
 }
-- 
2.49.0.504.g3bcea36a83-goog

Re: [PATCH] mm/cma: report base address of single range correctly

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] mm/cma: report base address of single range correctly

[Frank van der Linden]

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