diff mbox

[v2,1/3] Call scsi_initialize_rq() also for filesystem requests

Message ID 20170830000742.29653-2-bart.vanassche@wdc.com (mailing list archive)
State Superseded
Headers show

Commit Message

Bart Van Assche Aug. 30, 2017, 12:07 a.m. UTC
If a pass-through request is submitted then blk_get_request()
initializes that request by calling scsi_initialize_rq(). Also
call this function for filesystem requests. Introduce
CMD_INITIALIZED to keep track of whether or not a request has
already been initialized.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/scsi/scsi_lib.c  | 28 ++++++++++++++++++++++++----
 include/scsi/scsi_cmnd.h |  3 +++
 2 files changed, 27 insertions(+), 4 deletions(-)

Comments

Christoph Hellwig Aug. 30, 2017, 8:29 a.m. UTC | #1
Looks good except for the subject, which will need a little update:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Bart Van Assche Aug. 30, 2017, 9:13 p.m. UTC | #2
On Wed, 2017-08-30 at 10:29 +0200, Christoph Hellwig wrote:
> Looks good except for the subject, which will need a little update:


Hello Christoph,

Thanks for the review. Sorry but I don't see what's wrong with the subject.
Can you clarify your comment?

Bart.
Brian King Aug. 30, 2017, 10:14 p.m. UTC | #3
On 08/29/2017 07:07 PM, Bart Van Assche wrote:
> If a pass-through request is submitted then blk_get_request()
> initializes that request by calling scsi_initialize_rq(). Also
> call this function for filesystem requests. Introduce
> CMD_INITIALIZED to keep track of whether or not a request has
> already been initialized.

Bart,

I applied these patches and was able to then re-apply your now reverted patch:

270065e92 - scsi-mq: Always unprepare before requeuing a request

With this combination, I was still able to boot successfully on
a Power system with ipr.

Tested-by: Brian King <brking@linux.vnet.ibm.com>

Thanks,

Brian
Bart Van Assche Aug. 30, 2017, 11:03 p.m. UTC | #4
On Wed, 2017-08-30 at 17:14 -0500, Brian King wrote:
> On 08/29/2017 07:07 PM, Bart Van Assche wrote:

> > If a pass-through request is submitted then blk_get_request()

> > initializes that request by calling scsi_initialize_rq(). Also

> > call this function for filesystem requests. Introduce

> > CMD_INITIALIZED to keep track of whether or not a request has

> > already been initialized.

> 

> I applied these patches and was able to then re-apply your now reverted patch:

> 

> 270065e92 - scsi-mq: Always unprepare before requeuing a request

> 

> With this combination, I was still able to boot successfully on

> a Power system with ipr.


Hello Brian,

Thanks for testing! Sorry but a few minutes ago I found a bug in this
patch. I will repost this patch series.

Bart.
Christoph Hellwig Aug. 31, 2017, 9:50 a.m. UTC | #5
On Wed, Aug 30, 2017 at 09:13:00PM +0000, Bart Van Assche wrote:
> On Wed, 2017-08-30 at 10:29 +0200, Christoph Hellwig wrote:
> > Looks good except for the subject, which will need a little update:
> 
> Hello Christoph,
> 
> Thanks for the review. Sorry but I don't see what's wrong with the subject.
> Can you clarify your comment?

Hi Bart,

I'll take that comment back - I misread scsi_initialize_rq as
.initialize_rq.
diff mbox

