Alloc batch fixes
Jens Axboe
maintainerv2 cover Alloc batch fixes
Hi,
A few fixes related to the batched allocations:
- Have the requests hold a queue reference, flush them on schedule unplug as well.
- Make sure the queue matches, could be a mismatch if we're driving multiple devices.
Since v1:
- Reshuffle series to do plug rq alloc helper before enter changes
- Protect submit_bio_checks() by queue enter reference as well
Signature · 1 line
Jens Axboe
Jens Axboe
maintainerPATCH v2 1/5 block: have plug stored requests hold references to the queue
Requests that were stored in the cache deliberately didn't hold an enter reference to the queue, instead we grabbed one every time we pulled a request out of there. That made for awkward logic on freeing the remainder of the cached list, if needed, where we had to artificially raise the queue usage count before each free.
Grab references up front for cached plug requests. That's safer, and also more efficient.
Fixes 47c122e35d7e ("block: pre-allocate requests if plug is started and is a batch")
Signed-off-by Jens Axboe <[email protected]>
--- block/blk-core.c | 2 +- block/blk-mq.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-)
block/blk-core.c +1 -1
@@ -1643,7 +1643,7 @@ void blk_flush_plug(struct blk_plug *plug, bool from_schedule)flush_plug_callbacks (plug, from_schedule);if (!rq_list_empty (plug->mq_list))blk_mq_flush_plug_list (plug, from_schedule); -if (unlikely (!from_schedule && plug->cached_rq)) +if (unlikely (!rq_list_empty (plug->cached_rq)))blk_mq_free_plug_rqs (plug); }
block/blk-mq.c +4 -3
@@ -410,7 +410,10 @@ __blk_mq_alloc_requests_batch(struct blk_mq_alloc_data *data, tag_mask &= ~(1UL << i); rq =blk_mq_rq_ctx_init (data, tags, tag, alloc_time_ns);rq_list_add (data->cached_rq, rq); + nr++; } +/* caller already holds a reference, add for remainder */ +percpu_ref_get_many (&data->q->q_usage_counter, nr -1 ); data->nr_tags -= nr;return rq_list_pop (data->cached_rq);
@@ -630,10 +633,8 @@ void blk_mq_free_plug_rqs(struct blk_plug *plug) {struct request *rq; -while ((rq =rq_list_pop (&plug->cached_rq)) !=NULL ) { -percpu_ref_get (&rq->q->q_usage_counter); +while ((rq =rq_list_pop (&plug->cached_rq)) !=NULL )blk_mq_free_request (rq); - } } static void req_bio_endio(struct request *rq, struct bio *bio,
Signature · 1 line
2.33.1
Jens Axboe
maintainerPATCH v2 2/5 block: make blk_try_enter_queue() available for blk-mq
In reply to Jens AxboeJust a prep patch for shifting the queue enter logic.
Signed-off-by Jens Axboe <[email protected]>
--- block/blk-core.c | 26 +------------------------- block/blk.h | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 25 deletions(-)
block/blk-core.c +1 -25
@@ -386,30 +386,6 @@ void blk_cleanup_queue(struct request_queue *q) }EXPORT_SYMBOL (blk_cleanup_queue); -static bool blk_try_enter_queue (struct request_queue *q,bool pm) -{ -rcu_read_lock (); -if (!percpu_ref_tryget_live_rcu (&q->q_usage_counter)) - goto fail; - -/* -* The code that increments the pm_only counter must ensure that the -* counter is globally visible before the queue is unfrozen. -*/ -if (blk_queue_pm_only (q) && - (!pm ||queue_rpm_status (q) ==RPM_SUSPENDED )) - goto fail_put; - -rcu_read_unlock (); -return true; - -fail_put: -blk_queue_exit (q); -fail: -rcu_read_unlock (); -return false; -} -/** * blk_queue_enter() - try to increase q->q_usage_counter * @q: request queue pointer
@@ -442,7 +418,7 @@ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags)return 0 ; } -static inline int bio_queue_enter (struct bio *bio) +int bio_queue_enter (struct bio *bio) {struct request_queue *q =bdev_get_queue (bio->bi_bdev);
block/blk.h +25 -0
@@ -55,6 +55,31 @@ void blk_free_flush_queue(struct blk_flush_queue *q);void blk_freeze_queue (struct request_queue *q);void __blk_mq_unfreeze_queue (struct request_queue *q,bool force_atomic);void blk_queue_start_drain (struct request_queue *q); +int bio_queue_enter (struct bio *bio); + +static inline bool blk_try_enter_queue (struct request_queue *q,bool pm) +{ +rcu_read_lock (); +if (!percpu_ref_tryget_live_rcu (&q->q_usage_counter)) + goto fail; + +/* +* The code that increments the pm_only counter must ensure that the +* counter is globally visible before the queue is unfrozen. +*/ +if (blk_queue_pm_only (q) && + (!pm ||queue_rpm_status (q) ==RPM_SUSPENDED )) + goto fail_put; + +rcu_read_unlock (); +return true; + +fail_put: +blk_queue_exit (q); +fail: +rcu_read_unlock (); +return false; +}#define BIO_INLINE_VECS 4 struct bio_vec *bvec_alloc(mempool_t *pool, unsigned short *nr_vecs,
Signature · 1 line
2.33.1
Jens Axboe
maintainerPATCH v2 3/5 block: move plug rq alloc into helper
In reply to Jens AxboeThis is in preparation for a fix, but serves as a cleanup as well moving the plugged request logic out of blk_mq_submit_bio().
Signed-off-by Jens Axboe <[email protected]>
--- block/blk-mq.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)
block/blk-mq.c +19 -4
@@ -2478,6 +2478,23 @@ static inline unsigned short blk_plug_max_rq_count(struct blk_plug *plug)return BLK_MAX_REQUEST_COUNT ; } +static inline struct request *blk_get_plug_request (struct request_queue *q, +struct blk_plug *plug, +struct bio *bio) +{ +struct request *rq; + +if (!plug) +return NULL ; + rq =rq_list_peek (&plug->cached_rq); +if (rq) { + plug->cached_rq =rq_list_next (rq); +INIT_LIST_HEAD (&rq->queuelist); +return rq; + } +return NULL ; +} +/** * blk_mq_submit_bio - Create and send a request to block device. * @bio: Bio pointer.
@@ -2518,10 +2535,8 @@ void blk_mq_submit_bio(struct bio *bio)rq_qos_throttle (q, bio); plug =blk_mq_plug (q, bio); -if (plug && plug->cached_rq) { - rq =rq_list_pop (&plug->cached_rq); -INIT_LIST_HEAD (&rq->queuelist); - }else { + rq =blk_get_plug_request (q, plug, bio); +if (!rq) {struct blk_mq_alloc_data data = { .q = q, .nr_tags =1 ,
Signature · 1 line
2.33.1
Jens Axboe
maintainerPATCH v2 4/5 block: move queue enter logic into blk_mq_submit_bio()
In reply to Jens AxboeRetain the old logic for the fops based submit, but for our internal blk_mq_submit_bio(), move the queue entering logic into the core function itself.
We need to be a bit careful if going into the scheduler, as a scheduler or queue mappings can arbitrarily change before we have entered the queue. Have the bio scheduler mapping do that separately, it's a very cheap operation compared to actually doing merging locking and lookups.
Signed-off-by Jens Axboe <[email protected]>
--- block/blk-core.c | 17 ++++++--------- block/blk-mq-sched.c | 13 ++++++++--- block/blk-mq.c | 51 +++++++++++++++++++++++++++++--------------- block/blk.h | 1 + 4 files changed, 52 insertions(+), 30 deletions(-)
block/blk-core.c +7 -10
@@ -746,7 +746,7 @@ static inline blk_status_t blk_check_zone_append(struct request_queue *q,return BLK_STS_OK ; } -static noinline_for_stack bool submit_bio_checks(struct bio *bio) +noinline_for_stack bool submit_bio_checks(struct bio *bio) {struct block_device *bdev = bio->bi_bdev;struct request_queue *q =bdev_get_queue (bdev);
@@ -868,18 +868,15 @@ static void __submit_bio(struct bio *bio) {struct gendisk *disk = bio->bi_bdev->bd_disk; -if (unlikely (bio_queue_enter (bio) !=0 )) -return ; - -if (!submit_bio_checks (bio) || !blk_crypto_bio_prep (&bio)) - goto queue_exit;if (!disk->fops->submit_bio) {blk_mq_submit_bio (bio); -return ; + }else { +if (unlikely (bio_queue_enter (bio) !=0 )) +return ; +if (submit_bio_checks (bio) &&blk_crypto_bio_prep (&bio)) + disk->fops->submit_bio (bio); +blk_queue_exit (disk->queue); } - disk->fops->submit_bio (bio); -queue_exit: -blk_queue_exit (disk->queue); }/*
block/blk-mq-sched.c +10 -3
@@ -370,15 +370,20 @@ bool blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,bool ret = false;enum hctx_type type; -if (e && e->type->ops.bio_merge) -return e->type->ops.bio_merge (q, bio, nr_segs); +if (bio_queue_enter (bio)) +return false; + +if (e && e->type->ops.bio_merge) { + ret = e->type->ops.bio_merge (q, bio, nr_segs); + goto out_put; + } ctx =blk_mq_get_ctx (q); hctx =blk_mq_map_queue (q, bio->bi_opf, ctx); type = hctx->type;if (!(hctx->flags &BLK_MQ_F_SHOULD_MERGE ) ||list_empty_careful (&ctx->rq_lists[type])) -return false; + goto out_put;/* default per sw-queue merge */ spin_lock (&ctx->lock);
@@ -391,6 +396,8 @@ bool blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio, ret = true;spin_unlock (&ctx->lock); +out_put: +blk_queue_exit (q);return ret; }
block/blk-mq.c +34 -17
94 lines not on this page · raw message
block/blk.h +1 -0
@@ -56,6 +56,7 @@ void blk_freeze_queue(struct request_queue *q);void __blk_mq_unfreeze_queue (struct request_queue *q,bool force_atomic);void blk_queue_start_drain (struct request_queue *q);int bio_queue_enter (struct bio *bio); +bool submit_bio_checks (struct bio *bio);static inline bool blk_try_enter_queue (struct request_queue *q,bool pm) {
Signature · 1 line
2.33.1
Jens Axboe
maintainerPATCH v2 5/5 block: ensure cached plug request matches the current queue
In reply to Jens AxboeIf we're driving multiple devices, we could have pre-populated the cache for a different device. Ensure that the empty request matches the current queue.
Fixes 47c122e35d7e ("block: pre-allocate requests if plug is started and is a batch")
Signed-off-by Jens Axboe <[email protected]>
--- block/blk-mq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
block/blk-mq.c +1 -1
@@ -2487,7 +2487,7 @@ static inline struct request *blk_get_plug_request(struct request_queue *q,if (!plug)return NULL ; rq =rq_list_peek (&plug->cached_rq); -if (!rq) +if (!rq || rq->q != q)return NULL ;if (unlikely (!submit_bio_checks (bio)))return ERR_PTR (-EIO );
Signature · 1 line
2.33.1