| Message ID | 575be19c-01f4-4159-874b-d1e5dcbdc935@kernel.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: ensure we hold a queue reference when using queue limits | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Sat, Jan 13, 2024 at 12:15 AM Jens Axboe <axboe@kernel.dk> wrote: > > q_usage_counter is the only thing preventing us from the limits changing > under us in __bio_split_to_limits, but blk_mq_submit_bio doesn't hold > it while calling into it. > > Move the splitting inside the region where we know we've got a queue > reference. Ideally this could still remain a shared section of code, but > let's keep the fix simple and defer any refactoring here to later. > > Reported-by: Christoph Hellwig <hch@lst.de> > Fixes: 9d497e2941c3 ("block: don't protect submit_bio_checks by q_usage_counter") The fixes tag is wrong, and it should be: Fixes: 900e08075202 ("block: move queue enter logic into blk_mq_submit_bio()") > Signed-off-by: Jens Axboe <axboe@kernel.dk> > > --- > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index f57b86d6de6a..e02c4b1af8c5 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -2964,12 +2964,6 @@ void blk_mq_submit_bio(struct bio *bio) > blk_status_t ret; > > bio = blk_queue_bounce(bio, q); > - if (bio_may_exceed_limits(bio, &q->limits)) { > - bio = __bio_split_to_limits(bio, &q->limits, &nr_segs); > - if (!bio) > - return; > - } > - > bio_set_ioprio(bio); > > if (plug) { > @@ -2978,6 +2972,11 @@ void blk_mq_submit_bio(struct bio *bio) > rq = NULL; > } > if (rq) { > + if (unlikely(bio_may_exceed_limits(bio, &q->limits))) { > + bio = __bio_split_to_limits(bio, &q->limits, &nr_segs); > + if (!bio) > + return; > + } > if (!bio_integrity_prep(bio)) > return; > if (blk_mq_attempt_bio_merge(q, bio, nr_segs)) > @@ -2988,6 +2987,11 @@ void blk_mq_submit_bio(struct bio *bio) > } else { > if (unlikely(bio_queue_enter(bio))) > return; > + if (unlikely(bio_may_exceed_limits(bio, &q->limits))) { > + bio = __bio_split_to_limits(bio, &q->limits, &nr_segs); > + if (!bio) > + goto fail; > + } With "Fixes" tag fixed, the patch looks fine: Reviewed-by: Ming Lei <ming.lei@redhat.com>
On 1/12/24 7:05 PM, Ming Lei wrote: > On Sat, Jan 13, 2024 at 12:15?AM Jens Axboe <axboe@kernel.dk> wrote: >> >> q_usage_counter is the only thing preventing us from the limits changing >> under us in __bio_split_to_limits, but blk_mq_submit_bio doesn't hold >> it while calling into it. >> >> Move the splitting inside the region where we know we've got a queue >> reference. Ideally this could still remain a shared section of code, but >> let's keep the fix simple and defer any refactoring here to later. >> >> Reported-by: Christoph Hellwig <hch@lst.de> >> Fixes: 9d497e2941c3 ("block: don't protect submit_bio_checks by q_usage_counter") > > The fixes tag is wrong, and it should be: > > Fixes: 900e08075202 ("block: move queue enter logic into blk_mq_submit_bio()") You're right, I fixed it up.
diff --git a/block/blk-mq.c b/block/blk-mq.c index f57b86d6de6a..e02c4b1af8c5 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2964,12 +2964,6 @@ void blk_mq_submit_bio(struct bio *bio) blk_status_t ret; bio = blk_queue_bounce(bio, q); - if (bio_may_exceed_limits(bio, &q->limits)) { - bio = __bio_split_to_limits(bio, &q->limits, &nr_segs); - if (!bio) - return; - } - bio_set_ioprio(bio); if (plug) { @@ -2978,6 +2972,11 @@ void blk_mq_submit_bio(struct bio *bio) rq = NULL; } if (rq) { + if (unlikely(bio_may_exceed_limits(bio, &q->limits))) { + bio = __bio_split_to_limits(bio, &q->limits, &nr_segs); + if (!bio) + return; + } if (!bio_integrity_prep(bio)) return; if (blk_mq_attempt_bio_merge(q, bio, nr_segs)) @@ -2988,6 +2987,11 @@ void blk_mq_submit_bio(struct bio *bio) } else { if (unlikely(bio_queue_enter(bio))) return; + if (unlikely(bio_may_exceed_limits(bio, &q->limits))) { + bio = __bio_split_to_limits(bio, &q->limits, &nr_segs); + if (!bio) + goto fail; + } if (!bio_integrity_prep(bio)) goto fail; }
q_usage_counter is the only thing preventing us from the limits changing under us in __bio_split_to_limits, but blk_mq_submit_bio doesn't hold it while calling into it. Move the splitting inside the region where we know we've got a queue reference. Ideally this could still remain a shared section of code, but let's keep the fix simple and defer any refactoring here to later. Reported-by: Christoph Hellwig <hch@lst.de> Fixes: 9d497e2941c3 ("block: don't protect submit_bio_checks by q_usage_counter") Signed-off-by: Jens Axboe <axboe@kernel.dk> ---