[PATCH] mm/sparse: simplify the vmemmap alignment check

[PATCH] mm/sparse: simplify the vmemmap alignment check

[Kaitao Cheng]

From: Kaitao Cheng <chengkaitao@kylinos.cn>

sparse_init() is called only once during boot, and the vmemmap alignment
check is outside any loop, so VM_WARN_ON_ONCE() provides no benefit over
VM_WARN_ON() here.

Use VM_WARN_ON() and fold compound_info_has_mask() into its condition,
preserving short-circuit evaluation.

Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
 mm/sparse.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/sparse.c b/mm/sparse.c
index 6acb628c8a73..248bee2d39b9 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -277,10 +277,8 @@ static void __init sparse_metadata_init(void)
  */
 void __init sparse_init(void)
 {
-	if (compound_info_has_mask()) {
-		VM_WARN_ON_ONCE(!IS_ALIGNED((unsigned long) pfn_to_page(0),
-				    MAX_FOLIO_VMEMMAP_ALIGN));
-	}
+	VM_WARN_ON(compound_info_has_mask() && !IS_ALIGNED(
+		   (unsigned long)pfn_to_page(0), MAX_FOLIO_VMEMMAP_ALIGN));
 
 	sparse_metadata_init();
 	sparse_init_subsection_map();
-- 
2.53.0

Re: [PATCH] mm/sparse: simplify the vmemmap alignment check

[Muchun Song]

> On Oct 28, 2026, at 20:11, Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
> 
> From: Kaitao Cheng <chengkaitao@kylinos.cn>
> 
> sparse_init() is called only once during boot, and the vmemmap alignment
> check is outside any loop, so VM_WARN_ON_ONCE() provides no benefit over
> VM_WARN_ON() here.

Yes.

> 
> Use VM_WARN_ON() and fold compound_info_has_mask() into its condition,
> preserving short-circuit evaluation.
> 
> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
> ---
> mm/sparse.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 6acb628c8a73..248bee2d39b9 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -277,10 +277,8 @@ static void __init sparse_metadata_init(void)
>  */
> void __init sparse_init(void)
> {
> - 	if (compound_info_has_mask()) {
> - 		VM_WARN_ON_ONCE(!IS_ALIGNED((unsigned long) pfn_to_page(0),
> -    				MAX_FOLIO_VMEMMAP_ALIGN));
> - 	}
> + 	VM_WARN_ON(compound_info_has_mask() && !IS_ALIGNED(

We usually don't end with "(“. At least I rarely see that in the existing code.
We can break the line after &&.

Thanks,
Muchun

> +   		(unsigned long)pfn_to_page(0), MAX_FOLIO_VMEMMAP_ALIGN));
> 
> sparse_metadata_init();
> sparse_init_subsection_map();
> -- 
> 2.53.0
> 

Re: [PATCH] mm/sparse: simplify the vmemmap alignment check

[Mike Rapoport]

Hi,

On Wed, Oct 28, 2026 at 08:11:29PM +0800, Kaitao Cheng wrote:
> From: Kaitao Cheng <chengkaitao@kylinos.cn>
> 
> sparse_init() is called only once during boot, and the vmemmap alignment
> check is outside any loop, so VM_WARN_ON_ONCE() provides no benefit over
> VM_WARN_ON() here.
> 
> Use VM_WARN_ON() and fold compound_info_has_mask() into its condition,
> preserving short-circuit evaluation.

Do you think it's worth the churn?
 
> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
> ---
>  mm/sparse.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 6acb628c8a73..248bee2d39b9 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -277,10 +277,8 @@ static void __init sparse_metadata_init(void)
>   */
>  void __init sparse_init(void)
>  {
> -	if (compound_info_has_mask()) {
> -		VM_WARN_ON_ONCE(!IS_ALIGNED((unsigned long) pfn_to_page(0),
> -				    MAX_FOLIO_VMEMMAP_ALIGN));
> -	}
> +	VM_WARN_ON(compound_info_has_mask() && !IS_ALIGNED(
> +		   (unsigned long)pfn_to_page(0), MAX_FOLIO_VMEMMAP_ALIGN));
>  
>  	sparse_metadata_init();
>  	sparse_init_subsection_map();
> -- 
> 2.53.0
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH] mm/sparse: simplify the vmemmap alignment check

[Kaitao Cheng]

在 2026/10/29 00:08, Mike Rapoport 写道:
> Hi,
> 
> On Wed, Oct 28, 2026 at 08:11:29PM +0800, Kaitao Cheng wrote:
>> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>>
>> sparse_init() is called only once during boot, and the vmemmap alignment
>> check is outside any loop, so VM_WARN_ON_ONCE() provides no benefit over
>> VM_WARN_ON() here.
>>
>> Use VM_WARN_ON() and fold compound_info_has_mask() into its condition,
>> preserving short-circuit evaluation.
> 
> Do you think it's worth the churn?

I think saving two lines of code is nice. It also eliminates a variable,
even if only in debug kernels.

I’ll send a v2 following Muchun’s suggestion. If the consensus is that
this change isn’t worth merging, please feel free to ignore this email.

>> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
>> ---
>>  mm/sparse.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/mm/sparse.c b/mm/sparse.c
>> index 6acb628c8a73..248bee2d39b9 100644
>> --- a/mm/sparse.c
>> +++ b/mm/sparse.c
>> @@ -277,10 +277,8 @@ static void __init sparse_metadata_init(void)
>>   */
>>  void __init sparse_init(void)
>>  {
>> -	if (compound_info_has_mask()) {
>> -		VM_WARN_ON_ONCE(!IS_ALIGNED((unsigned long) pfn_to_page(0),
>> -				    MAX_FOLIO_VMEMMAP_ALIGN));
>> -	}
>> +	VM_WARN_ON(compound_info_has_mask() && !IS_ALIGNED(
>> +		   (unsigned long)pfn_to_page(0), MAX_FOLIO_VMEMMAP_ALIGN));
>>  
>>  	sparse_metadata_init();
>>  	sparse_init_subsection_map();
>> -- 
>> 2.53.0
>>
> 

-- 
Thanks
Kaitao Cheng