The LoongArch mem= parameter parser was previously limited to the
mem=SIZE@START format. This was inconvenient for the common use case
of simply capping the total system memory, as it forced users to
manually specify a start address. It was also inconsistent with the
behavior on other architectures.

This patch enhances the parser in early_parse_mem() to also support the
more user-friendly mem=SIZE format. The implementation now checks for
the presence of the '@' symbol to determine the user's intent:

- If mem=SIZE is provided (no '@'), the kernel now calls
  memblock_enforce_memory_limit(). This trims memory from the top down
  to the specified size.
- If mem=SIZE@START is used, the original behavior is retained for
  backward compatibility. This allows for defining specific memory
  banks.

This change introduces an important usage rule reflected in the code's
comments: the mem=SIZE format should only be specified once on the
kernel command line. It acts as a single, global cap on total memory. In
contrast, the mem=SIZE@START format can be used multiple times to
define several distinct memory regions.

Signed-off-by: Ming Wang <wangming01@loongson.cn>
---
 arch/loongarch/kernel/setup.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
index b99fbb388fe0..af59ba180dc2 100644
--- a/arch/loongarch/kernel/setup.c
+++ b/arch/loongarch/kernel/setup.c
@@ -191,6 +191,16 @@ static int __init early_parse_mem(char *p)
 		return -EINVAL;
 	}
 
