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

Guilherme G. Piccoli posted 2 patches 1 month ago
[PATCH V2 1/2] mm/memblock: Print out errors on reserve_mem parser
Posted by Guilherme G. Piccoli 1 month ago
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.

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


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;
+
 }
 __setup("reserve_mem=", reserve_mem);
 
-- 
2.50.1
Re: [PATCH V2 1/2] mm/memblock: Print out errors on reserve_mem parser
Posted by SeongJae Park 1 month ago
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
Re: [PATCH V2 1/2] mm/memblock: Print out errors on reserve_mem parser
Posted by Guilherme G. Piccoli 3 weeks, 6 days ago
On 04/03/2026 22:14, SeongJae Park wrote:
> 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>
> 

Hi SJ! thanks a lot for the review, I'll resubmit with your tag and
fixing the unnecessary blank line.
Cheers,


Guilherme