Re: btree: fix merge logic to use btree_last() return value

[Kuan-Wei Chiu <visitorckw@gmail.com>]

Hi Guan-Chun,

On Fri, Aug 22, 2025 at 04:58:51PM +0800, Guan-Chun.Wu wrote:
> Previously btree_merge() called btree_last() only to test existence,
> then performed an extra btree_lookup() to fetch the value. This patch
> changes it to directly use the value returned by btree_last(), avoiding
> redundant lookups and simplifying the merge loop.

The code change itself looks correct.

However, the subject line gives the impression that this patch fixes a
bug, while it actually just simplifies the logic. Could you consider
updating the subject to better reflect the nature of the change?

BTW, it seems that only the qla2xxx SCSI driver uses the btree
library, and that driver doesn't actually call btree_merge(). So in
practice, this function is unused in the kernel. Should we consider
removing it entirely?

> 
> Signed-off-by: Guan-Chun.Wu <409411716@gms.tku.edu.tw>

Is the dot in your real name intentional?

Regards,
Kuan-Wei

> ---
>  lib/btree.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/btree.c b/lib/btree.c
> index bb81d3393ac5..9c80c0c7bba8 100644
> --- a/lib/btree.c
> +++ b/lib/btree.c
> @@ -653,9 +653,9 @@ int btree_merge(struct btree_head *target, struct btree_head *victim,
>  	 * walks to remove a single object from the victim.
>  	 */
>  	for (;;) {
> -		if (!btree_last(victim, geo, key))
> +		val = btree_last(victim, geo, key);
> +		if (!val)
>  			break;
> -		val = btree_lookup(victim, geo, key);
>  		err = btree_insert(target, geo, key, val, gfp);
>  		if (err)
>  			return err;
> -- 
> 2.34.1
> 
>