Commit 73aeab37 authored by Yu Kuai's avatar Yu Kuai Committed by Jens Axboe

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 0e456dba ("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: 0e456dba ("block, bfq: choose the last bfqq from merge chain in bfq_setup_cooperator()")
Signed-off-by: default avatarYu Kuai <yukuai3@huawei.com>
Link: https://lore.kernel.org/r/20240909134154.954924-3-yukuai1@huaweicloud.comSigned-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 1ba0403a
...@@ -3129,10 +3129,12 @@ void bfq_release_process_ref(struct bfq_data *bfqd, struct bfq_queue *bfqq) ...@@ -3129,10 +3129,12 @@ void bfq_release_process_ref(struct bfq_data *bfqd, struct bfq_queue *bfqq)
bfq_put_queue(bfqq); bfq_put_queue(bfqq);
} }
static void static struct bfq_queue *bfq_merge_bfqqs(struct bfq_data *bfqd,
bfq_merge_bfqqs(struct bfq_data *bfqd, struct bfq_io_cq *bic, struct bfq_io_cq *bic,
struct bfq_queue *bfqq, struct bfq_queue *new_bfqq) struct bfq_queue *bfqq)
{ {
struct bfq_queue *new_bfqq = bfqq->new_bfqq;
bfq_log_bfqq(bfqd, bfqq, "merging with queue %lu", bfq_log_bfqq(bfqd, bfqq, "merging with queue %lu",
(unsigned long)new_bfqq->pid); (unsigned long)new_bfqq->pid);
/* Save weight raising and idle window of the merged queues */ /* 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, ...@@ -3226,6 +3228,8 @@ bfq_merge_bfqqs(struct bfq_data *bfqd, struct bfq_io_cq *bic,
bfq_reassign_last_bfqq(bfqq, new_bfqq); bfq_reassign_last_bfqq(bfqq, new_bfqq);
bfq_release_process_ref(bfqd, bfqq); bfq_release_process_ref(bfqd, bfqq);
return new_bfqq;
} }
static bool bfq_allow_bio_merge(struct request_queue *q, struct request *rq, 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, ...@@ -3261,14 +3265,8 @@ static bool bfq_allow_bio_merge(struct request_queue *q, struct request *rq,
* fulfilled, i.e., bic can be redirected to new_bfqq * fulfilled, i.e., bic can be redirected to new_bfqq
* and bfqq can be put. * and bfqq can be put.
*/ */
bfq_merge_bfqqs(bfqd, bfqd->bio_bic, bfqq, while (bfqq != new_bfqq)
new_bfqq); bfqq = bfq_merge_bfqqs(bfqd, bfqd->bio_bic, bfqq);
/*
* If we get here, bio will be queued into new_queue,
* so use new_bfqq to decide whether bio and rq can be
* merged.
*/
bfqq = new_bfqq;
/* /*
* Change also bqfd->bio_bfqq, as * Change also bqfd->bio_bfqq, as
...@@ -5705,9 +5703,7 @@ bfq_do_early_stable_merge(struct bfq_data *bfqd, struct bfq_queue *bfqq, ...@@ -5705,9 +5703,7 @@ bfq_do_early_stable_merge(struct bfq_data *bfqd, struct bfq_queue *bfqq,
* state before killing it. * state before killing it.
*/ */
bfqq->bic = bic; bfqq->bic = bic;
bfq_merge_bfqqs(bfqd, bic, bfqq, new_bfqq); return bfq_merge_bfqqs(bfqd, bic, bfqq);
return new_bfqq;
} }
/* /*
...@@ -6162,6 +6158,7 @@ static bool __bfq_insert_request(struct bfq_data *bfqd, struct request *rq) ...@@ -6162,6 +6158,7 @@ static bool __bfq_insert_request(struct bfq_data *bfqd, struct request *rq)
bool waiting, idle_timer_disabled = false; bool waiting, idle_timer_disabled = false;
if (new_bfqq) { if (new_bfqq) {
struct bfq_queue *old_bfqq = bfqq;
/* /*
* Release the request's reference to the old bfqq * Release the request's reference to the old bfqq
* and make sure one is taken to the shared queue. * 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) ...@@ -6178,18 +6175,18 @@ static bool __bfq_insert_request(struct bfq_data *bfqd, struct request *rq)
* new_bfqq. * new_bfqq.
*/ */
if (bic_to_bfqq(RQ_BIC(rq), true, if (bic_to_bfqq(RQ_BIC(rq), true,
bfq_actuator_index(bfqd, rq->bio)) == bfqq) bfq_actuator_index(bfqd, rq->bio)) == bfqq) {
bfq_merge_bfqqs(bfqd, RQ_BIC(rq), while (bfqq != new_bfqq)
bfqq, new_bfqq); bfqq = bfq_merge_bfqqs(bfqd, RQ_BIC(rq), bfqq);
}
bfq_clear_bfqq_just_created(bfqq); bfq_clear_bfqq_just_created(old_bfqq);
/* /*
* rq is about to be enqueued into new_bfqq, * rq is about to be enqueued into new_bfqq,
* release rq reference on bfqq * release rq reference on bfqq
*/ */
bfq_put_queue(bfqq); bfq_put_queue(old_bfqq);
rq->elv.priv[1] = new_bfqq; rq->elv.priv[1] = new_bfqq;
bfqq = new_bfqq;
} }
bfq_update_io_thinktime(bfqd, bfqq); bfq_update_io_thinktime(bfqd, bfqq);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment