| Message ID | 20170519183016.12646-8-bart.vanassche@sandisk.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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.
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.
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;
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(+)