Patch

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 938a7e398cd4..170a725e33ce 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -642,6 +642,11 @@  static bool scsi_end_request(struct request *req, blk_status_t error,
 	if (blk_queue_add_random(q))
 		add_disk_randomness(req->rq_disk);
 
+	if (!blk_rq_is_scsi(req)) {
+		WARN_ON_ONCE(!(cmd->flags & SCMD_INITIALIZED));
+		cmd->flags &= ~SCMD_INITIALIZED;
+	}
+
 	if (req->mq_ctx) {
 		/*
 		 * In the MQ case the command gets freed by __blk_mq_end_request,
@@ -1110,7 +1115,8 @@  EXPORT_SYMBOL(scsi_init_io);
  * scsi_initialize_rq - initialize struct scsi_cmnd.req
  * @rq: Request associated with the SCSI command to be initialized.
  *
- * Called from inside blk_get_request().
+ * Called from inside blk_get_request() for pass-through requests and from
+ * inside scsi_init_command() for filesystem requests.
  */
 void scsi_initialize_rq(struct request *rq)
 {
@@ -1154,7 +1160,13 @@  void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
 {
 	void *buf = cmd->sense_buffer;
 	void *prot = cmd->prot_sdb;
-	unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA;
+	struct request *rq = blk_mq_rq_from_pdu(cmd);
+	unsigned int flags = cmd->flags & SCMD_PRESERVED_FLAGS;
+
+	if (!blk_rq_is_scsi(rq) && !(flags & SCMD_INITIALIZED)) {
+		flags |= SCMD_INITIALIZED;
+		scsi_initialize_rq(rq);
+	}
 
 	/* zero out the cmd, except for the embedded scsi_request */
 	memset((char *)cmd + sizeof(cmd->req), 0,
@@ -1163,7 +1175,7 @@  void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
 	cmd->device = dev;
 	cmd->sense_buffer = buf;
 	cmd->prot_sdb = prot;
-	cmd->flags = unchecked_isa_dma;
+	cmd->flags = flags;
 	INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler);
 	cmd->jiffies_at_alloc = jiffies;
 
@@ -1350,6 +1362,8 @@  static int scsi_prep_fn(struct request_queue *q, struct request *req)
 
 	ret = scsi_setup_cmnd(sdev, req);
 out:
+	if (ret != BLKPREP_OK)
+		cmd->flags &= ~SCMD_INITIALIZED;
 	return scsi_prep_return(q, req, ret);
 }
 
@@ -1869,6 +1883,7 @@  static int scsi_mq_prep_fn(struct request *req)
 	struct scsi_device *sdev = req->q->queuedata;
 	struct Scsi_Host *shost = sdev->host;
 	struct scatterlist *sg;
+	int ret;
 
 	scsi_init_command(sdev, cmd);
 
@@ -1902,7 +1917,10 @@  static int scsi_mq_prep_fn(struct request *req)
 
 	blk_mq_start_request(req);
 
-	return scsi_setup_cmnd(sdev, req);
+	ret = scsi_setup_cmnd(sdev, req);
+	if (ret != BLK_STS_OK)
+		cmd->flags &= ~SCMD_INITIALIZED;
+	return ret;
 }
 
 static void scsi_mq_done(struct scsi_cmnd *cmd)
@@ -1938,6 +1956,8 @@  static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
 		goto out_dec_target_busy;
 
 	if (!(req->rq_flags & RQF_DONTPREP)) {
+		if (!blk_rq_is_scsi(req))
+			WARN_ON_ONCE(cmd->flags & SCMD_INITIALIZED);
 		ret = prep_to_mq(scsi_mq_prep_fn(req));
 		if (ret != BLK_STS_OK)
 			goto out_dec_host_busy;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index f5afcff8d76f..a9f8f7e79d83 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -57,6 +57,9 @@  struct scsi_pointer {
 /* for scmd->flags */
 #define SCMD_TAGGED		(1 << 0)
 #define SCMD_UNCHECKED_ISA_DMA	(1 << 1)
+#define SCMD_INITIALIZED	(1 << 3)
+/* flags preserved across unprep / reprep */
+#define SCMD_PRESERVED_FLAGS	(SCMD_UNCHECKED_ISA_DMA | SCMD_INITIALIZED)
 
 struct scsi_cmnd {
 	struct scsi_request req;