Re: [PATCH v2 1/2] x86/mm: rename the confusing local variable in early_memremap_is_setup_data()

[Tom Lendacky]

On 8/29/24 05:40, Baoquan He wrote:
> In function early_memremap_is_setup_data(), parameter 'size' passed has
> the same name as the local variable inside the while loop. That
> confuses people who sometime mix up them when reading code.
> 
> Here rename the local variable 'size' inside while loop to 'sd_size'.
> 
> And also add one local variable 'sd_size' likewise in function
> memremap_is_setup_data() to simplify code. In later patch, this can also
> be used.
> 
> Signed-off-by: Baoquan He <bhe@xxxxxxxxxx>

Acked-by: Tom Lendacky <thomas.lendacky@xxxxxxx>

> ---
>  arch/x86/mm/ioremap.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index aa7d279321ea..f1ee8822ddf1 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -640,7 +640,7 @@ static bool memremap_is_setup_data(resource_size_t phys_addr,
>  
>  	paddr = boot_params.hdr.setup_data;
>  	while (paddr) {
> -		unsigned int len;
> +		unsigned int len, sd_size;
>  
>  		if (phys_addr == paddr)
>  			return true;
> @@ -652,6 +652,8 @@ static bool memremap_is_setup_data(resource_size_t phys_addr,
>  			return false;
>  		}
>  
> +		sd_size = sizeof(*data);
> +
>  		paddr_next = data->next;
>  		len = data->len;
>  
> @@ -662,7 +664,9 @@ static bool memremap_is_setup_data(resource_size_t phys_addr,
>  
>  		if (data->type == SETUP_INDIRECT) {
>  			memunmap(data);
> -			data = memremap(paddr, sizeof(*data) + len,
> +
> +			sd_size += len;
> +			data = memremap(paddr, sd_size,
>  					MEMREMAP_WB | MEMREMAP_DEC);
>  			if (!data) {
>  				pr_warn("failed to memremap indirect setup_data\n");
> @@ -701,7 +705,7 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
>  
>  	paddr = boot_params.hdr.setup_data;
>  	while (paddr) {
> -		unsigned int len, size;
> +		unsigned int len, sd_size;
>  
>  		if (phys_addr == paddr)
>  			return true;
> @@ -712,7 +716,7 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
>  			return false;
>  		}
>  
> -		size = sizeof(*data);
> +		sd_size = sizeof(*data);
>  
>  		paddr_next = data->next;
>  		len = data->len;
> @@ -723,9 +727,9 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
>  		}
>  
>  		if (data->type == SETUP_INDIRECT) {
> -			size += len;
> +			sd_size += len;
>  			early_memunmap(data, sizeof(*data));
> -			data = early_memremap_decrypted(paddr, size);
> +			data = early_memremap_decrypted(paddr, sd_size);
>  			if (!data) {
>  				pr_warn("failed to early memremap indirect setup_data\n");
>  				return false;
> @@ -739,7 +743,7 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
>  			}
>  		}
>  
> -		early_memunmap(data, size);
> +		early_memunmap(data, sd_size);
>  
>  		if ((phys_addr > paddr) && (phys_addr < (paddr + len)))
>  			return true;