[PATCH for-6.12 2/4] block, bfq: choose the last bfqq from merge chain in bfq_setup_cooperator()

Yu Kuai posted 4 patches 1 year, 3 months ago
[PATCH for-6.12 2/4] block, bfq: choose the last bfqq from merge chain in bfq_setup_cooperator()
Posted by Yu Kuai 1 year, 3 months ago
From: Yu Kuai <yukuai3@huawei.com>

Consider the following merge chain:

Process 1       Process 2       Process 3	Process 4
 (BIC1)          (BIC2)          (BIC3)		 (BIC4)
  Λ                |               |               |
   \--------------\ \-------------\ \-------------\|
                   V               V		   V
  bfqq1--------->bfqq2---------->bfqq3----------->bfqq4

IO from Process 1 will get bfqf2 from BIC1 first, then
bfq_setup_cooperator() will found bfqq2 already merged to bfqq3 and then
handle this IO from bfqq3. However, the merge chain can be much deeper
and bfqq3 can be merged to other bfqq as well.

Fix this problem by iterating to the last bfqq in
bfq_setup_cooperator().

Fixes: 36eca8948323 ("block, bfq: add Early Queue Merge (EQM)")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 block/bfq-iosched.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 83adac3e71db..ffaa0d56328a 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2911,8 +2911,12 @@ bfq_setup_cooperator(struct bfq_data *bfqd, struct bfq_queue *bfqq,
 	struct bfq_iocq_bfqq_data *bfqq_data = &bic->bfqq_data[a_idx];
 
 	/* if a merge has already been setup, then proceed with that first */
-	if (bfqq->new_bfqq)
-		return bfqq->new_bfqq;
+	new_bfqq = bfqq->new_bfqq;
+	if (new_bfqq) {
+		while (new_bfqq->new_bfqq)
+			new_bfqq = new_bfqq->new_bfqq;
+		return new_bfqq;
+	}
 
 	/*
 	 * Check delayed stable merge for rotational or non-queueing
-- 
2.39.2

Re: [PATCH for-6.12 2/4] block, bfq: choose the last bfqq from merge chain in bfq_setup_cooperator()
Posted by Jan Kara 1 year, 3 months ago
On Mon 02-09-24 21:03:27, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> Consider the following merge chain:
> 
> Process 1       Process 2       Process 3	Process 4
>  (BIC1)          (BIC2)          (BIC3)		 (BIC4)
>   Λ                |               |               |
>    \--------------\ \-------------\ \-------------\|
>                    V               V		   V
>   bfqq1--------->bfqq2---------->bfqq3----------->bfqq4
> 
> IO from Process 1 will get bfqf2 from BIC1 first, then
> bfq_setup_cooperator() will found bfqq2 already merged to bfqq3 and then
> handle this IO from bfqq3. However, the merge chain can be much deeper
> and bfqq3 can be merged to other bfqq as well.
> 
> Fix this problem by iterating to the last bfqq in
> bfq_setup_cooperator().
> 
> Fixes: 36eca8948323 ("block, bfq: add Early Queue Merge (EQM)")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>

Good catch. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  block/bfq-iosched.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 83adac3e71db..ffaa0d56328a 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -2911,8 +2911,12 @@ bfq_setup_cooperator(struct bfq_data *bfqd, struct bfq_queue *bfqq,
>  	struct bfq_iocq_bfqq_data *bfqq_data = &bic->bfqq_data[a_idx];
>  
>  	/* if a merge has already been setup, then proceed with that first */
> -	if (bfqq->new_bfqq)
> -		return bfqq->new_bfqq;
> +	new_bfqq = bfqq->new_bfqq;
> +	if (new_bfqq) {
> +		while (new_bfqq->new_bfqq)
> +			new_bfqq = new_bfqq->new_bfqq;
> +		return new_bfqq;
> +	}
>  
>  	/*
>  	 * Check delayed stable merge for rotational or non-queueing
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR