[07/18] block: Introduce request_queue.initialize_rq_fn()
diff mbox

Message ID 20170519183016.12646-8-bart.vanassche@sandisk.com
State New
Headers show

Commit Message

Bart Van Assche May 19, 2017, 6:30 p.m. UTC
Several block drivers need to initialize the driver-private data
after having called blk_get_request() and before .prep_rq_fn() is
called, e.g. when submitting a REQ_OP_SCSI_* request. Avoid that
that initialization code has to be repeated after every
blk_get_request() call by adding a new callback function to struct
request_queue.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Jens Axboe <axboe@fb.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: linux-block@vger.kernel.org
---
 block/blk-core.c       | 3 +++
 block/blk-mq.c         | 3 +++
 include/linux/blkdev.h | 4 ++++
 3 files changed, 10 insertions(+)

Comments

Christoph Hellwig May 21, 2017, 6:34 a.m. UTC | #1
On Fri, May 19, 2017 at 11:30:05AM -0700, Bart Van Assche wrote:
> Several block drivers need to initialize the driver-private data
> after having called blk_get_request() and before .prep_rq_fn() is
> called, e.g. when submitting a REQ_OP_SCSI_* request. Avoid that
> that initialization code has to be repeated after every
> blk_get_request() call by adding a new callback function to struct
> request_queue.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Jens Axboe <axboe@fb.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: linux-block@vger.kernel.org
> ---
>  block/blk-core.c       | 3 +++
>  block/blk-mq.c         | 3 +++
>  include/linux/blkdev.h | 4 ++++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index a69d420b7ff0..f2540d164679 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -126,6 +126,9 @@ void blk_rq_init(struct request_queue *q, struct request *rq)
>  	rq->start_time = jiffies;
>  	set_start_time_ns(rq);
>  	rq->part = NULL;
> +
> +	if (q->initialize_rq_fn)
> +		q->initialize_rq_fn(rq);

Can we keep this out of the fast path and only do it from the
blk_get_request / blk_mq_alloc_request path?  And while were at it
I think those two should be merged as far as the public interface
goes, that is we should also expose the flags argument for the
legacy path.
Bart Van Assche May 22, 2017, 5:07 p.m. UTC | #2
On Sun, 2017-05-21 at 08:34 +0200, Christoph Hellwig wrote:
> On Fri, May 19, 2017 at 11:30:05AM -0700, Bart Van Assche wrote:
> > Several block drivers need to initialize the driver-private data
> > after having called blk_get_request() and before .prep_rq_fn() is
> > called, e.g. when submitting a REQ_OP_SCSI_* request. Avoid that
> > that initialization code has to be repeated after every
> > blk_get_request() call by adding a new callback function to struct
> > request_queue.
> > 
> > Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> > Cc: Jens Axboe <axboe@fb.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Omar Sandoval <osandov@fb.com>
> > Cc: Hannes Reinecke <hare@suse.com>
> > Cc: linux-block@vger.kernel.org
> > ---
> >  block/blk-core.c       | 3 +++
> >  block/blk-mq.c         | 3 +++
> >  include/linux/blkdev.h | 4 ++++
> >  3 files changed, 10 insertions(+)
> > 
> > diff --git a/block/blk-core.c b/block/blk-core.c
> > index a69d420b7ff0..f2540d164679 100644
> > --- a/block/blk-core.c
> > +++ b/block/blk-core.c
> > @@ -126,6 +126,9 @@ void blk_rq_init(struct request_queue *q, struct request *rq)
> >  	rq->start_time = jiffies;
> >  	set_start_time_ns(rq);
> >  	rq->part = NULL;
> > +
> > +	if (q->initialize_rq_fn)
> > +		q->initialize_rq_fn(rq);
> 
> Can we keep this out of the fast path and only do it from the
> blk_get_request / blk_mq_alloc_request path?  And while were at it
> I think those two should be merged as far as the public interface
> goes, that is we should also expose the flags argument for the
> legacy path.

Hello Christoph,

For blk-mq I could move the .initialize_rq_fn() call from
blk_mq_rq_ctx_init() into blk_mq_sched_get_request(). I can't move it
higher up in the request allocation call chain because otherwise
blk_mq_alloc_request_hctx() wouldn't call .initialize_rq_fn().

For blk-sq I'm not sure that the .initialize_rq_fn() call can be
moved because I'd like all blk_rq_init() callers to call
.initialize_rq_fn(), including ide_prep_sense().

Are you sure it would help to move the .initialize_rq_fn() calls?

Thanks,

Bart.

Patch
diff mbox

diff --git a/block/blk-core.c b/block/blk-core.c
index a69d420b7ff0..f2540d164679 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -126,6 +126,9 @@  void blk_rq_init(struct request_queue *q, struct request *rq)
 	rq->start_time = jiffies;
 	set_start_time_ns(rq);
 	rq->part = NULL;
+
+	if (q->initialize_rq_fn)
+		q->initialize_rq_fn(rq);
 }
 EXPORT_SYMBOL(blk_rq_init);
 
diff --git a/block/blk-mq.c b/block/blk-mq.c
index a69ad122ed66..2af43d4e5b96 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -241,6 +241,9 @@  void blk_mq_rq_ctx_init(struct request_queue *q, struct blk_mq_ctx *ctx,
 	rq->end_io_data = NULL;
 	rq->next_rq = NULL;
 
+	if (q->initialize_rq_fn)
+		q->initialize_rq_fn(rq);
+
 	ctx->rq_dispatched[op_is_sync(op)]++;
 }
 EXPORT_SYMBOL_GPL(blk_mq_rq_ctx_init);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 75b71374e1ba..2ee8da93619d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -410,8 +410,12 @@  struct request_queue {
 	rq_timed_out_fn		*rq_timed_out_fn;
 	dma_drain_needed_fn	*dma_drain_needed;
 	lld_busy_fn		*lld_busy_fn;
+	/* Called just after a request is allocated */
 	init_rq_fn		*init_rq_fn;
+	/* Called just before a request is freed */
 	exit_rq_fn		*exit_rq_fn;
+	/* Called from inside blk_get_request() */
+	void (*initialize_rq_fn)(struct request *rq);
 
 	const struct blk_mq_ops	*mq_ops;