diff mbox series

[RFC,3/3] scsi, usb: storage: Complete the blk-request directly.

Message ID 20211015151412.3229037-4-bigeasy@linutronix.de (mailing list archive)
State New, archived
Headers show
Series blk-mq: Allow to complete requests directly | expand

Commit Message

Sebastian Andrzej Siewior Oct. 15, 2021, 3:14 p.m. UTC
The usb-storage driver runs in a thread and completes its request from
that thread. Since it is a single queued device it always schedules
ksoftirqd for its completion.

The completion is performed in the SCSI stack. Add
scsi_done_preemptible() which inlines most of scsi_mq_done() and
completes the request directly via blk_mq_complete_request_direct().

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/scsi/scsi_lib.c   | 17 ++++++++++++++++-
 drivers/usb/storage/usb.c |  2 +-
 include/scsi/scsi_cmnd.h  |  7 +++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

Comments

Christoph Hellwig Oct. 15, 2021, 4:04 p.m. UTC | #1
Bart has been working on removing the ->scsi_done indirection, so this
will need to find a way to interact with that
Sebastian Andrzej Siewior Oct. 15, 2021, 4:16 p.m. UTC | #2
On 2021-10-15 09:04:47 [-0700], Christoph Hellwig wrote:
> Bart has been working on removing the ->scsi_done indirection, so this
> will need to find a way to interact with that

Okay. So I just wait until it is there. Is this v5.15/16 material?

Sebastian
Bart Van Assche Oct. 16, 2021, 9:21 p.m. UTC | #3
On 10/15/21 08:14, Sebastian Andrzej Siewior wrote:
> +static inline void scsi_done(struct scsi_cmnd *scmd)
> +{
> +	scmd->scsi_done(scmd);
> +}

How about leaving out this function definition and open-coding it into 
its callers?

Additionally, please rebase this patch series on top of "[PATCH v3 
00/88] Call scsi_done() directly" 
(https://lore.kernel.org/linux-scsi/20211007202923.2174984-1-bvanassche@acm.org/ 
or https://github.com/bvanassche/linux/tree/scsi-remove-done-callback). 
Otherwise Linus will have to resolve a very complicated merge conflict.

Thank you,

Bart.
Bart Van Assche Oct. 17, 2021, 2:17 a.m. UTC | #4
On 10/15/21 09:16, Sebastian Andrzej Siewior wrote:
> On 2021-10-15 09:04:47 [-0700], Christoph Hellwig wrote:
>> Bart has been working on removing the ->scsi_done indirection, so this
>> will need to find a way to interact with that
> 
> Okay. So I just wait until it is there. Is this v5.15/16 material?

Isn't it too late to submit patches for v5.15 other than bugfixes for 
patches merged during the v5.15 merge window?

Martin Petersen, the SCSI maintainer, has been so kind to queue the 
patch series that removes the scsi_done member for the v5.16 merge 
window. So that patch series should become available soon in the 
following git repository:
git://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git

Thanks,

Bart.
Sebastian Andrzej Siewior Oct. 18, 2021, 10:53 a.m. UTC | #5
On 2021-10-16 19:17:05 [-0700], Bart Van Assche wrote:
> On 10/15/21 09:16, Sebastian Andrzej Siewior wrote:
> > On 2021-10-15 09:04:47 [-0700], Christoph Hellwig wrote:
> > > Bart has been working on removing the ->scsi_done indirection, so this
> > > will need to find a way to interact with that
> > 
> > Okay. So I just wait until it is there. Is this v5.15/16 material?
> 
> Isn't it too late to submit patches for v5.15 other than bugfixes for
> patches merged during the v5.15 merge window?

yeah, off by one, meant 16/17 ;)

> Martin Petersen, the SCSI maintainer, has been so kind to queue the patch
> series that removes the scsi_done member for the v5.16 merge window. So that
> patch series should become available soon in the following git repository:
> git://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git

Thanks.

> Thanks,
> 
> Bart.

Sebastian
Sebastian Andrzej Siewior Oct. 18, 2021, 11:10 a.m. UTC | #6
On 2021-10-16 14:21:37 [-0700], Bart Van Assche wrote:
> On 10/15/21 08:14, Sebastian Andrzej Siewior wrote:
> > +static inline void scsi_done(struct scsi_cmnd *scmd)
> > +{
> > +	scmd->scsi_done(scmd);
> > +}
> 
> How about leaving out this function definition and open-coding it into its
> callers?

Let me reevaluate the situation with your series. I saw that you
provided a scsi_done() function.

> Thank you,
> 
> Bart.

Sebastian
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 572673873ddf8..f0eeedce6b081 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1575,16 +1575,31 @@  static blk_status_t scsi_prepare_cmd(struct request *req)
 	return scsi_cmd_to_driver(cmd)->init_command(cmd);
 }
 
-static void scsi_mq_done(struct scsi_cmnd *cmd)
+static void _scsi_mq_done(struct scsi_cmnd *cmd)
 {
 	if (unlikely(blk_should_fake_timeout(scsi_cmd_to_rq(cmd)->q)))
 		return;
 	if (unlikely(test_and_set_bit(SCMD_STATE_COMPLETE, &cmd->state)))
 		return;
 	trace_scsi_dispatch_cmd_done(cmd);
+}
+
+static void scsi_mq_done(struct scsi_cmnd *cmd)
+{
+	_scsi_mq_done(cmd);
 	blk_mq_complete_request(scsi_cmd_to_rq(cmd));
 }
 
+void scsi_done_preemptible(struct scsi_cmnd *scmd)
+{
+	if (scmd->scsi_done != scsi_mq_done) {
+		scmd->scsi_done(scmd);
+		return;
+	}
+	_scsi_mq_done(scmd);
+	blk_mq_complete_request_direct(scsi_cmd_to_rq(scmd));
+}
+
 static void scsi_mq_put_budget(struct request_queue *q, int budget_token)
 {
 	struct scsi_device *sdev = q->queuedata;
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 90aa9c12ffac5..6ceedd1e14ce7 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -417,7 +417,7 @@  static int usb_stor_control_thread(void * __us)
 		if (srb) {
 			usb_stor_dbg(us, "scsi cmd done, result=0x%x\n",
 					srb->result);
-			srb->scsi_done(srb);
+			scsi_done_preemptible(srb);
 		}
 	} /* for (;;) */
 
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index eaf04c9a1dfcb..e992f2f74dd69 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -396,4 +396,11 @@  static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd)
 extern void scsi_build_sense(struct scsi_cmnd *scmd, int desc,
 			     u8 key, u8 asc, u8 ascq);
 
+static inline void scsi_done(struct scsi_cmnd *scmd)
+{
+	scmd->scsi_done(scmd);
+}
+
+extern void scsi_done_preemptible(struct scsi_cmnd *scmd);
+
 #endif /* _SCSI_SCSI_CMND_H */