Re: [PATCH V2 1/2] mm/memblock: Print out errors on reserve_mem parser

[SeongJae Park <sj@kernel.org>]

On Wed,  4 Mar 2026 17:14:10 -0300 "Guilherme G. Piccoli" <gpiccoli@igalia.com> wrote:

> The parsing of kernel parameter "reserve_mem=" is subject to
> multiple failures, like duplicate naming, malformed expression
> or even lack of available memory. Right now, all of these fail
> silently. Let's add some messages so the kernel log can provide
> useful information in case of failures.

Makes sense to me.

> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>

Reviewed-by: SeongJae Park <sj@kernel.org>

> ---
> 
> 
> V2: no changes.
> 
>  mm/memblock.c | 28 +++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index b3ddfdec7a80..2d2646f7a120 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -2642,23 +2642,25 @@ static int __init reserve_mem(char *p)
>  	int len;
>  
>  	if (!p)
> -		return -EINVAL;
> +		goto err_param;
>  
>  	/* Check if there's room for more reserved memory */
> -	if (reserved_mem_count >= RESERVE_MEM_MAX_ENTRIES)
> +	if (reserved_mem_count >= RESERVE_MEM_MAX_ENTRIES) {
> +		pr_err("reserve_mem: no more room for reserved memory\n");
>  		return -EBUSY;
> +	}
>  
>  	oldp = p;
>  	size = memparse(p, &p);
>  	if (!size || p == oldp)
> -		return -EINVAL;
> +		goto err_param;
>  
>  	if (*p != ':')
> -		return -EINVAL;
> +		goto err_param;
>  
>  	align = memparse(p+1, &p);
>  	if (*p != ':')
> -		return -EINVAL;
> +		goto err_param;
>  
>  	/*
>  	 * memblock_phys_alloc() doesn't like a zero size align,
> @@ -2672,7 +2674,7 @@ static int __init reserve_mem(char *p)
>  
>  	/* name needs to have length but not too big */
>  	if (!len || len >= RESERVE_MEM_NAME_SIZE)
> -		return -EINVAL;
> +		goto err_param;
>  
>  	/* Make sure that name has text */
>  	for (p = name; *p; p++) {
> @@ -2680,11 +2682,13 @@ static int __init reserve_mem(char *p)
>  			break;
>  	}
>  	if (!*p)
> -		return -EINVAL;
> +		goto err_param;
>  
>  	/* Make sure the name is not already used */
> -	if (reserve_mem_find_by_name(name, &start, &tmp))
> +	if (reserve_mem_find_by_name(name, &start, &tmp)) {
> +		pr_err("reserve_mem: name \"%s\" was already used\n", name);
>  		return -EBUSY;
> +	}
>  
>  	/* Pick previous allocations up from KHO if available */
>  	if (reserve_mem_kho_revive(name, size, align))
> @@ -2692,12 +2696,18 @@ static int __init reserve_mem(char *p)
>  
>  	/* TODO: Allocation must be outside of scratch region */
>  	start = memblock_phys_alloc(size, align);
> -	if (!start)
> +	if (!start) {
> +		pr_err("reserve_mem: memblock allocation failed\n");
>  		return -ENOMEM;
> +	}
>  
>  	reserved_mem_add(start, size, name);
>  
>  	return 1;
> +err_param:
> +	pr_err("reserve_mem: empty or malformed parameter\n");
> +	return -EINVAL;
> +

Nit.  Above blank line seems not needed.

>  }
>  __setup("reserve_mem=", reserve_mem);
>  
> -- 
> 2.50.1


Thanks,
SJ