Skip to content

Commit 73aeab3

Browse files
YuKuai-huaweiaxboe
authored andcommitted
block, bfq: fix procress reference leakage for bfqq in merge chain
Original state: Process 1 Process 2 Process 3 Process 4 (BIC1) (BIC2) (BIC3) (BIC4) Λ | | | \--------------\ \-------------\ \-------------\| V V V bfqq1--------->bfqq2---------->bfqq3----------->bfqq4 ref 0 1 2 4 After commit 0e456db ("block, bfq: choose the last bfqq from merge chain in bfq_setup_cooperator()"), if P1 issues a new IO: Without the patch: Process 1 Process 2 Process 3 Process 4 (BIC1) (BIC2) (BIC3) (BIC4) Λ | | | \------------------------------\ \-------------\| V V bfqq1--------->bfqq2---------->bfqq3----------->bfqq4 ref 0 0 2 4 bfqq3 will be used to handle IO from P1, this is not expected, IO should be redirected to bfqq4; With the patch: ------------------------------------------- | | Process 1 Process 2 Process 3 | Process 4 (BIC1) (BIC2) (BIC3) | (BIC4) | | | | \-------------\ \-------------\| V V bfqq1--------->bfqq2---------->bfqq3----------->bfqq4 ref 0 0 2 4 IO is redirected to bfqq4, however, procress reference of bfqq3 is still 2, while there is only P2 using it. Fix the problem by calling bfq_merge_bfqqs() for each bfqq in the merge chain. Also change bfqq_merge_bfqqs() to return new_bfqq to simplify code. Fixes: 0e456db ("block, bfq: choose the last bfqq from merge chain in bfq_setup_cooperator()") Signed-off-by: Yu Kuai <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jens Axboe <[email protected]>
1 parent 1ba0403 commit 73aeab3

File tree

1 file changed

+17
-20
lines changed

1 file changed

+17
-20
lines changed

block/bfq-iosched.c

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3129,10 +3129,12 @@ void bfq_release_process_ref(struct bfq_data *bfqd, struct bfq_queue *bfqq)
31293129
bfq_put_queue(bfqq);
31303130
}
31313131

3132-
static void
3133-
bfq_merge_bfqqs(struct bfq_data *bfqd, struct bfq_io_cq *bic,
3134-
struct bfq_queue *bfqq, struct bfq_queue *new_bfqq)
3132+
static struct bfq_queue *bfq_merge_bfqqs(struct bfq_data *bfqd,
3133+
struct bfq_io_cq *bic,
3134+
struct bfq_queue *bfqq)
31353135
{
3136+
struct bfq_queue *new_bfqq = bfqq->new_bfqq;
3137+
31363138
bfq_log_bfqq(bfqd, bfqq, "merging with queue %lu",
31373139
(unsigned long)new_bfqq->pid);
31383140
/* Save weight raising and idle window of the merged queues */
@@ -3226,6 +3228,8 @@ bfq_merge_bfqqs(struct bfq_data *bfqd, struct bfq_io_cq *bic,
32263228
bfq_reassign_last_bfqq(bfqq, new_bfqq);
32273229

32283230
bfq_release_process_ref(bfqd, bfqq);
3231+
3232+
return new_bfqq;
32293233
}
32303234

32313235
static bool bfq_allow_bio_merge(struct request_queue *q, struct request *rq,
@@ -3261,14 +3265,8 @@ static bool bfq_allow_bio_merge(struct request_queue *q, struct request *rq,
32613265
* fulfilled, i.e., bic can be redirected to new_bfqq
32623266
* and bfqq can be put.
32633267
*/
3264-
bfq_merge_bfqqs(bfqd, bfqd->bio_bic, bfqq,
3265-
new_bfqq);
3266-
/*
3267-
* If we get here, bio will be queued into new_queue,
3268-
* so use new_bfqq to decide whether bio and rq can be
3269-
* merged.
3270-
*/
3271-
bfqq = new_bfqq;
3268+
while (bfqq != new_bfqq)
3269+
bfqq = bfq_merge_bfqqs(bfqd, bfqd->bio_bic, bfqq);
32723270

32733271
/*
32743272
* Change also bqfd->bio_bfqq, as
@@ -5705,9 +5703,7 @@ bfq_do_early_stable_merge(struct bfq_data *bfqd, struct bfq_queue *bfqq,
57055703
* state before killing it.
57065704
*/
57075705
bfqq->bic = bic;
5708-
bfq_merge_bfqqs(bfqd, bic, bfqq, new_bfqq);
5709-
5710-
return new_bfqq;
5706+
return bfq_merge_bfqqs(bfqd, bic, bfqq);
57115707
}
57125708

57135709
/*
@@ -6162,6 +6158,7 @@ static bool __bfq_insert_request(struct bfq_data *bfqd, struct request *rq)
61626158
bool waiting, idle_timer_disabled = false;
61636159

61646160
if (new_bfqq) {
6161+
struct bfq_queue *old_bfqq = bfqq;
61656162
/*
61666163
* Release the request's reference to the old bfqq
61676164
* and make sure one is taken to the shared queue.
@@ -6178,18 +6175,18 @@ static bool __bfq_insert_request(struct bfq_data *bfqd, struct request *rq)
61786175
* new_bfqq.
61796176
*/
61806177
if (bic_to_bfqq(RQ_BIC(rq), true,
6181-
bfq_actuator_index(bfqd, rq->bio)) == bfqq)
6182-
bfq_merge_bfqqs(bfqd, RQ_BIC(rq),
6183-
bfqq, new_bfqq);
6178+
bfq_actuator_index(bfqd, rq->bio)) == bfqq) {
6179+
while (bfqq != new_bfqq)
6180+
bfqq = bfq_merge_bfqqs(bfqd, RQ_BIC(rq), bfqq);
6181+
}
61846182

6185-
bfq_clear_bfqq_just_created(bfqq);
6183+
bfq_clear_bfqq_just_created(old_bfqq);
61866184
/*
61876185
* rq is about to be enqueued into new_bfqq,
61886186
* release rq reference on bfqq
61896187
*/
6190-
bfq_put_queue(bfqq);
6188+
bfq_put_queue(old_bfqq);
61916189
rq->elv.priv[1] = new_bfqq;
6192-
bfqq = new_bfqq;
61936190
}
61946191

61956192
bfq_update_io_thinktime(bfqd, bfqq);

0 commit comments

Comments
 (0)