Re: [PATCH RESEND 5/5] block: Remove __blk_mq_sched_bio_merge() helper
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.
Re: [PATCH RESEND 5/5] block: Remove __blk_mq_sched_bio_merge() helper
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.
Re: [PATCH RESEND 5/5] block: Remove __blk_mq_sched_bio_merge() helper
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.
Re: [PATCH RESEND 5/5] block: Remove __blk_mq_sched_bio_merge() helper
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.
[PATCH RESEND 5/5] block: Remove __blk_mq_sched_bio_merge() helper
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
---
block/blk-mq-sched.c | 5 -
block/blk-mq-sched.h | 13 ++---
2 files changed, 6 insertions(+), 12 deletions(-)
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,
--
1.8.3.1