+	start = 0;
+	size = memparse(p, &p);
+	if (*p == '@')	/* Every mem=... should contain '@' */
+		start = memparse(p + 1, &p);
+	else {			/* Only one mem=... is allowed if no '@' */
+		usermem = 1;
+		memblock_enforce_memory_limit(size);
+		return 0;
+	}
+
 	/*
 	 * If a user specifies memory size, we
 	 * blow away any automatically generated
@@ -201,14 +211,6 @@ static int __init early_parse_mem(char *p)
 		memblock_remove(memblock_start_of_DRAM(),
 			memblock_end_of_DRAM() - memblock_start_of_DRAM());
 	}
-	start = 0;
-	size = memparse(p, &p);
-	if (*p == '@')
-		start = memparse(p + 1, &p);
-	else {
-		pr_err("Invalid format!\n");
-		return -EINVAL;
-	}
 
 	if (!IS_ENABLED(CONFIG_NUMA))
 		memblock_add(start, size);
-- 
2.43.0

在 7/1/25 5:04 PM, Ming Wang 写道:
> The LoongArch mem= parameter parser was previously limited to the
> mem=SIZE@START format. This was inconvenient for the common use case
> of simply capping the total system memory, as it forced users to
> manually specify a start address. It was also inconsistent with the
> behavior on other architectures.
> 
> This patch enhances the parser in early_parse_mem() to also support the
> more user-friendly mem=SIZE format. The implementation now checks for
> the presence of the '@' symbol to determine the user's intent:
> 
> - If mem=SIZE is provided (no '@'), the kernel now calls
>    memblock_enforce_memory_limit(). This trims memory from the top down
>    to the specified size.
> - If mem=SIZE@START is used, the original behavior is retained for
>    backward compatibility. This allows for defining specific memory
>    banks.
> 
> This change introduces an important usage rule reflected in the code's
> comments: the mem=SIZE format should only be specified once on the
> kernel command line. It acts as a single, global cap on total memory. In
> contrast, the mem=SIZE@START format can be used multiple times to
> define several distinct memory regions.
> 
> Signed-off-by: Ming Wang <wangming01@loongson.cn>
> ---
>   arch/loongarch/kernel/setup.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> index b99fbb388fe0..af59ba180dc2 100644
> --- a/arch/loongarch/kernel/setup.c
> +++ b/arch/loongarch/kernel/setup.c
> @@ -191,6 +191,16 @@ static int __init early_parse_mem(char *p)
>   		return -EINVAL;
>   	}
>   
> +	start = 0;
> +	size = memparse(p, &p);
> +	if (*p == '@')	/* Every mem=... should contain '@' */
> +		start = memparse(p + 1, &p);
> +	else {			/* Only one mem=... is allowed if no '@' */
> +		usermem = 1;
> +		memblock_enforce_memory_limit(size);
> +		return 0;
> +	}
> +
>   	/*
>   	 * If a user specifies memory size, we
>   	 * blow away any automatically generated
> @@ -201,14 +211,6 @@ static int __init early_parse_mem(char *p)
>   		memblock_remove(memblock_start_of_DRAM(),
>   			memblock_end_of_DRAM() - memblock_start_of_DRAM());
>   	}
> -	start = 0;
> -	size = memparse(p, &p);
> -	if (*p == '@')
> -		start = memparse(p + 1, &p);
> -	else {
> -		pr_err("Invalid format!\n");
> -		return -EINVAL;
> -	}
I don't understand. Isn't it better to modify the else{} directly here?

Thanks,
Yanteng

Hi Yanteng,

Thanks for reviewing the patch and for your insightful question.

On 7/2/25 10:11, Yanteng Si wrote:
> 在 7/1/25 5:04 PM, Ming Wang 写道:
>> The LoongArch mem= parameter parser was previously limited to the
>> mem=SIZE@START format. This was inconvenient for the common use case
>> of simply capping the total system memory, as it forced users to
>> manually specify a start address. It was also inconsistent with the
>> behavior on other architectures.
>>
>> This patch enhances the parser in early_parse_mem() to also support the
>> more user-friendly mem=SIZE format. The implementation now checks for
>> the presence of the '@' symbol to determine the user's intent:
>>
>> - If mem=SIZE is provided (no '@'), the kernel now calls
>>    memblock_enforce_memory_limit(). This trims memory from the top down
>>    to the specified size.
>> - If mem=SIZE@START is used, the original behavior is retained for
>>    backward compatibility. This allows for defining specific memory
>>    banks.
>>
>> This change introduces an important usage rule reflected in the code's
>> comments: the mem=SIZE format should only be specified once on the
>> kernel command line. It acts as a single, global cap on total memory. In
>> contrast, the mem=SIZE@START format can be used multiple times to
>> define several distinct memory regions.
>>
>> Signed-off-by: Ming Wang <wangming01@loongson.cn>
>> ---
>>   arch/loongarch/kernel/setup.c | 18 ++++++++++--------
>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/ 
>> setup.c
>> index b99fbb388fe0..af59ba180dc2 100644
>> --- a/arch/loongarch/kernel/setup.c
>> +++ b/arch/loongarch/kernel/setup.c
>> @@ -191,6 +191,16 @@ static int __init early_parse_mem(char *p)
>>           return -EINVAL;
>>       }
>> +    start = 0;
>> +    size = memparse(p, &p);
>> +    if (*p == '@')    /* Every mem=... should contain '@' */
>> +        start = memparse(p + 1, &p);
>> +    else {            /* Only one mem=... is allowed if no '@' */
>> +        usermem = 1;
>> +        memblock_enforce_memory_limit(size);
>> +        return 0;
>> +    }
>> +
>>       /*
>>        * If a user specifies memory size, we
>>        * blow away any automatically generated
>> @@ -201,14 +211,6 @@ static int __init early_parse_mem(char *p)
>>           memblock_remove(memblock_start_of_DRAM(),
>>               memblock_end_of_DRAM() - memblock_start_of_DRAM());
>>       }
>> -    start = 0;
>> -    size = memparse(p, &p);
>> -    if (*p == '@')
>> -        start = memparse(p + 1, &p);
>> -    else {
>> -        pr_err("Invalid format!\n");
>> -        return -EINVAL;
>> -    }
> I don't understand. Isn't it better to modify the else{} directly here?
> 
You've raised a very good point. The reason for moving the parsing logic 
to the top, rather than just modifying the original else block, is to 
handle the fundamentally different behaviors required for mem=SIZE 
versus mem=SIZE@START. The key lies in thisexisting block of code which 
handles the mem=SIZE@START case:

```
/*
* If a user specifies memory size, we
* blow away any automatically generated
* size.
*/
if (usermem == 0) {
	usermem = 1;
	memblock_remove(memblock_start_of_DRAM(),
			memblock_end_of_DRAM() - memblock_start_of_DRAM());
}
```

This code is destructive. As the comment says, it "blows away" the 
entire memory map discovered from the firmware (UEFI/ACPI). After this 
call, memblock is essentially empty, waiting for new regions to be added 
via memblock_add(). This is the correct behavior for mem=SIZE@START.

However, the new mem=SIZE functionality is meant to be non-destructive. 
It should take the existing firmware-provided memory map and simply trim 
it down to the desired size. The function 
memblock_enforce_memory_limit() is designed for this purpose—it operates 
on the current state of memblock.

If we were to keep the parsing logic at the end and only modify the else 
block, the destructive memblock_remove() call would have already 
executed for both cases. By that point, calling 
memblock_enforce_memory_limit() would be meaningless, as there would be 
no memory regions left in memblock to limit.

Therefore, the patch moves the parsing logic to the very beginning to 
create a clean separation:
1. It first checks if the format is mem=SIZE (no '@').
2. If it is, it performs the non-destructive limit on the intact memory 
map and returns immediately, completely bypassing the destructive 
memblock_remove() logic.
3. If the format is mem=SIZE@START, it falls through to the original 
destructive path as before.

I hope this explanation clarifies why the code structure was changed 
this way. It's crucial to ensure the non-destructive path is handled 
before any memory map information is lost.

Best regards,
Ming


> Thanks,
> Yanteng
> 

在 7/3/25 9:36 AM, Ming Wang 写道:
> Hi Yanteng,
>
> Thanks for reviewing the patch and for your insightful question.
>
> On 7/2/25 10:11, Yanteng Si wrote:
>> 在 7/1/25 5:04 PM, Ming Wang 写道:
>>> The LoongArch mem= parameter parser was previously limited to the
>>> mem=SIZE@START format. This was inconvenient for the common use case
>>> of simply capping the total system memory, as it forced users to
>>> manually specify a start address. It was also inconsistent with the
>>> behavior on other architectures.
>>>
>>> This patch enhances the parser in early_parse_mem() to also support the
>>> more user-friendly mem=SIZE format. The implementation now checks for
>>> the presence of the '@' symbol to determine the user's intent:
>>>
>>> - If mem=SIZE is provided (no '@'), the kernel now calls
>>>    memblock_enforce_memory_limit(). This trims memory from the top down
>>>    to the specified size.
>>> - If mem=SIZE@START is used, the original behavior is retained for
>>>    backward compatibility. This allows for defining specific memory
>>>    banks.
>>>
>>> This change introduces an important usage rule reflected in the code's
>>> comments: the mem=SIZE format should only be specified once on the
>>> kernel command line. It acts as a single, global cap on total memory. In
>>> contrast, the mem=SIZE@START format can be used multiple times to
>>> define several distinct memory regions.
>>>
>>> Signed-off-by: Ming Wang <wangming01@loongson.cn>
>>> ---
>>>   arch/loongarch/kernel/setup.c | 18 ++++++++++--------
>>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/ setup.c
>>> index b99fbb388fe0..af59ba180dc2 100644
>>> --- a/arch/loongarch/kernel/setup.c
>>> +++ b/arch/loongarch/kernel/setup.c
>>> @@ -191,6 +191,16 @@ static int __init early_parse_mem(char *p)
>>>           return -EINVAL;
>>>       }
>>> +    start = 0;
>>> +    size = memparse(p, &p);
>>> +    if (*p == '@')    /* Every mem=... should contain '@' */
>>> +        start = memparse(p + 1, &p);
>>> +    else {            /* Only one mem=... is allowed if no '@' */
>>> +        usermem = 1;
>>> +        memblock_enforce_memory_limit(size);
>>> +        return 0;
>>> +    }
>>> +
>>>       /*
>>>        * If a user specifies memory size, we
>>>        * blow away any automatically generated
>>> @@ -201,14 +211,6 @@ static int __init early_parse_mem(char *p)
>>>           memblock_remove(memblock_start_of_DRAM(),
>>>               memblock_end_of_DRAM() - memblock_start_of_DRAM());
>>>       }
>>> -    start = 0;
>>> -    size = memparse(p, &p);
>>> -    if (*p == '@')
>>> -        start = memparse(p + 1, &p);
>>> -    else {
>>> -        pr_err("Invalid format!\n");
>>> -        return -EINVAL;
>>> -    }
>> I don't understand. Isn't it better to modify the else{} directly here?
>>
> You've raised a very good point. The reason for moving the parsing logic to the top, rather than just modifying the original else block, is to handle the fundamentally different behaviors required for mem=SIZE versus mem=SIZE@START. The key lies in thisexisting block of code which handles the mem=SIZE@START case:
>
> ```
> /*
> * If a user specifies memory size, we
> * blow away any automatically generated
> * size.
> */
> if (usermem == 0) {
>     usermem = 1;
>     memblock_remove(memblock_start_of_DRAM(),
>             memblock_end_of_DRAM() - memblock_start_of_DRAM());
> }
> ```
>
> This code is destructive. As the comment says, it "blows away" the entire memory map discovered from the firmware (UEFI/ACPI). After this call, memblock is essentially empty, waiting for new regions to be added via memblock_add(). This is the correct behavior for mem=SIZE@START.
>
> However, the new mem=SIZE functionality is meant to be non-destructive. It should take the existing firmware-provided memory map and simply trim it down to the desired size. The function memblock_enforce_memory_limit() is designed for this purpose—it operates on the current state of memblock.
>
> If we were to keep the parsing logic at the end and only modify the else block, the destructive memblock_remove() call would have already executed for both cases. By that point, calling memblock_enforce_memory_limit() would be meaningless, as there would be no memory regions left in memblock to limit.
>
> Therefore, the patch moves the parsing logic to the very beginning to create a clean separation:
> 1. It first checks if the format is mem=SIZE (no '@').
> 2. If it is, it performs the non-destructive limit on the intact memory map and returns immediately, completely bypassing the destructive memblock_remove() logic.
> 3. If the format is mem=SIZE@START, it falls through to the original destructive path as before.

I have an idea: what if we move the destructive code into the if block?


Thanks,

Yanteng

>
> I hope this explanation clarifies why the code structure was changed this way. It's crucial to ensure the non-destructive path is handled before any memory map information is lost.
>
> Best regards,
> Ming
>
>
>> Thanks,
>> Yanteng
>>
>

Hi Yanteng, Huacai,

在 2025/7/7 10:33, Yanteng Si 写道:
> 
> 在 7/3/25 9:36 AM, Ming Wang 写道:
>> Hi Yanteng,
>>
>> Thanks for reviewing the patch and for your insightful question.
>>
>> On 7/2/25 10:11, Yanteng Si wrote:
>>> 在 7/1/25 5:04 PM, Ming Wang 写道:
>>>> The LoongArch mem= parameter parser was previously limited to the
>>>> mem=SIZE@START format. This was inconvenient for the common use case
>>>> of simply capping the total system memory, as it forced users to
>>>> manually specify a start address. It was also inconsistent with the
>>>> behavior on other architectures.
>>>>
>>>> This patch enhances the parser in early_parse_mem() to also support the
>>>> more user-friendly mem=SIZE format. The implementation now checks for
>>>> the presence of the '@' symbol to determine the user's intent:
>>>>
>>>> - If mem=SIZE is provided (no '@'), the kernel now calls
>>>>    memblock_enforce_memory_limit(). This trims memory from the top down
>>>>    to the specified size.
>>>> - If mem=SIZE@START is used, the original behavior is retained for
>>>>    backward compatibility. This allows for defining specific memory
>>>>    banks.
>>>>
>>>> This change introduces an important usage rule reflected in the code's
>>>> comments: the mem=SIZE format should only be specified once on the
>>>> kernel command line. It acts as a single, global cap on total 
>>>> memory. In
>>>> contrast, the mem=SIZE@START format can be used multiple times to
>>>> define several distinct memory regions.
>>>>
>>>> Signed-off-by: Ming Wang <wangming01@loongson.cn>
>>>> ---
>>>>   arch/loongarch/kernel/setup.c | 18 ++++++++++--------
>>>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/ 
>>>> setup.c
>>>> index b99fbb388fe0..af59ba180dc2 100644
>>>> --- a/arch/loongarch/kernel/setup.c
>>>> +++ b/arch/loongarch/kernel/setup.c
>>>> @@ -191,6 +191,16 @@ static int __init early_parse_mem(char *p)
>>>>           return -EINVAL;
>>>>       }
>>>> +    start = 0;
>>>> +    size = memparse(p, &p);
>>>> +    if (*p == '@')    /* Every mem=... should contain '@' */
>>>> +        start = memparse(p + 1, &p);
>>>> +    else {            /* Only one mem=... is allowed if no '@' */
>>>> +        usermem = 1;
>>>> +        memblock_enforce_memory_limit(size);
>>>> +        return 0;
>>>> +    }
>>>> +
>>>>       /*
>>>>        * If a user specifies memory size, we
>>>>        * blow away any automatically generated
>>>> @@ -201,14 +211,6 @@ static int __init early_parse_mem(char *p)
>>>>           memblock_remove(memblock_start_of_DRAM(),
>>>>               memblock_end_of_DRAM() - memblock_start_of_DRAM());
>>>>       }
>>>> -    start = 0;
>>>> -    size = memparse(p, &p);
>>>> -    if (*p == '@')
>>>> -        start = memparse(p + 1, &p);
>>>> -    else {
>>>> -        pr_err("Invalid format!\n");
>>>> -        return -EINVAL;
>>>> -    }
>>> I don't understand. Isn't it better to modify the else{} directly here?
>>>
>> You've raised a very good point. The reason for moving the parsing 
>> logic to the top, rather than just modifying the original else block, 
>> is to handle the fundamentally different behaviors required for 
>> mem=SIZE versus mem=SIZE@START. The key lies in thisexisting block of 
>> code which handles the mem=SIZE@START case:
>>
>> ```
>> /*
>> * If a user specifies memory size, we
>> * blow away any automatically generated
>> * size.
>> */
>> if (usermem == 0) {
>>     usermem = 1;
>>     memblock_remove(memblock_start_of_DRAM(),
>>             memblock_end_of_DRAM() - memblock_start_of_DRAM());
>> }
>> ```
>>
>> This code is destructive. As the comment says, it "blows away" the 
>> entire memory map discovered from the firmware (UEFI/ACPI). After this 
>> call, memblock is essentially empty, waiting for new regions to be 
>> added via memblock_add(). This is the correct behavior for 
>> mem=SIZE@START.
>>
>> However, the new mem=SIZE functionality is meant to be non- 
>> destructive. It should take the existing firmware-provided memory map 
>> and simply trim it down to the desired size. The function 
>> memblock_enforce_memory_limit() is designed for this purpose—it 
>> operates on the current state of memblock.
>>
>> If we were to keep the parsing logic at the end and only modify the 
>> else block, the destructive memblock_remove() call would have already 
>> executed for both cases. By that point, calling 
>> memblock_enforce_memory_limit() would be meaningless, as there would 
>> be no memory regions left in memblock to limit.
>>
>> Therefore, the patch moves the parsing logic to the very beginning to 
>> create a clean separation:
>> 1. It first checks if the format is mem=SIZE (no '@').
>> 2. If it is, it performs the non-destructive limit on the intact 
>> memory map and returns immediately, completely bypassing the 
>> destructive memblock_remove() logic.
>> 3. If the format is mem=SIZE@START, it falls through to the original 
>> destructive path as before.
> 
> I have an idea: what if we move the destructive code into the if block?

@Yanteng,
That's an excellent suggestion. You are right. Moving the destructive 
memblock_remove() logic inside the if (*p == '@') block is indeed a much 
cleaner way to structure the code. It improves readability by making the 
logic for each case self-contained within a direct if/else structure.

@Huacai,
Yanteng proposed a great improvement to the patch structure.

```
static int __init early_parse_mem(char *p)
{
     // ...
     size = memparse(p, &p);
     if (*p == '@') {
         // Handle 'mem=SIZE@START'
         // The destructive memblock_remove() goes here.
         // ...
         // memblock_add_node()
     } else {
         // Handle 'mem=SIZE'
         // The non-destructive memblock_enforce_memory_limit() goes here.
     }
     return 0;
}
```

Before I send out a v2, I'd like to ask for your opinion on this 
proposed change as well. Do you agree that this revised structure is the 
better approach?

Best regards,
Ming

> 
> 
> Thanks,
> 
> Yanteng
> 
>>
>> I hope this explanation clarifies why the code structure was changed 
>> this way. It's crucial to ensure the non-destructive path is handled 
>> before any memory map information is lost.
>>
>> Best regards,
>> Ming
>>
>>
>>> Thanks,
>>> Yanteng
>>>
>>

On Mon, Jul 7, 2025 at 6:03 PM Ming Wang <wangming01@loongson.cn> wrote:
>
> Hi Yanteng, Huacai,
>
> 在 2025/7/7 10:33, Yanteng Si 写道:
> >
> > 在 7/3/25 9:36 AM, Ming Wang 写道:
> >> Hi Yanteng,
> >>
> >> Thanks for reviewing the patch and for your insightful question.
> >>
> >> On 7/2/25 10:11, Yanteng Si wrote:
> >>> 在 7/1/25 5:04 PM, Ming Wang 写道:
> >>>> The LoongArch mem= parameter parser was previously limited to the
> >>>> mem=SIZE@START format. This was inconvenient for the common use case
> >>>> of simply capping the total system memory, as it forced users to
> >>>> manually specify a start address. It was also inconsistent with the
> >>>> behavior on other architectures.
> >>>>
> >>>> This patch enhances the parser in early_parse_mem() to also support the
> >>>> more user-friendly mem=SIZE format. The implementation now checks for
> >>>> the presence of the '@' symbol to determine the user's intent:
> >>>>
> >>>> - If mem=SIZE is provided (no '@'), the kernel now calls
> >>>>    memblock_enforce_memory_limit(). This trims memory from the top down
> >>>>    to the specified size.
> >>>> - If mem=SIZE@START is used, the original behavior is retained for
> >>>>    backward compatibility. This allows for defining specific memory
> >>>>    banks.
> >>>>
> >>>> This change introduces an important usage rule reflected in the code's
> >>>> comments: the mem=SIZE format should only be specified once on the
> >>>> kernel command line. It acts as a single, global cap on total
> >>>> memory. In
> >>>> contrast, the mem=SIZE@START format can be used multiple times to
> >>>> define several distinct memory regions.
> >>>>
> >>>> Signed-off-by: Ming Wang <wangming01@loongson.cn>
> >>>> ---
> >>>>   arch/loongarch/kernel/setup.c | 18 ++++++++++--------
> >>>>   1 file changed, 10 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/
> >>>> setup.c
> >>>> index b99fbb388fe0..af59ba180dc2 100644
> >>>> --- a/arch/loongarch/kernel/setup.c
> >>>> +++ b/arch/loongarch/kernel/setup.c
> >>>> @@ -191,6 +191,16 @@ static int __init early_parse_mem(char *p)
> >>>>           return -EINVAL;
> >>>>       }
> >>>> +    start = 0;
> >>>> +    size = memparse(p, &p);
> >>>> +    if (*p == '@')    /* Every mem=... should contain '@' */
> >>>> +        start = memparse(p + 1, &p);
> >>>> +    else {            /* Only one mem=... is allowed if no '@' */
> >>>> +        usermem = 1;
> >>>> +        memblock_enforce_memory_limit(size);
> >>>> +        return 0;
> >>>> +    }
> >>>> +
> >>>>       /*
> >>>>        * If a user specifies memory size, we
> >>>>        * blow away any automatically generated
> >>>> @@ -201,14 +211,6 @@ static int __init early_parse_mem(char *p)
> >>>>           memblock_remove(memblock_start_of_DRAM(),
> >>>>               memblock_end_of_DRAM() - memblock_start_of_DRAM());
> >>>>       }
> >>>> -    start = 0;
> >>>> -    size = memparse(p, &p);
> >>>> -    if (*p == '@')
> >>>> -        start = memparse(p + 1, &p);
> >>>> -    else {
> >>>> -        pr_err("Invalid format!\n");
> >>>> -        return -EINVAL;
> >>>> -    }
> >>> I don't understand. Isn't it better to modify the else{} directly here?
> >>>
> >> You've raised a very good point. The reason for moving the parsing
> >> logic to the top, rather than just modifying the original else block,
> >> is to handle the fundamentally different behaviors required for
> >> mem=SIZE versus mem=SIZE@START. The key lies in thisexisting block of
> >> code which handles the mem=SIZE@START case:
> >>
> >> ```
> >> /*
> >> * If a user specifies memory size, we
> >> * blow away any automatically generated
> >> * size.
> >> */
> >> if (usermem == 0) {
> >>     usermem = 1;
> >>     memblock_remove(memblock_start_of_DRAM(),
> >>             memblock_end_of_DRAM() - memblock_start_of_DRAM());
> >> }
> >> ```
> >>
> >> This code is destructive. As the comment says, it "blows away" the
> >> entire memory map discovered from the firmware (UEFI/ACPI). After this
> >> call, memblock is essentially empty, waiting for new regions to be
> >> added via memblock_add(). This is the correct behavior for
> >> mem=SIZE@START.
> >>
> >> However, the new mem=SIZE functionality is meant to be non-
> >> destructive. It should take the existing firmware-provided memory map
> >> and simply trim it down to the desired size. The function
> >> memblock_enforce_memory_limit() is designed for this purpose—it
> >> operates on the current state of memblock.
> >>
> >> If we were to keep the parsing logic at the end and only modify the
> >> else block, the destructive memblock_remove() call would have already
> >> executed for both cases. By that point, calling
> >> memblock_enforce_memory_limit() would be meaningless, as there would
> >> be no memory regions left in memblock to limit.
> >>
> >> Therefore, the patch moves the parsing logic to the very beginning to
> >> create a clean separation:
> >> 1. It first checks if the format is mem=SIZE (no '@').
> >> 2. If it is, it performs the non-destructive limit on the intact
> >> memory map and returns immediately, completely bypassing the
> >> destructive memblock_remove() logic.
> >> 3. If the format is mem=SIZE@START, it falls through to the original
> >> destructive path as before.
> >
> > I have an idea: what if we move the destructive code into the if block?
>
> @Yanteng,
> That's an excellent suggestion. You are right. Moving the destructive
> memblock_remove() logic inside the if (*p == '@') block is indeed a much
> cleaner way to structure the code. It improves readability by making the
> logic for each case self-contained within a direct if/else structure.
>
> @Huacai,
> Yanteng proposed a great improvement to the patch structure.
>
> ```
> static int __init early_parse_mem(char *p)
> {
>      // ...
>      size = memparse(p, &p);
>      if (*p == '@') {
>          // Handle 'mem=SIZE@START'
>          // The destructive memblock_remove() goes here.
>          // ...
>          // memblock_add_node()
>      } else {
>          // Handle 'mem=SIZE'
>          // The non-destructive memblock_enforce_memory_limit() goes here.
>      }
>      return 0;
> }
> ```
>
> Before I send out a v2, I'd like to ask for your opinion on this
> proposed change as well. Do you agree that this revised structure is the
> better approach?
Nested if conditions make the logic unclear, so I prefer the current patch.

Huacai

>
> Best regards,
> Ming
>
> >
> >
> > Thanks,
> >
> > Yanteng
> >
> >>
> >> I hope this explanation clarifies why the code structure was changed
> >> this way. It's crucial to ensure the non-destructive path is handled
> >> before any memory map information is lost.
> >>
> >> Best regards,
> >> Ming
> >>
> >>
> >>> Thanks,
> >>> Yanteng
> >>>
> >>
>