Re: [PATCH] powerpc/kexec_file: Simplify add_usable_mem()

[Sourabh Jain]

On 16/09/26 21:01, Thorsten Blum wrote:
> Use max() and min() to calculate loc_base and loc_end, respectively.
> Skip to the next range if loc_base > loc_end and drop the now-obsolete
> boolean add variable.
>
> Signed-off-by: Thorsten Blum <blum@xxxxxxxxxx>
> ---
>   arch/powerpc/kexec/file_load_64.c | 29 +++++++++--------------------
>   1 file changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index bd80c5fb1b1f..6162afbe919a 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -20,6 +20,7 @@
>   #include <linux/of.h>
>   #include <linux/of_address.h>
>   #include <linux/memblock.h>
> +#include <linux/minmax.h>
>   #include <linux/slab.h>
>   #include <linux/vmalloc.h>
>   #include <asm/setup.h>
> @@ -104,31 +105,19 @@ static __be64 *check_realloc_usable_mem(struct umem_info *um_info, int cnt)
>   static int add_usable_mem(struct umem_info *um_info, u64 base, u64 end)
>   {
For context:

The primary purpose of this function is to add the given range
(base, end) to buf if it falls within any of the ranges in
um_info->ranges.

If (base, end) is completely contained within any of the ranges in
um_info->ranges, the entire range is added to buf. If it overlaps
with any of the ranges, only the overlapping part is added to buf.

The changes below implement the same logic with less code by eliminating
the ranges that fall outside (base, end), instead of checking whether
(base, end) is completely contained within or overlaps with each range.

So, the changes look good to me. Feel free to add:

Reviewed-by: Sourabh Jain sourabhjain@xxxxxxxxxxxxx

>   	u64 loc_base, loc_end;
> -	bool add;
>   	int i;
>   
>   	for (i = 0; i < um_info->nr_ranges; i++) {
> -		add = false;
> -		loc_base = um_info->ranges[i].start;
> -		loc_end = um_info->ranges[i].end;
> -		if (loc_base >= base && loc_end <= end)
> -			add = true;
> -		else if (base <= loc_end && end >= loc_base) {
> -			if (loc_base < base)
> -				loc_base = base;
> -			if (loc_end > end)
> -				loc_end = end;
> -			add = true;
> -		}
> +		loc_base = max(base, um_info->ranges[i].start);
> +		loc_end = min(end, um_info->ranges[i].end);
> +		if (loc_base > loc_end)
> +			continue;
>   
> -		if (add) {
> -			if (!check_realloc_usable_mem(um_info, 2))
> -				return -ENOMEM;
> +		if (!check_realloc_usable_mem(um_info, 2))
> +			return -ENOMEM;
>   
> -			um_info->buf[um_info->idx++] = cpu_to_be64(loc_base);
> -			um_info->buf[um_info->idx++] =
> -					cpu_to_be64(loc_end - loc_base + 1);
> -		}
> +		um_info->buf[um_info->idx++] = cpu_to_be64(loc_base);
> +		um_info->buf[um_info->idx++] = cpu_to_be64(loc_end - loc_base + 1);
>   	}
>   
>   	return 0;