| Message ID | 4ad0888df567a8bd75676b618ad87147c634d7b0.1597637287.git.baolin.wang@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Some clean-ups for bio merge | expand |
On Mon, Aug 17, 2020 at 12:09:19PM +0800, Baolin Wang wrote: > The blk_mq_sched_bio_merge() just wrap the __blk_mq_sched_bio_merge(), and > no other places will use __blk_mq_sched_bio_merge(). Thus we can combine > these 2 similar functions into one function. I think the idea was to avoid the function call for the nomerges fast path. Not sure if that is really worth it.
On Mon, Aug 17, 2020 at 08:32:41AM +0200, Christoph Hellwig wrote: > On Mon, Aug 17, 2020 at 12:09:19PM +0800, Baolin Wang wrote: > > The blk_mq_sched_bio_merge() just wrap the __blk_mq_sched_bio_merge(), and > > no other places will use __blk_mq_sched_bio_merge(). Thus we can combine > > these 2 similar functions into one function. > > I think the idea was to avoid the function call for the nomerges fast > path. Not sure if that is really worth it. Um, no places will use __blk_mq_sched_bio_merge(), not sure if it is a good choice we still keep an unused and similar function? Thanks for all your good suggestion.
On Mon, Aug 17, 2020 at 08:14:08PM +0800, Baolin Wang wrote: > On Mon, Aug 17, 2020 at 08:32:41AM +0200, Christoph Hellwig wrote: > > On Mon, Aug 17, 2020 at 12:09:19PM +0800, Baolin Wang wrote: > > > The blk_mq_sched_bio_merge() just wrap the __blk_mq_sched_bio_merge(), and > > > no other places will use __blk_mq_sched_bio_merge(). Thus we can combine > > > these 2 similar functions into one function. > > > > I think the idea was to avoid the function call for the nomerges fast > > path. Not sure if that is really worth it. > > Um, no places will use __blk_mq_sched_bio_merge(), not sure if it is a > good choice we still keep an unused and similar function? Well, blk_mq_sched_bio_merge calls __blk_mq_sched_bio_merge, after performing two fast path checks.
On Mon, Aug 17, 2020 at 02:26:08PM +0200, Christoph Hellwig wrote: > On Mon, Aug 17, 2020 at 08:14:08PM +0800, Baolin Wang wrote: > > On Mon, Aug 17, 2020 at 08:32:41AM +0200, Christoph Hellwig wrote: > > > On Mon, Aug 17, 2020 at 12:09:19PM +0800, Baolin Wang wrote: > > > > The blk_mq_sched_bio_merge() just wrap the __blk_mq_sched_bio_merge(), and > > > > no other places will use __blk_mq_sched_bio_merge(). Thus we can combine > > > > these 2 similar functions into one function. > > > > > > I think the idea was to avoid the function call for the nomerges fast > > > path. Not sure if that is really worth it. > > > > Um, no places will use __blk_mq_sched_bio_merge(), not sure if it is a > > good choice we still keep an unused and similar function? > > Well, blk_mq_sched_bio_merge calls __blk_mq_sched_bio_merge, after > performing two fast path checks. What I mean is blk_mq_sched_bio_merge() just wrap the __blk_mq_sched_bio_merge(), and no other users will call __blk_mq_sched_bio_merge(). Anyway, I will drop this patch as you suggested.
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c index 1cc7919..ba34460 100644 --- a/block/blk-mq-sched.c +++ b/block/blk-mq-sched.c @@ -408,7 +408,7 @@ bool blk_mq_bio_list_merge(struct request_queue *q, struct list_head *list, } EXPORT_SYMBOL_GPL(blk_mq_bio_list_merge); -bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio, +bool blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio, unsigned int nr_segs) { struct elevator_queue *e = q->elevator; @@ -417,6 +417,9 @@ bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio, bool ret = false; enum hctx_type type; + if (blk_queue_nomerges(q) || !bio_mergeable(bio)) + return false; + if (e && e->type->ops.bio_merge) return e->type->ops.bio_merge(hctx, bio, nr_segs); diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h index 126021f..65151de 100644 --- a/block/blk-mq-sched.h +++ b/block/blk-mq-sched.h @@ -13,8 +13,6 @@ void blk_mq_sched_free_hctx_data(struct request_queue *q, void blk_mq_sched_request_inserted(struct request *rq); bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio, unsigned int nr_segs, struct request **merged_request); -bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio, - unsigned int nr_segs); bool blk_mq_sched_try_insert_merge(struct request_queue *q, struct request *rq); void blk_mq_sched_mark_restart_hctx(struct blk_mq_hw_ctx *hctx); void blk_mq_sched_restart(struct blk_mq_hw_ctx *hctx); @@ -31,15 +29,8 @@ void blk_mq_sched_insert_requests(struct blk_mq_hw_ctx *hctx, void blk_mq_exit_sched(struct request_queue *q, struct elevator_queue *e); void blk_mq_sched_free_requests(struct request_queue *q); -static inline bool -blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio, - unsigned int nr_segs) -{ - if (blk_queue_nomerges(q) || !bio_mergeable(bio)) - return false; - - return __blk_mq_sched_bio_merge(q, bio, nr_segs); -} +bool blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio, + unsigned int nr_segs); static inline bool blk_mq_sched_allow_merge(struct request_queue *q, struct request *rq,
The blk_mq_sched_bio_merge() just wrap the __blk_mq_sched_bio_merge(), and no other places will use __blk_mq_sched_bio_merge(). Thus we can combine these 2 similar functions into one function. Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> --- block/blk-mq-sched.c | 5 ++++- block/blk-mq-sched.h | 13 ++----------- 2 files changed, 6 insertions(+), 12 deletions(-)