Skip to contentSkip to conversation

Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE

PATCH In review Boaz Harrosh 7 participants 22 messages Sep 10, 2007 – Oct 8, 2007

Subsystems (5)
SCSI USB USB mass storage driver Arm/riscpc architecture +1 more subsystems
lore
Share & tools
lore

Boaz Harrosh

author
#0 lore raw

cover Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE

Thank you Christoph, Randy, Alen for your comments. Here is ver 2 incorporating all your suggestions.

Quoted 5 lines · Christoph Hellwig

I think just struct "struct scsi_eh_save *save" is descriptive enough and almost fits on a line as well.. Also continuation of the prototype is indented by two tabs normally.
I think you can kill the old prefixes in the struct, they're saved per defintion.

Done

Quoted 3 lines · Alan Stern

A trivial problem is that quilt complains about patch 3, which leaves extra whitespace at the end of line 591 in transport.c.

Thanks

Quoted 4 lines

More seriously, why make the caller define a static generic_sense array? Why not put the array in scsi_eh_prep_cmnd(), where it can be used whenever copy_sense is nonzero?

I hope you like my solution

Quoted 3 lines

The line initializing the sense buffer to zero should be inside the copy_sense conditional block.

Thanks

Quoted 4 lines

The code setting the LUN bits in scmd[1] is wrong. It should be like the code in scsi_dispatch_cmd(); i.e., those bits should not be set if the scsi_level is SCSI_UNKNOWN.

Thanks, good catch

Quoted 6 lines

Finally, a really serious problem. The code sets the buffer length for the REQUEST SENSE command to be the length of scmd->sense_buffer, which is 96. But many USB devices won't work properly unless the requesed sense data length is 18. The appropriate adjustment must be added to transport.c in patch 3.

I was afraid of that at the beginning. But testing both with a Seagate USB sata hard-disk and Sandisk 2Gb usb-stick. They both returned a short read of 56 but other wise were happy. So I thought it is leftovers from The time the sense buffer was driver allocated. Also the standard recommends a short-read behavior. But I have taken your advise to hart and changed the code accordingly. Please check me out.

----------------------------------------------------------------

In motivation to abstract scsi_cmnd members and insulate drivers/transports from scsi_cmnd internals. The last place left was the REQUEST_SENSE sequence when done synchronous, by drivers.

Also all these drivers would do a use_sg==0 command invocation, preventing from doing cleanups on these drivers/transports. So this patchset is left-overs from Christoph's 2.6.18 cleanups.

In this patchset I have re-factored scsi_error to make it possible for drivers to use an abstract (easy to use, I hope) API for invoking a REQUEST_SENSE command. The API is declared in scsi/scsi_eh.h

[patch 1/5] scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()
Move some code around and add new fixtures here. So next patch is left a Mechanical breakup of code.

[patch 2/5] scsi_error: Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE
Here new API is introduced in scsi/scsi_eh.h and mechanical move of code to the new functions.

[PATCH 3/5] usb: transport.c use scsi_eh API in REQUEST_SENSE execution
Use above API for drivers/usb/storage/transport.c. Now that last User of use_sg==0, we can do the USB storage cleanups.

[PATCH 4/5] NCR5380: Use scsi_eh API for REQUEST_SENSE invocation
All NCR5380 family of drivers used to send a REQUEST_SENSE command.

[PATCH 5/5] arm: fas216 Use scsi_eh API for REQUEST_SENSE invocation
drivers/scsi/arm/fas216.c need change also.

Please Test as much as possible, and report any problems/differences.

Boaz Harrosh

author
#1 lore raw

PATCH 1/5 scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()

- regrouped variables for easier reviewing of next patch
- Support of cmnd==NULL in call to scsi_send_eh_cmnd()
- In the copy_sense case set transfer size to the minimum size of sense_buffer and passed copy_sense. cmnd[4] is set accordingly.
- REQUEST_SENSE is set into cmnd[0] so if copy_sense is not Zero passed cmnd can/should be NULL.
- Also save/restore resid of faild command.

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/scsi_error.c |   63 +++++++++++++++++++++++++--------------------
 1 files changed, 35 insertions(+), 28 deletions(-)
drivers/scsi/scsi_error.c +35 -28
index c8e351f..20a72aa 100644
@@ -607,21 +607,24 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
  *    SUCCESS or FAILED or NEEDS_RETRY
  **/
 static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
-			     int cmnd_size, int timeout, int copy_sense)
+			     int cmnd_size, int timeout, unsigned copy_sense)
 {
 	struct scsi_device *sdev = scmd->device;
 	struct Scsi_Host *shost = sdev->host;
-	int old_result = scmd->result;
 	DECLARE_COMPLETION_ONSTACK(done);
 	unsigned long timeleft;
 	unsigned long flags;
-	struct scatterlist sgl;
+
+	unsigned char old_cmd_len;
 	unsigned char old_cmnd[MAX_COMMAND_SIZE];
 	enum dma_data_direction old_data_direction;
-	unsigned short old_use_sg;
-	unsigned char old_cmd_len;
 	unsigned old_bufflen;
 	void *old_buffer;
+	unsigned short old_use_sg;
+	int old_resid;
+	int old_result;
+
+	struct scatterlist sgl;
 	int rtn;
 
 	/*

@@ -631,24 +634,37 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	 * we will need to restore these values prior to running the actual
 	 * command.
 	 */
-	old_buffer = scmd->request_buffer;
-	old_bufflen = scmd->request_bufflen;
+	old_cmd_len = scmd->cmd_len;
 	memcpy(old_cmnd, scmd->cmnd, sizeof(scmd->cmnd));
 	old_data_direction = scmd->sc_data_direction;
-	old_cmd_len = scmd->cmd_len;
+	old_bufflen = scmd->request_bufflen;
+	old_buffer = scmd->request_buffer;
 	old_use_sg = scmd->use_sg;
+	old_resid = scmd->resid;
+	old_result = scmd->result;
 
-	memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
-	memcpy(scmd->cmnd, cmnd, cmnd_size);
+	if (cmnd) {
+		memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
+		memcpy(scmd->cmnd, cmnd, cmnd_size);
+		scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
+	}
 
 	if (copy_sense) {
 		sg_init_one(&sgl, scmd->sense_buffer,
 			    sizeof(scmd->sense_buffer));
-
-		scmd->sc_data_direction = DMA_FROM_DEVICE;
-		scmd->request_bufflen = sgl.length;
 		scmd->request_buffer = &sgl;
+		scmd->sc_data_direction = DMA_FROM_DEVICE;
+		scmd->request_bufflen = min_t(unsigned,
+		                       sizeof(scmd->sense_buffer), copy_sense);
 		scmd->use_sg = 1;
+		memset(scmd->cmnd, 0, 6);
+		scmd->cmnd[0] = REQUEST_SENSE;
+		scmd->cmnd[4] = scmd->request_bufflen;
+		/*
+		 * Zero the sense buffer.  The scsi spec mandates that any
+		 * untransferred sense data should be interpreted as being zero.
+		 */
+		memset(scmd->sense_buffer, 0, sizeof(scmd->sense_buffer));
 	} else {
 		scmd->request_buffer = NULL;
 		scmd->request_bufflen = 0;

@@ -657,18 +673,11 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	}
 
 	scmd->underflow = 0;
-	scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
 
-	if (sdev->scsi_level <= SCSI_2)
+	if (sdev->scsi_level <= SCSI_2 && sdev->scsi_level != SCSI_UNKNOWN)
 		scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
 			(sdev->lun << 5 & 0xe0);
 
-	/*
-	 * Zero the sense buffer.  The scsi spec mandates that any
-	 * untransferred sense data should be interpreted as being zero.
-	 */
-	memset(scmd->sense_buffer, 0, sizeof(scmd->sense_buffer));
-
 	shost->eh_action = &done;
 
 	spin_lock_irqsave(shost->host_lock, flags);

@@ -716,12 +725,13 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	/*
 	 * Restore original data
 	 */
-	scmd->request_buffer = old_buffer;
-	scmd->request_bufflen = old_bufflen;
+	scmd->cmd_len = old_cmd_len;
 	memcpy(scmd->cmnd, old_cmnd, sizeof(scmd->cmnd));
 	scmd->sc_data_direction = old_data_direction;
-	scmd->cmd_len = old_cmd_len;
+	scmd->request_bufflen = old_bufflen;
+	scmd->request_buffer = old_buffer;
 	scmd->use_sg = old_use_sg;
+	scmd->resid = old_resid;
 	scmd->result = old_result;
 	return rtn;
 }

@@ -737,10 +747,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
  **/
 static int scsi_request_sense(struct scsi_cmnd *scmd)
 {
-	static unsigned char generic_sense[6] =
-		{REQUEST_SENSE, 0, 0, 0, 252, 0};
-
-	return scsi_send_eh_cmnd(scmd, generic_sense, 6, SENSE_TIMEOUT, 1);
+	return scsi_send_eh_cmnd(scmd, NULL, 0, SENSE_TIMEOUT, ~0);
 }
 
 /**
Signature · 1 line
1.5.3.1
author
#2 lore raw

PATCH 1/5 scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()

- regrouped variables for easier reviewing of next patch
- Support of cmnd==NULL in call to scsi_send_eh_cmnd()
- In the copy_sense case set transfer size to the minimum size of sense_buffer and passed @sense_bytes. cmnd[4] is set accordingly.
- REQUEST_SENSE is set into cmnd[0] so if @sense_bytes is not Zero passed cmnd can/should be NULL.
- Also save/restore resid of faild command.

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/scsi_error.c |   68 ++++++++++++++++++++++++--------------------
 1 files changed, 37 insertions(+), 31 deletions(-)
drivers/scsi/scsi_error.c +37 -31
index c8e351f..46d57f1 100644
@@ -607,21 +607,24 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
  *    SUCCESS or FAILED or NEEDS_RETRY
  **/
 static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
-			     int cmnd_size, int timeout, int copy_sense)
+			     int cmnd_size, int timeout, unsigned sense_bytes)
 {
 	struct scsi_device *sdev = scmd->device;
 	struct Scsi_Host *shost = sdev->host;
-	int old_result = scmd->result;
 	DECLARE_COMPLETION_ONSTACK(done);
 	unsigned long timeleft;
 	unsigned long flags;
-	struct scatterlist sgl;
+
+	unsigned char old_cmd_len;
 	unsigned char old_cmnd[MAX_COMMAND_SIZE];
 	enum dma_data_direction old_data_direction;
-	unsigned short old_use_sg;
-	unsigned char old_cmd_len;
 	unsigned old_bufflen;
 	void *old_buffer;
+	unsigned short old_use_sg;
+	int old_resid;
+	int old_result;
+
+	struct scatterlist sgl;
 	int rtn;
 
 	/*

@@ -631,24 +634,36 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	 * we will need to restore these values prior to running the actual
 	 * command.
 	 */
-	old_buffer = scmd->request_buffer;
-	old_bufflen = scmd->request_bufflen;
+	old_cmd_len = scmd->cmd_len;
 	memcpy(old_cmnd, scmd->cmnd, sizeof(scmd->cmnd));
 	old_data_direction = scmd->sc_data_direction;
-	old_cmd_len = scmd->cmd_len;
+	old_bufflen = scmd->request_bufflen;
+	old_buffer = scmd->request_buffer;
 	old_use_sg = scmd->use_sg;
+	old_resid = scmd->resid;
+	old_result = scmd->result;
 
-	memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
-	memcpy(scmd->cmnd, cmnd, cmnd_size);
-
-	if (copy_sense) {
-		sg_init_one(&sgl, scmd->sense_buffer,
-			    sizeof(scmd->sense_buffer));
+	if (cmnd) {
+		memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
+		memcpy(scmd->cmnd, cmnd, cmnd_size);
+		scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
+	}
 
-		scmd->sc_data_direction = DMA_FROM_DEVICE;
-		scmd->request_bufflen = sgl.length;
+	if (sense_bytes) {
+		scmd->request_bufflen = min_t(unsigned,
+		                       sizeof(scmd->sense_buffer), sense_bytes);
+		sg_init_one(&sgl, scmd->sense_buffer, scmd->request_bufflen);
 		scmd->request_buffer = &sgl;
+		scmd->sc_data_direction = DMA_FROM_DEVICE;
 		scmd->use_sg = 1;
+		memset(scmd->cmnd, 0, 6);
+		scmd->cmnd[0] = REQUEST_SENSE;
+		scmd->cmnd[4] = scmd->request_bufflen;
+		/*
+		 * Zero the sense buffer.  The scsi spec mandates that any
+		 * untransferred sense data should be interpreted as being zero.
+		 */
+		memset(scmd->sense_buffer, 0, sizeof(scmd->sense_buffer));
 	} else {
 		scmd->request_buffer = NULL;
 		scmd->request_bufflen = 0;

@@ -657,18 +672,11 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	}
 
 	scmd->underflow = 0;
-	scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
 
-	if (sdev->scsi_level <= SCSI_2)
+	if (sdev->scsi_level <= SCSI_2 && sdev->scsi_level != SCSI_UNKNOWN)
 		scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
 			(sdev->lun << 5 & 0xe0);
 
-	/*
-	 * Zero the sense buffer.  The scsi spec mandates that any
-	 * untransferred sense data should be interpreted as being zero.
-	 */
-	memset(scmd->sense_buffer, 0, sizeof(scmd->sense_buffer));
-
 	shost->eh_action = &done;
 
 	spin_lock_irqsave(shost->host_lock, flags);

@@ -716,12 +724,13 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	/*
 	 * Restore original data
 	 */
-	scmd->request_buffer = old_buffer;
-	scmd->request_bufflen = old_bufflen;
+	scmd->cmd_len = old_cmd_len;
 	memcpy(scmd->cmnd, old_cmnd, sizeof(scmd->cmnd));
 	scmd->sc_data_direction = old_data_direction;
-	scmd->cmd_len = old_cmd_len;
+	scmd->request_bufflen = old_bufflen;
+	scmd->request_buffer = old_buffer;
 	scmd->use_sg = old_use_sg;
+	scmd->resid = old_resid;
 	scmd->result = old_result;
 	return rtn;
 }

@@ -737,10 +746,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
  **/
 static int scsi_request_sense(struct scsi_cmnd *scmd)
 {
-	static unsigned char generic_sense[6] =
-		{REQUEST_SENSE, 0, 0, 0, 252, 0};
-
-	return scsi_send_eh_cmnd(scmd, generic_sense, 6, SENSE_TIMEOUT, 1);
+	return scsi_send_eh_cmnd(scmd, NULL, 0, SENSE_TIMEOUT, ~0);
 }
 
 /**
Signature · 1 line
1.5.3.1
#3 lore raw

Re: [PATCH ver3 1/5] scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()

(added CCs - that's what you get for sending emails after 5.)

drivers/scsi/scsi_error.c static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd) · line 610

Quoted 4 lines · Boaz Harrosh

static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,

drivers/scsi/scsi_error.c +1 -1
-                            int cmnd_size, int timeout, int copy_sense)
+                            int cmnd_size, int timeout, unsigned sense_bytes)

Shouldn't that be unsigned _int_?

Thanks,

--

Julian Calaby

Email: [email protected]

#4 lore raw

Re: [PATCH ver3 1/5] scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()

Quoted 8 lines · Julian Calaby

(added CCs - that's what you get for sending emails after 5.)

drivers/scsi/scsi_error.c static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd) · line 610

Quoted 3 lines · Boaz Harrosh

static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,

drivers/scsi/scsi_error.c +1 -1
-                            int cmnd_size, int timeout, int copy_sense)
+                            int cmnd_size, int timeout, unsigned sense_bytes)

Shouldn't that be unsigned _int_?

It could but first, "unsigned" and "unsigned int" are the same type and second, this is consistent with *bufflen type which is "unsigned" all over the place.

Benny

Quoted 12 lines

Thanks,

--

Julian Calaby

Email: [email protected]
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer
#5 lore raw

Re: [PATCH ver3 1/5] scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()

In reply to Boaz Harrosh
Quoted 8 lines · Boaz Harrosh

- regrouped variables for easier reviewing of next patch
- Support of cmnd==NULL in call to scsi_send_eh_cmnd()
- In the copy_sense case set transfer size to the minimum size of sense_buffer and passed @sense_bytes. cmnd[4] is set accordingly.
- REQUEST_SENSE is set into cmnd[0] so if @sense_bytes is not Zero passed cmnd can/should be NULL.
- Also save/restore resid of faild command.

The "if (sense_bytes)" block doesn't set scmd->cmd_len. Yes, I know that value gets changed by the usb-storage driver, but nevertheless it should be set here to 6.

Apart from that, this looks fine to me.

Alan Stern

author
#6 lore raw

Re: [PATCH ver3 1/5] scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()

Quoted 11 lines · Alan Stern
Quoted 8 lines · Boaz Harrosh

- regrouped variables for easier reviewing of next patch
- Support of cmnd==NULL in call to scsi_send_eh_cmnd()
- In the copy_sense case set transfer size to the minimum size of sense_buffer and passed @sense_bytes. cmnd[4] is set accordingly.
- REQUEST_SENSE is set into cmnd[0] so if @sense_bytes is not Zero passed cmnd can/should be NULL.
- Also save/restore resid of faild command.

Quoted 8 lines

The "if (sense_bytes)" block doesn't set scmd->cmd_len. Yes, I know that value gets changed by the usb-storage driver, but nevertheless it should be set here to 6.

Apart from that, this looks fine to me.

Alan Stern

Thanks a million. The 22:45 effect again.
Will patch ver4 of this patch as reply to
first one.

Boaz

author
#7 lore raw

Re: [PATCH ver4 1/5] scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()

In reply to Boaz Harrosh

- regrouped variables for easier reviewing of next patch
- Support of cmnd==NULL in call to scsi_send_eh_cmnd()
- In the copy_sense case set transfer size to the minimum size of sense_buffer and passed @sense_bytes. cmnd[4] is set accordingly.
- REQUEST_SENSE is set into cmnd[0] so if @sense_bytes is not Zero passed cmnd can/should be NULL.
- Also save/restore resid of faild command.

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/scsi_error.c |   69 ++++++++++++++++++++++++--------------------
 1 files changed, 38 insertions(+), 31 deletions(-)
drivers/scsi/scsi_error.c +38 -31
index c8e351f..11c928d 100644
@@ -607,21 +607,24 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
  *    SUCCESS or FAILED or NEEDS_RETRY
  **/
 static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
-			     int cmnd_size, int timeout, int copy_sense)
+			     int cmnd_size, int timeout, unsigned sense_bytes)
 {
 	struct scsi_device *sdev = scmd->device;
 	struct Scsi_Host *shost = sdev->host;
-	int old_result = scmd->result;
 	DECLARE_COMPLETION_ONSTACK(done);
 	unsigned long timeleft;
 	unsigned long flags;
-	struct scatterlist sgl;
+
+	unsigned char old_cmd_len;
 	unsigned char old_cmnd[MAX_COMMAND_SIZE];
 	enum dma_data_direction old_data_direction;
-	unsigned short old_use_sg;
-	unsigned char old_cmd_len;
 	unsigned old_bufflen;
 	void *old_buffer;
+	unsigned short old_use_sg;
+	int old_resid;
+	int old_result;
+
+	struct scatterlist sgl;
 	int rtn;
 
 	/*

@@ -631,24 +634,37 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	 * we will need to restore these values prior to running the actual
 	 * command.
 	 */
-	old_buffer = scmd->request_buffer;
-	old_bufflen = scmd->request_bufflen;
+	old_cmd_len = scmd->cmd_len;
 	memcpy(old_cmnd, scmd->cmnd, sizeof(scmd->cmnd));
 	old_data_direction = scmd->sc_data_direction;
-	old_cmd_len = scmd->cmd_len;
+	old_bufflen = scmd->request_bufflen;
+	old_buffer = scmd->request_buffer;
 	old_use_sg = scmd->use_sg;
+	old_resid = scmd->resid;
+	old_result = scmd->result;
 
-	memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
-	memcpy(scmd->cmnd, cmnd, cmnd_size);
-
-	if (copy_sense) {
-		sg_init_one(&sgl, scmd->sense_buffer,
-			    sizeof(scmd->sense_buffer));
+	if (cmnd) {
+		memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
+		memcpy(scmd->cmnd, cmnd, cmnd_size);
+		scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
+	}
 
-		scmd->sc_data_direction = DMA_FROM_DEVICE;
-		scmd->request_bufflen = sgl.length;
+	if (sense_bytes) {
+		scmd->request_bufflen = min_t(unsigned,
+		                       sizeof(scmd->sense_buffer), sense_bytes);
+		sg_init_one(&sgl, scmd->sense_buffer, scmd->request_bufflen);
 		scmd->request_buffer = &sgl;
+		scmd->sc_data_direction = DMA_FROM_DEVICE;
 		scmd->use_sg = 1;
+		memset(scmd->cmnd, 0, 6);
+		scmd->cmnd[0] = REQUEST_SENSE;
+		scmd->cmnd[4] = scmd->request_bufflen;
+		scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
+		/*
+		 * Zero the sense buffer.  The scsi spec mandates that any
+		 * untransferred sense data should be interpreted as being zero.
+		 */
+		memset(scmd->sense_buffer, 0, sizeof(scmd->sense_buffer));
 	} else {
 		scmd->request_buffer = NULL;
 		scmd->request_bufflen = 0;

@@ -657,18 +673,11 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	}
 
 	scmd->underflow = 0;
-	scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
 
-	if (sdev->scsi_level <= SCSI_2)
+	if (sdev->scsi_level <= SCSI_2 && sdev->scsi_level != SCSI_UNKNOWN)
 		scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
 			(sdev->lun << 5 & 0xe0);
 
-	/*
-	 * Zero the sense buffer.  The scsi spec mandates that any
-	 * untransferred sense data should be interpreted as being zero.
-	 */
-	memset(scmd->sense_buffer, 0, sizeof(scmd->sense_buffer));
-
 	shost->eh_action = &done;
 
 	spin_lock_irqsave(shost->host_lock, flags);

@@ -716,12 +725,13 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	/*
 	 * Restore original data
 	 */
-	scmd->request_buffer = old_buffer;
-	scmd->request_bufflen = old_bufflen;
+	scmd->cmd_len = old_cmd_len;
 	memcpy(scmd->cmnd, old_cmnd, sizeof(scmd->cmnd));
 	scmd->sc_data_direction = old_data_direction;
-	scmd->cmd_len = old_cmd_len;
+	scmd->request_bufflen = old_bufflen;
+	scmd->request_buffer = old_buffer;
 	scmd->use_sg = old_use_sg;
+	scmd->resid = old_resid;
 	scmd->result = old_result;
 	return rtn;
 }

@@ -737,10 +747,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
  **/
 static int scsi_request_sense(struct scsi_cmnd *scmd)
 {
-	static unsigned char generic_sense[6] =
-		{REQUEST_SENSE, 0, 0, 0, 252, 0};
-
-	return scsi_send_eh_cmnd(scmd, generic_sense, 6, SENSE_TIMEOUT, 1);
+	return scsi_send_eh_cmnd(scmd, NULL, 0, SENSE_TIMEOUT, ~0);
 }
 
 /**
Signature · 1 line
1.5.3.1
#8 lore raw

Re: [PATCH ver4 1/5] scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()

Quoted 19 lines · Boaz Harrosh

- regrouped variables for easier reviewing of next patch
- Support of cmnd==NULL in call to scsi_send_eh_cmnd()
- In the copy_sense case set transfer size to the minimum size of sense_buffer and passed @sense_bytes. cmnd[4] is set accordingly.
- REQUEST_SENSE is set into cmnd[0] so if @sense_bytes is not Zero passed cmnd can/should be NULL.
- Also save/restore resid of faild command.

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/scsi_error.c |   69 ++++++++++++++++++++++++--------------------
 1 files changed, 38 insertions(+), 31 deletions(-)
drivers/scsi/scsi_error.c +0 -0
index c8e351f..11c928d 100644

drivers/scsi/scsi_error.c static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd) · line 610

Quoted 5 lines
drivers/scsi/scsi_error.c +1 -1
@@ -607,21 +607,24 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
  *    SUCCESS or FAILED or NEEDS_RETRY
  **/
 static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
-			     int cmnd_size, int timeout, int copy_sense)
+			     int cmnd_size, int timeout, unsigned sense_bytes)

You've altered the signature and function of the routine here, this needs to be documented by updating the docbook description above the function. (I know you do it later, but each changeset should really be logically complete).

Quoted 52 lines
drivers/scsi/scsi_error.c +17 -13
 {
 	struct scsi_device *sdev = scmd->device;
 	struct Scsi_Host *shost = sdev->host;
-	int old_result = scmd->result;
 	DECLARE_COMPLETION_ONSTACK(done);
 	unsigned long timeleft;
 	unsigned long flags;
-	struct scatterlist sgl;
+
+	unsigned char old_cmd_len;
 	unsigned char old_cmnd[MAX_COMMAND_SIZE];
 	enum dma_data_direction old_data_direction;
-	unsigned short old_use_sg;
-	unsigned char old_cmd_len;
 	unsigned old_bufflen;
 	void *old_buffer;
+	unsigned short old_use_sg;
+	int old_resid;
+	int old_result;
+
+	struct scatterlist sgl;
 	int rtn;
 
 	/*

@@ -631,24 +634,37 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	 * we will need to restore these values prior to running the actual
 	 * command.
 	 */
-	old_buffer = scmd->request_buffer;
-	old_bufflen = scmd->request_bufflen;
+	old_cmd_len = scmd->cmd_len;
 	memcpy(old_cmnd, scmd->cmnd, sizeof(scmd->cmnd));
 	old_data_direction = scmd->sc_data_direction;
-	old_cmd_len = scmd->cmd_len;
+	old_bufflen = scmd->request_bufflen;
+	old_buffer = scmd->request_buffer;
 	old_use_sg = scmd->use_sg;
+	old_resid = scmd->resid;
+	old_result = scmd->result;
 
-	memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
-	memcpy(scmd->cmnd, cmnd, cmnd_size);
-
-	if (copy_sense) {
-		sg_init_one(&sgl, scmd->sense_buffer,
-			    sizeof(scmd->sense_buffer));
+	if (cmnd) {
+		memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
+		memcpy(scmd->cmnd, cmnd, cmnd_size);
+		scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
+	}

drivers/scsi/scsi_error.c static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd, · line 659

Quoted 10 lines
drivers/scsi/scsi_error.c +6 -2
-		scmd->sc_data_direction = DMA_FROM_DEVICE;
-		scmd->request_bufflen = sgl.length;
+	if (sense_bytes) {
+		scmd->request_bufflen = min_t(unsigned,
+		                       sizeof(scmd->sense_buffer), sense_bytes);
+		sg_init_one(&sgl, scmd->sense_buffer, scmd->request_bufflen);
 		scmd->request_buffer = &sgl;
+		scmd->sc_data_direction = DMA_FROM_DEVICE;
 		scmd->use_sg = 1;
+		memset(scmd->cmnd, 0, 6);

This will lead to subtle bugs: some devices (notably ATAPI) have a fixed command slot they will copy this into. You have potentially trailing bytes of junk that this could pick up ... we have ATAPI devices known to fall over if they see trailing junk in the command. Just consolidate the scmd->cmnd memsets to

memset(scmd->cmnd, 0, sizeof(scmd->cmnd);

outside of the ifs

Quoted 17 lines
drivers/scsi/scsi_error.c +8 -1
+		scmd->cmnd[0] = REQUEST_SENSE;
+		scmd->cmnd[4] = scmd->request_bufflen;
+		scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
+		/*
+		 * Zero the sense buffer.  The scsi spec mandates that any
+		 * untransferred sense data should be interpreted as being zero.
+		 */
+		memset(scmd->sense_buffer, 0, sizeof(scmd->sense_buffer));
 	} else {
 		scmd->request_buffer = NULL;
 		scmd->request_bufflen = 0;

@@ -657,18 +673,11 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	}
 
 	scmd->underflow = 0;
-	scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);

drivers/scsi/scsi_error.c static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd, · line 670

Quoted 10 lines
drivers/scsi/scsi_error.c +1 -6
-	if (sdev->scsi_level <= SCSI_2)
+	if (sdev->scsi_level <= SCSI_2 && sdev->scsi_level != SCSI_UNKNOWN)
 		scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
 			(sdev->lun << 5 & 0xe0);
 
-	/*
-	 * Zero the sense buffer.  The scsi spec mandates that any
-	 * untransferred sense data should be interpreted as being zero.
-	 */
-	memset(scmd->sense_buffer, 0, sizeof(scmd->sense_buffer));

Moving this to the sense_bytes specific piece of the if also introduces a subtle bug: If the driver doesn't do auto request sense and the command completes with CHECK CONDITION, the sense checking routine will potentially pick up stale sense data here

Quoted 23 lines
drivers/scsi/scsi_error.c +4 -4
-
 	shost->eh_action = &done;
 
 	spin_lock_irqsave(shost->host_lock, flags);

@@ -716,12 +725,13 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	/*
 	 * Restore original data
 	 */
-	scmd->request_buffer = old_buffer;
-	scmd->request_bufflen = old_bufflen;
+	scmd->cmd_len = old_cmd_len;
 	memcpy(scmd->cmnd, old_cmnd, sizeof(scmd->cmnd));
 	scmd->sc_data_direction = old_data_direction;
-	scmd->cmd_len = old_cmd_len;
+	scmd->request_bufflen = old_bufflen;
+	scmd->request_buffer = old_buffer;
 	scmd->use_sg = old_use_sg;
+	scmd->resid = old_resid;
 	scmd->result = old_result;
 	return rtn;
 }

@@ -737,10 +747,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
  **/

drivers/scsi/scsi_error.c static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd, · line 753

Quoted 10 lines
drivers/scsi/scsi_error.c +1 -4
 static int scsi_request_sense(struct scsi_cmnd *scmd)
 {
-	static unsigned char generic_sense[6] =
-		{REQUEST_SENSE, 0, 0, 0, 252, 0};
-
-	return scsi_send_eh_cmnd(scmd, generic_sense, 6, SENSE_TIMEOUT, 1);
+	return scsi_send_eh_cmnd(scmd, NULL, 0, SENSE_TIMEOUT, ~0);
 }
 
 /**

Other than the three comments, this looks OK.

James

author
#9 lore raw

Re: [PATCH ver4 1/5] scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()

I will send a ver4 of these patches as reply to base patches Meanwhile I have some comments below.

Thanks for the review.

Quoted 6 lines · James Bottomley

You've altered the signature and function of the routine here, this needs to be documented by updating the docbook description above the function. (I know you do it later, but each changeset should really be logically complete).

Done, thanks

Quoted 8 lines

drivers/scsi/scsi_error.c static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd, · line 659

Quoted 1 line
drivers/scsi/scsi_error.c +1 -0
+		memset(scmd->cmnd, 0, 6);

This will lead to subtle bugs: some devices (notably ATAPI) have a fixed command slot they will copy this into. You have potentially trailing bytes of junk that this could pick up ... we have ATAPI devices known to fall over if they see trailing junk in the command. Just consolidate the scmd->cmnd memsets to

Quoted 4 lines

memset(scmd->cmnd, 0, sizeof(scmd->cmnd);

outside of the ifs

I wanted to have a fixture where if @cmnd and @sense_bytes are Zero than scsi_cmnd->cmnd is not touched at all. So I fixed it by doing if (@cmnd) only in the second arm of the if and fixed the above to what you said. (See patch that follows)

Quoted 6 lines

Moving this to the sense_bytes specific piece of the if also introduces a subtle bug: If the driver doesn't do auto request sense and the command completes with CHECK CONDITION, the sense checking routine will potentially pick up stale sense data here

Good call thanks, done.

Quoted 6 lines

Other than the three comments, this looks OK.

James

author
#10 lore raw

PATCH 1/5 scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()

In reply to Boaz Harrosh

- regrouped variables for easier reviewing of next patch
- Support of cmnd==NULL in call to scsi_send_eh_cmnd()
- In the @sense_bytes case set transfer size to the minimum size of sense_buffer and passed @sense_bytes. cmnd[4] is set accordingly.
- REQUEST_SENSE is set into cmnd[0] so if @sense_bytes is not Zero passed @cmnd should be NULL.
- Also save/restore resid of failed command.
- Adjust caller

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/scsi_error.c |   71 ++++++++++++++++++++++++--------------------
 1 files changed, 39 insertions(+), 32 deletions(-)
drivers/scsi/scsi_error.c +39 -32
index f49feb9..4d53501 100644
@@ -592,36 +592,39 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
 /**
  * scsi_send_eh_cmnd  - submit a scsi command as part of error recory
  * @scmd:       SCSI command structure to hijack
- * @cmnd:       CDB to send
+ * @cmnd:       CDB to send. Can be NULL if no new cmnd is needed
  * @cmnd_size:  size in bytes of @cmnd
  * @timeout:    timeout for this request
- * @copy_sense: request sense data if set to 1
+ * @sense_bytes: size of sense data to copy. or 0 (if != 0 @cmnd is ignored)
  *
  * This function is used to send a scsi command down to a target device
- * as part of the error recovery process.  If @copy_sense is 0 the command
- * sent must be one that does not transfer any data.  If @copy_sense is 1
- * the command must be REQUEST_SENSE and this functions copies out the
- * sense buffer it got into @scmd->sense_buffer.
+ * as part of the error recovery process.  If @sense_bytes is 0 the command
+ * sent must be one that does not transfer any data.  If @sense_bytes != 0
+ * @cmnd is ignored and this functions sets up a REQUEST_SENSE command
+ * and cmnd buffers to read @sense_bytes into @scmd->sense_buffer.
  *
  * Return value:
  *    SUCCESS or FAILED or NEEDS_RETRY
  **/
 static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
-			     int cmnd_size, int timeout, int copy_sense)
+			     int cmnd_size, int timeout, unsigned sense_bytes)
 {
 	struct scsi_device *sdev = scmd->device;
 	struct Scsi_Host *shost = sdev->host;
-	int old_result = scmd->result;
 	DECLARE_COMPLETION_ONSTACK(done);
 	unsigned long timeleft;
 	unsigned long flags;
-	struct scatterlist sgl;
+
+	unsigned char old_cmd_len;
 	unsigned char old_cmnd[MAX_COMMAND_SIZE];
 	enum dma_data_direction old_data_direction;
-	unsigned short old_use_sg;
-	unsigned char old_cmd_len;
 	unsigned old_bufflen;
 	void *old_buffer;
+	unsigned short old_use_sg;
+	int old_resid;
+	int old_result;
+
+	struct scatterlist sgl;
 	int rtn;
 
 	/*

@@ -631,35 +634,41 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	 * we will need to restore these values prior to running the actual
 	 * command.
 	 */
-	old_buffer = scmd->request_buffer;
-	old_bufflen = scmd->request_bufflen;
+	old_cmd_len = scmd->cmd_len;
 	memcpy(old_cmnd, scmd->cmnd, sizeof(scmd->cmnd));
 	old_data_direction = scmd->sc_data_direction;
-	old_cmd_len = scmd->cmd_len;
+	old_bufflen = scmd->request_bufflen;
+	old_buffer = scmd->request_buffer;
 	old_use_sg = scmd->use_sg;
+	old_resid = scmd->resid;
+	old_result = scmd->result;
 
-	memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
-	memcpy(scmd->cmnd, cmnd, cmnd_size);
-
-	if (copy_sense) {
-		sg_init_one(&sgl, scmd->sense_buffer,
-			    sizeof(scmd->sense_buffer));
-
-		scmd->sc_data_direction = DMA_FROM_DEVICE;
-		scmd->request_bufflen = sgl.length;
+	if (sense_bytes) {
+		scmd->request_bufflen = min_t(unsigned,
+		                       sizeof(scmd->sense_buffer), sense_bytes);
+		sg_init_one(&sgl, scmd->sense_buffer, scmd->request_bufflen);
 		scmd->request_buffer = &sgl;
+		scmd->sc_data_direction = DMA_FROM_DEVICE;
 		scmd->use_sg = 1;
+		memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
+		scmd->cmnd[0] = REQUEST_SENSE;
+		scmd->cmnd[4] = scmd->request_bufflen;
+		scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
 	} else {
 		scmd->request_buffer = NULL;
 		scmd->request_bufflen = 0;
 		scmd->sc_data_direction = DMA_NONE;
 		scmd->use_sg = 0;
+		if (cmnd) {
+			memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
+			memcpy(scmd->cmnd, cmnd, cmnd_size);
+			scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
+		}
 	}
 
 	scmd->underflow = 0;
-	scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
 
-	if (sdev->scsi_level <= SCSI_2)
+	if (sdev->scsi_level <= SCSI_2 && sdev->scsi_level != SCSI_UNKNOWN)
 		scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
 			(sdev->lun << 5 & 0xe0);
 

@@ -716,12 +725,13 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	/*
 	 * Restore original data
 	 */
-	scmd->request_buffer = old_buffer;
-	scmd->request_bufflen = old_bufflen;
+	scmd->cmd_len = old_cmd_len;
 	memcpy(scmd->cmnd, old_cmnd, sizeof(scmd->cmnd));
 	scmd->sc_data_direction = old_data_direction;
-	scmd->cmd_len = old_cmd_len;
+	scmd->request_bufflen = old_bufflen;
+	scmd->request_buffer = old_buffer;
 	scmd->use_sg = old_use_sg;
+	scmd->resid = old_resid;
 	scmd->result = old_result;
 	return rtn;
 }

@@ -737,10 +747,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
  **/
 static int scsi_request_sense(struct scsi_cmnd *scmd)
 {
-	static unsigned char generic_sense[6] =
-		{REQUEST_SENSE, 0, 0, 0, 252, 0};
-
-	return scsi_send_eh_cmnd(scmd, generic_sense, 6, SENSE_TIMEOUT, 1);
+	return scsi_send_eh_cmnd(scmd, NULL, 0, SENSE_TIMEOUT, ~0);
 }
 
 /**
Signature · 1 line
1.5.3.1
author
#11 lore raw

PATCH 2/5 scsi_error: Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE

In reply to Boaz Harrosh

- Drivers/transports that want to send a synchronous REQUEST_SENSE command as part of their .queuecommand sequence, have 2 new API's that facilitate in doing so and abstract them from scsi-ml internals.

void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
struct scsi_eh_save *sesci, unsigned char *cmnd, int cmnd_size, int copy_sense)

Will hijack a command and prepare it for request sense if needed. And will save any later needed info into a scsi_eh_save structure.

void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd,

	struct scsi_eh_save *sesci);

Will undo any changes done to a command by above function. Making it ready for completion.

- Re-factor scsi_send_eh_cmnd to use above APIs

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/scsi_error.c |  125 ++++++++++++++++++++++++++-------------------
 include/scsi/scsi_eh.h    |   23 ++++++++-
 2 files changed, 95 insertions(+), 53 deletions(-)
drivers/scsi/scsi_error.c +73 -52
index 20a72aa..a02400c 100644
@@ -590,42 +590,23 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
 }
 
 /**
- * scsi_send_eh_cmnd  - submit a scsi command as part of error recory
+ * scsi_eh_prep_cmnd  - Save a scsi command info as part of error recory
  * @scmd:       SCSI command structure to hijack
- * @cmnd:       CDB to send
+ * @ses:        structure to save restore information
+ * @cmnd:       CDB to send. Can be NULL if no new cmnd is needed
  * @cmnd_size:  size in bytes of @cmnd
- * @timeout:    timeout for this request
- * @copy_sense: request sense data if set to 1
+ * @copy_sense: size of sense data to copy. or 0
  *
- * This function is used to send a scsi command down to a target device
- * as part of the error recovery process.  If @copy_sense is 0 the command
- * sent must be one that does not transfer any data.  If @copy_sense is 1
- * the command must be REQUEST_SENSE and this functions copies out the
- * sense buffer it got into @scmd->sense_buffer.
- *
- * Return value:
- *    SUCCESS or FAILED or NEEDS_RETRY
+ * This function is used to save a scsi command information before re-execution
+ * as part of an error recovery process.  If @copy_sense is 0 the command
+ * given must be one that does not transfer any data.  If @copy_sense != 0
+ * the command should be NULL and this functions sets up the cmnd and
+ * command buffers to be read into @scmd->sense_buffer.
  **/
-static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
-			     int cmnd_size, int timeout, unsigned copy_sense)
+void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
+			unsigned char *cmnd, int cmnd_size, unsigned copy_sense)
 {
 	struct scsi_device *sdev = scmd->device;
-	struct Scsi_Host *shost = sdev->host;
-	DECLARE_COMPLETION_ONSTACK(done);
-	unsigned long timeleft;
-	unsigned long flags;
-
-	unsigned char old_cmd_len;
-	unsigned char old_cmnd[MAX_COMMAND_SIZE];
-	enum dma_data_direction old_data_direction;
-	unsigned old_bufflen;
-	void *old_buffer;
-	unsigned short old_use_sg;
-	int old_resid;
-	int old_result;
-
-	struct scatterlist sgl;
-	int rtn;
 
 	/*
 	 * We need saved copies of a number of fields - this is because

@@ -634,14 +615,14 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	 * we will need to restore these values prior to running the actual
 	 * command.
 	 */
-	old_cmd_len = scmd->cmd_len;
-	memcpy(old_cmnd, scmd->cmnd, sizeof(scmd->cmnd));
-	old_data_direction = scmd->sc_data_direction;
-	old_bufflen = scmd->request_bufflen;
-	old_buffer = scmd->request_buffer;
-	old_use_sg = scmd->use_sg;
-	old_resid = scmd->resid;
-	old_result = scmd->result;
+	ses->cmd_len = scmd->cmd_len;
+	memcpy(ses->cmnd, scmd->cmnd, sizeof(scmd->cmnd));
+	ses->data_direction = scmd->sc_data_direction;
+	ses->bufflen = scmd->request_bufflen;
+	ses->buffer = scmd->request_buffer;
+	ses->use_sg = scmd->use_sg;
+	ses->resid = scmd->resid;
+	ses->result = scmd->result;
 
 	if (cmnd) {
 		memset(scmd->cmnd, 0, sizeof(scmd->cmnd));

@@ -650,9 +631,9 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	}
 
 	if (copy_sense) {
-		sg_init_one(&sgl, scmd->sense_buffer,
+		sg_init_one(&ses->sense_sgl, scmd->sense_buffer,
 			    sizeof(scmd->sense_buffer));
-		scmd->request_buffer = &sgl;
+		scmd->request_buffer = &ses->sense_sgl;
 		scmd->sc_data_direction = DMA_FROM_DEVICE;
 		scmd->request_bufflen = min_t(unsigned,
 		                       sizeof(scmd->sense_buffer), copy_sense);

@@ -677,7 +658,58 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	if (sdev->scsi_level <= SCSI_2 && sdev->scsi_level != SCSI_UNKNOWN)
 		scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
 			(sdev->lun << 5 & 0xe0);
+}
+EXPORT_SYMBOL(scsi_eh_prep_cmnd);
+
+/**
+ * scsi_eh_restore_cmnd  - Restore a scsi command info as part of error recory
+ * @scmd:       SCSI command structure to restore
+ * @ses:        saved information from a coresponding call to scsi_prep_eh_cmnd
+ *
+ * Undo any damage done by above scsi_prep_eh_cmnd().
+ **/
+void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
+{
+	/*
+	 * Restore original data
+	 */
+	scmd->cmd_len = ses->cmd_len;
+	memcpy(scmd->cmnd, ses->cmnd, sizeof(scmd->cmnd));
+	scmd->sc_data_direction = ses->data_direction;
+	scmd->request_bufflen = ses->bufflen;
+	scmd->request_buffer = ses->buffer;
+	scmd->use_sg = ses->use_sg;
+	scmd->resid = ses->resid;
+	scmd->result = ses->result;
+}
+EXPORT_SYMBOL(scsi_eh_restore_cmnd);
 
+/**
+ * scsi_send_eh_cmnd  - submit a scsi command as part of error recory
+ * @scmd:       SCSI command structure to hijack
+ * @cmnd:       CDB to send
+ * @cmnd_size:  size in bytes of @cmnd
+ * @timeout:    timeout for this request
+ * @copy_sense: size of sense data to copy or 0
+ *
+ * This function is used to send a scsi command down to a target device
+ * as part of the error recovery process. See also scsi_eh_prep_cmnd() above.
+ *
+ * Return value:
+ *    SUCCESS or FAILED or NEEDS_RETRY
+ **/
+static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
+			     int cmnd_size, int timeout, unsigned copy_sense)
+{
+	struct scsi_device *sdev = scmd->device;
+	struct Scsi_Host *shost = sdev->host;
+	DECLARE_COMPLETION_ONSTACK(done);
+	unsigned long timeleft;
+	unsigned long flags;
+	struct scsi_eh_save ses;
+	int rtn;
+
+	scsi_eh_prep_cmnd(scmd, &ses, cmnd, cmnd_size, copy_sense);
 	shost->eh_action = &done;
 
 	spin_lock_irqsave(shost->host_lock, flags);

@@ -721,18 +753,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 		rtn = FAILED;
 	}
 
-
-	/*
-	 * Restore original data
-	 */
-	scmd->cmd_len = old_cmd_len;
-	memcpy(scmd->cmnd, old_cmnd, sizeof(scmd->cmnd));
-	scmd->sc_data_direction = old_data_direction;
-	scmd->request_bufflen = old_bufflen;
-	scmd->request_buffer = old_buffer;
-	scmd->use_sg = old_use_sg;
-	scmd->resid = old_resid;
-	scmd->result = old_result;
+	scsi_eh_restore_cmnd(scmd, &ses);
 	return rtn;
 }
include/scsi/scsi_eh.h +22 -1
index c5c0f67..85b0447 100644
@@ -1,7 +1,7 @@
 #ifndef _SCSI_SCSI_EH_H
 #define _SCSI_SCSI_EH_H
 
-struct scsi_cmnd;
+#include <scsi/scsi_cmnd.h>
 struct scsi_device;
 struct Scsi_Host;
 

@@ -65,4 +65,25 @@ extern int scsi_get_sense_info_fld(const u8 * sense_buffer, int sb_len,
 
 extern int scsi_reset_provider(struct scsi_device *, int);
 
+struct scsi_eh_save {
+	int result;
+	enum dma_data_direction data_direction;
+	unsigned char cmd_len;
+	unsigned char cmnd[MAX_COMMAND_SIZE];
+
+	void *buffer;
+	unsigned bufflen;
+	unsigned short use_sg;
+	int resid;
+
+	struct scatterlist sense_sgl;
+};
+
+extern void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
+		struct scsi_eh_save *ses, unsigned char *cmnd,
+		int cmnd_size, unsigned copy_sense);
+
+extern void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd,
+		struct scsi_eh_save *ses);
+
 #endif /* _SCSI_SCSI_EH_H */
Signature · 1 line
1.5.3.1
#12 lore raw

Re: [PATCH ver2 2/5] scsi_error: Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE

Quoted 27 lines · Boaz Harrosh
drivers/scsi/scsi_error.c +4 -12
index 20a72aa..a02400c 100644
@@ -590,42 +590,23 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
 }
 
 /**
- * scsi_send_eh_cmnd  - submit a scsi command as part of error recory
+ * scsi_eh_prep_cmnd  - Save a scsi command info as part of error recory
  * @scmd:       SCSI command structure to hijack
- * @cmnd:       CDB to send
+ * @ses:        structure to save restore information
+ * @cmnd:       CDB to send. Can be NULL if no new cmnd is needed
  * @cmnd_size:  size in bytes of @cmnd
- * @timeout:    timeout for this request
- * @copy_sense: request sense data if set to 1
+ * @copy_sense: size of sense data to copy. or 0
  *
- * This function is used to send a scsi command down to a target device
- * as part of the error recovery process.  If @copy_sense is 0 the command
- * sent must be one that does not transfer any data.  If @copy_sense is 1
- * the command must be REQUEST_SENSE and this functions copies out the
- * sense buffer it got into @scmd->sense_buffer.
- *
- * Return value:
- *    SUCCESS or FAILED or NEEDS_RETRY

drivers/scsi/scsi_error.c static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd) · line 607

Quoted 10 lines
drivers/scsi/scsi_error.c +7 -2
+ * This function is used to save a scsi command information before re-execution
+ * as part of an error recovery process.  If @copy_sense is 0 the command
+ * given must be one that does not transfer any data.  If @copy_sense != 0
+ * the command should be NULL and this functions sets up the cmnd and
+ * command buffers to be read into @scmd->sense_buffer.
  **/
-static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
-			     int cmnd_size, int timeout, unsigned copy_sense)
+void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
+			unsigned char *cmnd, int cmnd_size, unsigned copy_sense)

I'm not terribly keen on the naming of copy_sense, tho I see what you're trying to do. A better name and better comments about what it means when != 0 would fix this.

Matt

Signature · 7 lines
Matthew Dharm                              Home: [email protected] 
Maintainer, Linux USB Mass Storage Driver

My mother not mind to die for stoppink Windows NT!  She is rememberink 
Stalin!
					-- Pitr
User Friendly, 9/6/1998
author
#13 lore raw

Re: [PATCH ver2 2/5] scsi_error: Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE

Quoted 43 lines · Matthew Dharm
Quoted 27 lines · Boaz Harrosh
drivers/scsi/scsi_error.c +4 -12
index 20a72aa..a02400c 100644
@@ -590,42 +590,23 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
 }
 
 /**
- * scsi_send_eh_cmnd  - submit a scsi command as part of error recory
+ * scsi_eh_prep_cmnd  - Save a scsi command info as part of error recory
  * @scmd:       SCSI command structure to hijack
- * @cmnd:       CDB to send
+ * @ses:        structure to save restore information
+ * @cmnd:       CDB to send. Can be NULL if no new cmnd is needed
  * @cmnd_size:  size in bytes of @cmnd
- * @timeout:    timeout for this request
- * @copy_sense: request sense data if set to 1
+ * @copy_sense: size of sense data to copy. or 0
  *
- * This function is used to send a scsi command down to a target device
- * as part of the error recovery process.  If @copy_sense is 0 the command
- * sent must be one that does not transfer any data.  If @copy_sense is 1
- * the command must be REQUEST_SENSE and this functions copies out the
- * sense buffer it got into @scmd->sense_buffer.
- *
- * Return value:
- *    SUCCESS or FAILED or NEEDS_RETRY

drivers/scsi/scsi_error.c static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd) · line 607

Quoted 10 lines
drivers/scsi/scsi_error.c +7 -2
+ * This function is used to save a scsi command information before re-execution
+ * as part of an error recovery process.  If @copy_sense is 0 the command
+ * given must be one that does not transfer any data.  If @copy_sense != 0
+ * the command should be NULL and this functions sets up the cmnd and
+ * command buffers to be read into @scmd->sense_buffer.
  **/
-static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
-			     int cmnd_size, int timeout, unsigned copy_sense)
+void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
+			unsigned char *cmnd, int cmnd_size, unsigned copy_sense)

I'm not terribly keen on the naming of copy_sense, tho I see what you're trying to do. A better name and better comments about what it means when != 0 would fix this.

Quoted 2 lines

Matt

When you're right you're right. This is what happens when you try to write comprehensible comments at 22:45 ;)

I will send first two patches as replays to the original patches

Thanks

author
#14 lore raw

PATCH 2/5 scsi_error: Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE

In reply to Boaz Harrosh

- Drivers/transports that want to send a synchronous REQUEST_SENSE command as part of their .queuecommand sequence, have 2 new API's that facilitate in doing so and abstract them from scsi-ml internals.

void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
struct scsi_eh_save *sesci, unsigned char *cmnd, int cmnd_size, int sense_bytes)

Will hijack a command and prepare it for request sense if needed. And will save any later needed info into a scsi_eh_save structure.

void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd,

	struct scsi_eh_save *sesci);

Will undo any changes done to a command by above function. Making it ready for completion.

- Re-factor scsi_send_eh_cmnd() to use above APIs

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/scsi_error.c |  126 ++++++++++++++++++++++++++------------------
 include/scsi/scsi_eh.h    |   23 ++++++++-
 2 files changed, 96 insertions(+), 53 deletions(-)
drivers/scsi/scsi_error.c +74 -52
index 46d57f1..cd9fb34 100644
@@ -590,42 +590,23 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
 }
 
 /**
- * scsi_send_eh_cmnd  - submit a scsi command as part of error recory
+ * scsi_eh_prep_cmnd  - Save a scsi command info as part of error recory
  * @scmd:       SCSI command structure to hijack
- * @cmnd:       CDB to send
+ * @ses:        structure to save restore information
+ * @cmnd:       CDB to send. Can be NULL if no new cmnd is needed
  * @cmnd_size:  size in bytes of @cmnd
- * @timeout:    timeout for this request
- * @copy_sense: request sense data if set to 1
+ * @sense_bytes: size of sense data to copy. or 0
  *
- * This function is used to send a scsi command down to a target device
- * as part of the error recovery process.  If @copy_sense is 0 the command
- * sent must be one that does not transfer any data.  If @copy_sense is 1
- * the command must be REQUEST_SENSE and this functions copies out the
- * sense buffer it got into @scmd->sense_buffer.
- *
- * Return value:
- *    SUCCESS or FAILED or NEEDS_RETRY
+ * This function is used to save a scsi command information before re-execution
+ * as part of an error recovery process.  If @sense_bytes is 0 the command
+ * given must be one that does not transfer any data.  If @sense_bytes != 0
+ * the command should be NULL and this functions sets up the cmnd and
+ * command buffers to read @sense_bytes into @scmd->sense_buffer.
  **/
-static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
-			     int cmnd_size, int timeout, unsigned sense_bytes)
+void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
+			unsigned char *cmnd, int cmnd_size, unsigned sense_bytes)
 {
 	struct scsi_device *sdev = scmd->device;
-	struct Scsi_Host *shost = sdev->host;
-	DECLARE_COMPLETION_ONSTACK(done);
-	unsigned long timeleft;
-	unsigned long flags;
-
-	unsigned char old_cmd_len;
-	unsigned char old_cmnd[MAX_COMMAND_SIZE];
-	enum dma_data_direction old_data_direction;
-	unsigned old_bufflen;
-	void *old_buffer;
-	unsigned short old_use_sg;
-	int old_resid;
-	int old_result;
-
-	struct scatterlist sgl;
-	int rtn;
 
 	/*
 	 * We need saved copies of a number of fields - this is because

@@ -634,14 +615,14 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	 * we will need to restore these values prior to running the actual
 	 * command.
 	 */
-	old_cmd_len = scmd->cmd_len;
-	memcpy(old_cmnd, scmd->cmnd, sizeof(scmd->cmnd));
-	old_data_direction = scmd->sc_data_direction;
-	old_bufflen = scmd->request_bufflen;
-	old_buffer = scmd->request_buffer;
-	old_use_sg = scmd->use_sg;
-	old_resid = scmd->resid;
-	old_result = scmd->result;
+	ses->cmd_len = scmd->cmd_len;
+	memcpy(ses->cmnd, scmd->cmnd, sizeof(scmd->cmnd));
+	ses->data_direction = scmd->sc_data_direction;
+	ses->bufflen = scmd->request_bufflen;
+	ses->buffer = scmd->request_buffer;
+	ses->use_sg = scmd->use_sg;
+	ses->resid = scmd->resid;
+	ses->result = scmd->result;
 
 	if (cmnd) {
 		memset(scmd->cmnd, 0, sizeof(scmd->cmnd));

@@ -652,8 +633,9 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	if (sense_bytes) {
 		scmd->request_bufflen = min_t(unsigned,
 		                       sizeof(scmd->sense_buffer), sense_bytes);
-		sg_init_one(&sgl, scmd->sense_buffer, scmd->request_bufflen);
-		scmd->request_buffer = &sgl;
+		sg_init_one(&ses->sense_sgl, scmd->sense_buffer,
+		                                       scmd->request_bufflen);
+		scmd->request_buffer = &ses->sense_sgl;
 		scmd->sc_data_direction = DMA_FROM_DEVICE;
 		scmd->use_sg = 1;
 		memset(scmd->cmnd, 0, 6);

@@ -676,7 +658,58 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	if (sdev->scsi_level <= SCSI_2 && sdev->scsi_level != SCSI_UNKNOWN)
 		scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
 			(sdev->lun << 5 & 0xe0);
+}
+EXPORT_SYMBOL(scsi_eh_prep_cmnd);
+
+/**
+ * scsi_eh_restore_cmnd  - Restore a scsi command info as part of error recory
+ * @scmd:       SCSI command structure to restore
+ * @ses:        saved information from a coresponding call to scsi_prep_eh_cmnd
+ *
+ * Undo any damage done by above scsi_prep_eh_cmnd().
+ **/
+void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
+{
+	/*
+	 * Restore original data
+	 */
+	scmd->cmd_len = ses->cmd_len;
+	memcpy(scmd->cmnd, ses->cmnd, sizeof(scmd->cmnd));
+	scmd->sc_data_direction = ses->data_direction;
+	scmd->request_bufflen = ses->bufflen;
+	scmd->request_buffer = ses->buffer;
+	scmd->use_sg = ses->use_sg;
+	scmd->resid = ses->resid;
+	scmd->result = ses->result;
+}
+EXPORT_SYMBOL(scsi_eh_restore_cmnd);
 
+/**
+ * scsi_send_eh_cmnd  - submit a scsi command as part of error recory
+ * @scmd:       SCSI command structure to hijack
+ * @cmnd:       CDB to send
+ * @cmnd_size:  size in bytes of @cmnd
+ * @timeout:    timeout for this request
+ * @sense_bytes: size of sense data to copy or 0
+ *
+ * This function is used to send a scsi command down to a target device
+ * as part of the error recovery process. See also scsi_eh_prep_cmnd() above.
+ *
+ * Return value:
+ *    SUCCESS or FAILED or NEEDS_RETRY
+ **/
+static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
+			     int cmnd_size, int timeout, unsigned sense_bytes)
+{
+	struct scsi_device *sdev = scmd->device;
+	struct Scsi_Host *shost = sdev->host;
+	DECLARE_COMPLETION_ONSTACK(done);
+	unsigned long timeleft;
+	unsigned long flags;
+	struct scsi_eh_save ses;
+	int rtn;
+
+	scsi_eh_prep_cmnd(scmd, &ses, cmnd, cmnd_size, sense_bytes);
 	shost->eh_action = &done;
 
 	spin_lock_irqsave(shost->host_lock, flags);

@@ -720,18 +753,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 		rtn = FAILED;
 	}
 
-
-	/*
-	 * Restore original data
-	 */
-	scmd->cmd_len = old_cmd_len;
-	memcpy(scmd->cmnd, old_cmnd, sizeof(scmd->cmnd));
-	scmd->sc_data_direction = old_data_direction;
-	scmd->request_bufflen = old_bufflen;
-	scmd->request_buffer = old_buffer;
-	scmd->use_sg = old_use_sg;
-	scmd->resid = old_resid;
-	scmd->result = old_result;
+	scsi_eh_restore_cmnd(scmd, &ses);
 	return rtn;
 }
include/scsi/scsi_eh.h +22 -1
index c5c0f67..44224ba 100644
@@ -1,7 +1,7 @@
 #ifndef _SCSI_SCSI_EH_H
 #define _SCSI_SCSI_EH_H
 
-struct scsi_cmnd;
+#include <scsi/scsi_cmnd.h>
 struct scsi_device;
 struct Scsi_Host;
 

@@ -65,4 +65,25 @@ extern int scsi_get_sense_info_fld(const u8 * sense_buffer, int sb_len,
 
 extern int scsi_reset_provider(struct scsi_device *, int);
 
+struct scsi_eh_save {
+	int result;
+	enum dma_data_direction data_direction;
+	unsigned char cmd_len;
+	unsigned char cmnd[MAX_COMMAND_SIZE];
+
+	void *buffer;
+	unsigned bufflen;
+	unsigned short use_sg;
+	int resid;
+
+	struct scatterlist sense_sgl;
+};
+
+extern void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
+		struct scsi_eh_save *ses, unsigned char *cmnd,
+		int cmnd_size, unsigned sense_bytes);
+
+extern void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd,
+		struct scsi_eh_save *ses);
+
 #endif /* _SCSI_SCSI_EH_H */
Signature · 1 line
1.5.3.1
#15 lore raw

Re: [PATCH ver3 2/5] scsi_error: Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE

Quoted 12 lines · Boaz Harrosh

- Drivers/transports that want to send a synchronous REQUEST_SENSE command as part of their .queuecommand sequence, have 2 new API's that facilitate in doing so and abstract them from scsi-ml internals.

void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
struct scsi_eh_save *sesci, unsigned char *cmnd, int cmnd_size, int sense_bytes)

Will hijack a command and prepare it for request sense if needed. And will save any later needed info into a scsi_eh_save structure.

void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd,

Quoted 8 lines
	struct scsi_eh_save *sesci);

Will undo any changes done to a command by above function. Making it ready for completion.

- Re-factor scsi_send_eh_cmnd() to use above APIs

Signed-off-by Boaz Harrosh <[email protected]>

This one looks OK, but it will likely not apply when you fix the first patch, so could you rebase and resend this as well.

Thanks,

James

author
#16 lore raw

PATCH 2/5 scsi_error: Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE

In reply to Boaz Harrosh

- Drivers/transports that want to send a synchronous REQUEST_SENSE command as part of their .queuecommand sequence, have 2 new API's that facilitate in doing so and abstract them from scsi-ml internals.

void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
struct scsi_eh_save *sesci, unsigned char *cmnd, int cmnd_size, int sense_bytes)

Will hijack a command and prepare it for request sense if needed. And will save any later needed info into a scsi_eh_save structure.

void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd,

	struct scsi_eh_save *sesci);

Will undo any changes done to a command by above function. Making it ready for completion.

- Re-factor scsi_send_eh_cmnd() to use above APIs

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/scsi_error.c |  114 +++++++++++++++++++++++++++------------------
 include/scsi/scsi_eh.h    |   23 +++++++++-
 2 files changed, 90 insertions(+), 47 deletions(-)
drivers/scsi/scsi_error.c +68 -46
index 4d53501..d29f846 100644
@@ -590,42 +590,23 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
 }
 
 /**
- * scsi_send_eh_cmnd  - submit a scsi command as part of error recory
+ * scsi_eh_prep_cmnd  - Save a scsi command info as part of error recory
  * @scmd:       SCSI command structure to hijack
+ * @ses:        structure to save restore information
  * @cmnd:       CDB to send. Can be NULL if no new cmnd is needed
  * @cmnd_size:  size in bytes of @cmnd
- * @timeout:    timeout for this request
  * @sense_bytes: size of sense data to copy. or 0 (if != 0 @cmnd is ignored)
  *
- * This function is used to send a scsi command down to a target device
+ * This function is used to save a scsi command information before re-execution
  * as part of the error recovery process.  If @sense_bytes is 0 the command
  * sent must be one that does not transfer any data.  If @sense_bytes != 0
  * @cmnd is ignored and this functions sets up a REQUEST_SENSE command
  * and cmnd buffers to read @sense_bytes into @scmd->sense_buffer.
- *
- * Return value:
- *    SUCCESS or FAILED or NEEDS_RETRY
  **/
-static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
-			     int cmnd_size, int timeout, unsigned sense_bytes)
+void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
+			unsigned char *cmnd, int cmnd_size, unsigned sense_bytes)
 {
 	struct scsi_device *sdev = scmd->device;
-	struct Scsi_Host *shost = sdev->host;
-	DECLARE_COMPLETION_ONSTACK(done);
-	unsigned long timeleft;
-	unsigned long flags;
-
-	unsigned char old_cmd_len;
-	unsigned char old_cmnd[MAX_COMMAND_SIZE];
-	enum dma_data_direction old_data_direction;
-	unsigned old_bufflen;
-	void *old_buffer;
-	unsigned short old_use_sg;
-	int old_resid;
-	int old_result;
-
-	struct scatterlist sgl;
-	int rtn;
 
 	/*
 	 * We need saved copies of a number of fields - this is because

@@ -634,20 +615,21 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	 * we will need to restore these values prior to running the actual
 	 * command.
 	 */
-	old_cmd_len = scmd->cmd_len;
-	memcpy(old_cmnd, scmd->cmnd, sizeof(scmd->cmnd));
-	old_data_direction = scmd->sc_data_direction;
-	old_bufflen = scmd->request_bufflen;
-	old_buffer = scmd->request_buffer;
-	old_use_sg = scmd->use_sg;
-	old_resid = scmd->resid;
-	old_result = scmd->result;
+	ses->cmd_len = scmd->cmd_len;
+	memcpy(ses->cmnd, scmd->cmnd, sizeof(scmd->cmnd));
+	ses->data_direction = scmd->sc_data_direction;
+	ses->bufflen = scmd->request_bufflen;
+	ses->buffer = scmd->request_buffer;
+	ses->use_sg = scmd->use_sg;
+	ses->resid = scmd->resid;
+	ses->result = scmd->result;
 
 	if (sense_bytes) {
 		scmd->request_bufflen = min_t(unsigned,
 		                       sizeof(scmd->sense_buffer), sense_bytes);
-		sg_init_one(&sgl, scmd->sense_buffer, scmd->request_bufflen);
-		scmd->request_buffer = &sgl;
+		sg_init_one(&ses->sense_sgl, scmd->sense_buffer,
+		                                       scmd->request_bufflen);
+		scmd->request_buffer = &ses->sense_sgl;
 		scmd->sc_data_direction = DMA_FROM_DEVICE;
 		scmd->use_sg = 1;
 		memset(scmd->cmnd, 0, sizeof(scmd->cmnd));

@@ -677,7 +659,58 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	 * untransferred sense data should be interpreted as being zero.
 	 */
 	memset(scmd->sense_buffer, 0, sizeof(scmd->sense_buffer));
+}
+EXPORT_SYMBOL(scsi_eh_prep_cmnd);
+
+/**
+ * scsi_eh_restore_cmnd  - Restore a scsi command info as part of error recory
+ * @scmd:       SCSI command structure to restore
+ * @ses:        saved information from a coresponding call to scsi_prep_eh_cmnd
+ *
+ * Undo any damage done by above scsi_prep_eh_cmnd().
+ **/
+void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
+{
+	/*
+	 * Restore original data
+	 */
+	scmd->cmd_len = ses->cmd_len;
+	memcpy(scmd->cmnd, ses->cmnd, sizeof(scmd->cmnd));
+	scmd->sc_data_direction = ses->data_direction;
+	scmd->request_bufflen = ses->bufflen;
+	scmd->request_buffer = ses->buffer;
+	scmd->use_sg = ses->use_sg;
+	scmd->resid = ses->resid;
+	scmd->result = ses->result;
+}
+EXPORT_SYMBOL(scsi_eh_restore_cmnd);
 
+/**
+ * scsi_send_eh_cmnd  - submit a scsi command as part of error recory
+ * @scmd:       SCSI command structure to hijack
+ * @cmnd:       CDB to send
+ * @cmnd_size:  size in bytes of @cmnd
+ * @timeout:    timeout for this request
+ * @sense_bytes: size of sense data to copy or 0
+ *
+ * This function is used to send a scsi command down to a target device
+ * as part of the error recovery process. See also scsi_eh_prep_cmnd() above.
+ *
+ * Return value:
+ *    SUCCESS or FAILED or NEEDS_RETRY
+ **/
+static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
+			     int cmnd_size, int timeout, unsigned sense_bytes)
+{
+	struct scsi_device *sdev = scmd->device;
+	struct Scsi_Host *shost = sdev->host;
+	DECLARE_COMPLETION_ONSTACK(done);
+	unsigned long timeleft;
+	unsigned long flags;
+	struct scsi_eh_save ses;
+	int rtn;
+
+	scsi_eh_prep_cmnd(scmd, &ses, cmnd, cmnd_size, sense_bytes);
 	shost->eh_action = &done;
 
 	spin_lock_irqsave(shost->host_lock, flags);

@@ -721,18 +754,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 		rtn = FAILED;
 	}
 
-
-	/*
-	 * Restore original data
-	 */
-	scmd->cmd_len = old_cmd_len;
-	memcpy(scmd->cmnd, old_cmnd, sizeof(scmd->cmnd));
-	scmd->sc_data_direction = old_data_direction;
-	scmd->request_bufflen = old_bufflen;
-	scmd->request_buffer = old_buffer;
-	scmd->use_sg = old_use_sg;
-	scmd->resid = old_resid;
-	scmd->result = old_result;
+	scsi_eh_restore_cmnd(scmd, &ses);
 	return rtn;
 }
include/scsi/scsi_eh.h +22 -1
index c5c0f67..44224ba 100644
@@ -1,7 +1,7 @@
 #ifndef _SCSI_SCSI_EH_H
 #define _SCSI_SCSI_EH_H
 
-struct scsi_cmnd;
+#include <scsi/scsi_cmnd.h>
 struct scsi_device;
 struct Scsi_Host;
 

@@ -65,4 +65,25 @@ extern int scsi_get_sense_info_fld(const u8 * sense_buffer, int sb_len,
 
 extern int scsi_reset_provider(struct scsi_device *, int);
 
+struct scsi_eh_save {
+	int result;
+	enum dma_data_direction data_direction;
+	unsigned char cmd_len;
+	unsigned char cmnd[MAX_COMMAND_SIZE];
+
+	void *buffer;
+	unsigned bufflen;
+	unsigned short use_sg;
+	int resid;
+
+	struct scatterlist sense_sgl;
+};
+
+extern void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
+		struct scsi_eh_save *ses, unsigned char *cmnd,
+		int cmnd_size, unsigned sense_bytes);
+
+extern void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd,
+		struct scsi_eh_save *ses);
+
 #endif /* _SCSI_SCSI_EH_H */
Signature · 1 line
1.5.3.1
author
#17 lore raw

PATCH 3/5 usb: transport.c use scsi_eh API in REQUEST_SENSE execution

In reply to Boaz Harrosh

- Use new scsi_eh_prep/restor_cmnd() for synchronous REQUEST_SENSE invocation.

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/usb/storage/transport.c |   46 +++-----------------------------------
 1 files changed, 4 insertions(+), 42 deletions(-)
drivers/usb/storage/transport.c +4 -42
index 323293a..c646750 100644
@@ -50,7 +50,7 @@
 #include <linux/slab.h>
 
 #include <scsi/scsi.h>
-#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_eh.h>
 #include <scsi/scsi_device.h>
 
 #include "usb.h"

@@ -580,25 +580,11 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
 	/* Now, if we need to do the auto-sense, let's do it */
 	if (need_auto_sense) {
 		int temp_result;
-		void* old_request_buffer;
-		unsigned short old_sg;
-		unsigned old_request_bufflen;
-		unsigned char old_sc_data_direction;
-		unsigned char old_cmd_len;
-		unsigned char old_cmnd[MAX_COMMAND_SIZE];
-		int old_resid;
+		struct scsi_eh_save ses;
 
 		US_DEBUGP("Issuing auto-REQUEST_SENSE\n");
 
-		/* save the old command */
-		memcpy(old_cmnd, srb->cmnd, MAX_COMMAND_SIZE);
-		old_cmd_len = srb->cmd_len;
-
-		/* set the command and the LUN */
-		memset(srb->cmnd, 0, MAX_COMMAND_SIZE);
-		srb->cmnd[0] = REQUEST_SENSE;
-		srb->cmnd[1] = old_cmnd[1] & 0xE0;
-		srb->cmnd[4] = 18;
+		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, US_SENSE_SIZE);
 
 		/* FIXME: we must do the protocol translation here */
 		if (us->subclass == US_SC_RBC || us->subclass == US_SC_SCSI)

@@ -606,36 +592,12 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
 		else
 			srb->cmd_len = 12;
 
-		/* set the transfer direction */
-		old_sc_data_direction = srb->sc_data_direction;
-		srb->sc_data_direction = DMA_FROM_DEVICE;
-
-		/* use the new buffer we have */
-		old_request_buffer = srb->request_buffer;
-		srb->request_buffer = us->sensebuf;
-
-		/* set the buffer length for transfer */
-		old_request_bufflen = srb->request_bufflen;
-		srb->request_bufflen = US_SENSE_SIZE;
-
-		/* set up for no scatter-gather use */
-		old_sg = srb->use_sg;
-		srb->use_sg = 0;
-
 		/* issue the auto-sense command */
-		old_resid = srb->resid;
 		srb->resid = 0;
 		temp_result = us->transport(us->srb, us);
 
 		/* let's clean up right away */
-		memcpy(srb->sense_buffer, us->sensebuf, US_SENSE_SIZE);
-		srb->resid = old_resid;
-		srb->request_buffer = old_request_buffer;
-		srb->request_bufflen = old_request_bufflen;
-		srb->use_sg = old_sg;
-		srb->sc_data_direction = old_sc_data_direction;
-		srb->cmd_len = old_cmd_len;
-		memcpy(srb->cmnd, old_cmnd, MAX_COMMAND_SIZE);
+		scsi_eh_restore_cmnd(srb, &ses);
 
 		if (test_bit(US_FLIDX_TIMED_OUT, &us->flags)) {
 			US_DEBUGP("-- auto-sense aborted\n");
Signature · 1 line
1.5.3.1
author
#18 lore raw

PATCH 4/5 NCR5380: Use scsi_eh API for REQUEST_SENSE invocation

In reply to Boaz Harrosh

- Use new scsi_eh_prep/restor_cmnd() for synchronous REQUEST_SENSE invocation.

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/NCR5380.c       |   23 +++++++++--------------
 drivers/scsi/NCR5380.h       |    7 +++++++
 drivers/scsi/atari_NCR5380.c |   23 ++++++++---------------
 drivers/scsi/sun3_NCR5380.c  |   18 +++++++-----------
 4 files changed, 31 insertions(+), 40 deletions(-)
drivers/scsi/NCR5380.c +9 -14
index f8e449a..575c715 100644
@@ -1542,9 +1542,7 @@ part2:
 	hostdata->connected = cmd;
 	hostdata->busy[cmd->device->id] |= (1 << cmd->device->lun);
 
-	if (cmd->SCp.ptr != (char *)cmd->sense_buffer) {
-		initialize_SCp(cmd);
-	}
+	initialize_SCp(cmd);
 
 	return 0;
 

@@ -2280,19 +2278,16 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) {
 						cmd->result = (cmd->result & 0x00ffff) | (DID_ERROR << 16);
 
 #ifdef AUTOSENSE
+					if ((cmd->cmnd[0] == REQUEST_SENSE) &&
+						hostdata->ses.cmd_len) {
+						scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+						hostdata->ses.cmd_len = 0 ;
+					}
+
 					if ((cmd->cmnd[0] != REQUEST_SENSE) && (status_byte(cmd->SCp.Status) == CHECK_CONDITION)) {
+						scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
+
 						dprintk(NDEBUG_AUTOSENSE, ("scsi%d : performing request sense\n", instance->host_no));
-						cmd->cmnd[0] = REQUEST_SENSE;
-						cmd->cmnd[1] &= 0xe0;
-						cmd->cmnd[2] = 0;
-						cmd->cmnd[3] = 0;
-						cmd->cmnd[4] = sizeof(cmd->sense_buffer);
-						cmd->cmnd[5] = 0;
-
-						cmd->SCp.buffer = NULL;
-						cmd->SCp.buffers_residual = 0;
-						cmd->SCp.ptr = (char *) cmd->sense_buffer;
-						cmd->SCp.this_residual = sizeof(cmd->sense_buffer);
 
 						LIST(cmd, hostdata->issue_queue);
 						cmd->host_scribble = (unsigned char *)
drivers/scsi/NCR5380.h +7 -0
index bccf13f..f9587e6 100644
@@ -30,6 +30,10 @@
 
 #include <linux/interrupt.h>
 
+#ifdef AUTOSENSE
+#include <scsi/scsi_eh.h>
+#endif
+
 #define NCR5380_PUBLIC_RELEASE 7
 #define NCR53C400_PUBLIC_RELEASE 2
 

@@ -281,6 +285,9 @@ struct NCR5380_hostdata {
 	unsigned pendingr;
 	unsigned pendingw;
 #endif
+#ifdef AUTOSENSE
+	struct scsi_eh_save ses;
+#endif
 };
 
 #ifdef __KERNEL__
drivers/scsi/atari_NCR5380.c +8 -15
index 03dbe60..b253d37 100644
@@ -2235,24 +2235,17 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance)
 						cmd->result = (cmd->result & 0x00ffff) | (DID_ERROR << 16);
 
 #ifdef AUTOSENSE
+					if ((cmd->cmnd[0] == REQUEST_SENSE) &&
+						hostdata->ses.cmd_len) {
+						scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+						hostdata->ses.cmd_len = 0 ;
+					}
+
 					if ((cmd->cmnd[0] != REQUEST_SENSE) &&
 					    (status_byte(cmd->SCp.Status) == CHECK_CONDITION)) {
+						scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
+
 						ASEN_PRINTK("scsi%d: performing request sense\n", HOSTNO);
-						cmd->cmnd[0] = REQUEST_SENSE;
-						cmd->cmnd[1] &= 0xe0;
-						cmd->cmnd[2] = 0;
-						cmd->cmnd[3] = 0;
-						cmd->cmnd[4] = sizeof(cmd->sense_buffer);
-						cmd->cmnd[5] = 0;
-						cmd->cmd_len = COMMAND_SIZE(cmd->cmnd[0]);
-
-						cmd->use_sg = 0;
-						/* this is initialized from initialize_SCp
-						cmd->SCp.buffer = NULL;
-						cmd->SCp.buffers_residual = 0;
-						*/
-						cmd->request_buffer = (char *) cmd->sense_buffer;
-						cmd->request_bufflen = sizeof(cmd->sense_buffer);
 
 						local_irq_save(flags);
 						LIST(cmd,hostdata->issue_queue);
drivers/scsi/sun3_NCR5380.c +7 -11
index 98e3fe1..74081dd 100644
@@ -2254,25 +2254,21 @@ static void NCR5380_information_transfer (struct Scsi_Host *instance)
 			cmd->result = (cmd->result & 0x00ffff) | (DID_ERROR << 16);
 		    
 #ifdef AUTOSENSE
+		    if ((cmd->cmnd[0] == REQUEST_SENSE) &&
+			                        hostdata->ses.cmd_len) {
+			scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+			hostdata->ses.cmd_len = 0 ;
+		    }
+
 		    if ((cmd->cmnd[0] != REQUEST_SENSE) && 
 			(status_byte(cmd->SCp.Status) == CHECK_CONDITION)) {
+			scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
 			ASEN_PRINTK("scsi%d: performing request sense\n",
 				    HOSTNO);
-			cmd->cmnd[0] = REQUEST_SENSE;
-			cmd->cmnd[1] &= 0xe0;
-			cmd->cmnd[2] = 0;
-			cmd->cmnd[3] = 0;
-			cmd->cmnd[4] = sizeof(cmd->sense_buffer);
-			cmd->cmnd[5] = 0;
-			cmd->cmd_len = COMMAND_SIZE(cmd->cmnd[0]);
-
-			cmd->use_sg = 0;
 			/* this is initialized from initialize_SCp 
 			cmd->SCp.buffer = NULL;
 			cmd->SCp.buffers_residual = 0;
 			*/
-			cmd->request_buffer = (char *) cmd->sense_buffer;
-			cmd->request_bufflen = sizeof(cmd->sense_buffer);
 
 			local_irq_save(flags);
 			LIST(cmd,hostdata->issue_queue);
Signature · 1 line
1.5.3.1
author
#19 lore raw

PATCH 5/5 arm: fas216 Use scsi_eh API for REQUEST_SENSE invocation

In reply to Boaz Harrosh

- Use new scsi_eh_prep/restor_cmnd() for synchronous REQUEST_SENSE invocation.

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/arm/fas216.c |   16 +++-------------
 drivers/scsi/arm/fas216.h |    3 +++
 2 files changed, 6 insertions(+), 13 deletions(-)
drivers/scsi/arm/fas216.c +3 -13
index fb5f202..a715632 100644
@@ -2018,6 +2018,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt,
 	 * the upper layers to process.  This would have been set
 	 * correctly by fas216_std_done.
 	 */
+	scsi_eh_restore_cmnd(SCpnt, &info->ses);
 	SCpnt->scsi_done(SCpnt);
 }
 

@@ -2103,23 +2104,12 @@ request_sense:
 	if (SCpnt->cmnd[0] == REQUEST_SENSE)
 		goto done;
 
+	scsi_eh_prep_cmnd(SCpnt, &info->ses, NULL, 0, ~0);
 	fas216_log_target(info, LOG_CONNECT, SCpnt->device->id,
 			  "requesting sense");
-	memset(SCpnt->cmnd, 0, sizeof (SCpnt->cmnd));
-	SCpnt->cmnd[0] = REQUEST_SENSE;
-	SCpnt->cmnd[1] = SCpnt->device->lun << 5;
-	SCpnt->cmnd[4] = sizeof(SCpnt->sense_buffer);
-	SCpnt->cmd_len = COMMAND_SIZE(SCpnt->cmnd[0]);
-	SCpnt->SCp.buffer = NULL;
-	SCpnt->SCp.buffers_residual = 0;
-	SCpnt->SCp.ptr = (char *)SCpnt->sense_buffer;
-	SCpnt->SCp.this_residual = sizeof(SCpnt->sense_buffer);
-	SCpnt->SCp.phase = sizeof(SCpnt->sense_buffer);
+	init_SCp(SCpnt);
 	SCpnt->SCp.Message = 0;
 	SCpnt->SCp.Status = 0;
-	SCpnt->request_bufflen = sizeof(SCpnt->sense_buffer);
-	SCpnt->sc_data_direction = DMA_FROM_DEVICE;
-	SCpnt->use_sg = 0;
 	SCpnt->tag = 0;
 	SCpnt->host_scribble = (void *)fas216_rq_sns_done;
drivers/scsi/arm/fas216.h +3 -0
index 00e5f05..c39882e 100644
@@ -16,6 +16,8 @@
 #define NO_IRQ 255
 #endif
 
+#include <scsi/scsi_eh.h>
+
 #include "queue.h"
 #include "msgqueue.h"
 

@@ -311,6 +313,7 @@ typedef struct {
 
 	/* miscellaneous */
 	int			internal_done;		/* flag to indicate request done */
+	struct scsi_eh_save	*ses;		/* holds request sense restore info */
 	unsigned long		magic_end;
 } FAS216_Info;
Signature · 1 line
1.5.3.1
#20 lore raw

Re: [PATCH ver2 5/5] arm: fas216 Use scsi_eh API for REQUEST_SENSE invocation

Quoted 37 lines · Boaz Harrosh

- Use new scsi_eh_prep/restor_cmnd() for synchronous REQUEST_SENSE invocation.

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/arm/fas216.c |   16 +++-------------
 drivers/scsi/arm/fas216.h |    3 +++
 2 files changed, 6 insertions(+), 13 deletions(-)
drivers/scsi/arm/fas216.c +2 -8
index fb5f202..a715632 100644
@@ -2018,6 +2018,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt,
 	 * the upper layers to process.  This would have been set
 	 * correctly by fas216_std_done.
 	 */
+	scsi_eh_restore_cmnd(SCpnt, &info->ses);
 	SCpnt->scsi_done(SCpnt);
 }
 

@@ -2103,23 +2104,12 @@ request_sense:
 	if (SCpnt->cmnd[0] == REQUEST_SENSE)
 		goto done;
 
+	scsi_eh_prep_cmnd(SCpnt, &info->ses, NULL, 0, ~0);
 	fas216_log_target(info, LOG_CONNECT, SCpnt->device->id,
 			  "requesting sense");
-	memset(SCpnt->cmnd, 0, sizeof (SCpnt->cmnd));
-	SCpnt->cmnd[0] = REQUEST_SENSE;
-	SCpnt->cmnd[1] = SCpnt->device->lun << 5;
-	SCpnt->cmnd[4] = sizeof(SCpnt->sense_buffer);
-	SCpnt->cmd_len = COMMAND_SIZE(SCpnt->cmnd[0]);
-	SCpnt->SCp.buffer = NULL;
-	SCpnt->SCp.buffers_residual = 0;
-	SCpnt->SCp.ptr = (char *)SCpnt->sense_buffer;

drivers/scsi/arm/fas216.c request_sense: · line 2114

Quoted 10 lines
drivers/scsi/arm/fas216.c +1 -5
-	SCpnt->SCp.this_residual = sizeof(SCpnt->sense_buffer);
-	SCpnt->SCp.phase = sizeof(SCpnt->sense_buffer);
+	init_SCp(SCpnt);
 	SCpnt->SCp.Message = 0;
 	SCpnt->SCp.Status = 0;
-	SCpnt->request_bufflen = sizeof(SCpnt->sense_buffer);
-	SCpnt->sc_data_direction = DMA_FROM_DEVICE;
-	SCpnt->use_sg = 0;
 	SCpnt->tag = 0;
 	SCpnt->host_scribble = (void *)fas216_rq_sns_done;

So where do we end up setting up the request sense command?

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
#21 lore raw

Re: [PATCH ver2 5/5] arm: fas216 Use scsi_eh API for REQUEST_SENSE invocation

Quoted 43 lines · Russell King
Quoted 41 lines · Boaz Harrosh

- Use new scsi_eh_prep/restor_cmnd() for synchronous REQUEST_SENSE invocation.

Signed-off-by Boaz Harrosh <[email protected]>
---
 drivers/scsi/arm/fas216.c |   16 +++-------------
 drivers/scsi/arm/fas216.h |    3 +++
 2 files changed, 6 insertions(+), 13 deletions(-)
drivers/scsi/arm/fas216.c +3 -10
index fb5f202..a715632 100644
@@ -2018,6 +2018,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt,
 	 * the upper layers to process.  This would have been set
 	 * correctly by fas216_std_done.
 	 */
+	scsi_eh_restore_cmnd(SCpnt, &info->ses);
 	SCpnt->scsi_done(SCpnt);
 }
 

@@ -2103,23 +2104,12 @@ request_sense:
 	if (SCpnt->cmnd[0] == REQUEST_SENSE)
 		goto done;
 
+	scsi_eh_prep_cmnd(SCpnt, &info->ses, NULL, 0, ~0);
 	fas216_log_target(info, LOG_CONNECT, SCpnt->device->id,
 			  "requesting sense");
-	memset(SCpnt->cmnd, 0, sizeof (SCpnt->cmnd));
-	SCpnt->cmnd[0] = REQUEST_SENSE;
-	SCpnt->cmnd[1] = SCpnt->device->lun << 5;
-	SCpnt->cmnd[4] = sizeof(SCpnt->sense_buffer);
-	SCpnt->cmd_len = COMMAND_SIZE(SCpnt->cmnd[0]);
-	SCpnt->SCp.buffer = NULL;
-	SCpnt->SCp.buffers_residual = 0;
-	SCpnt->SCp.ptr = (char *)SCpnt->sense_buffer;
-	SCpnt->SCp.this_residual = sizeof(SCpnt->sense_buffer);
-	SCpnt->SCp.phase = sizeof(SCpnt->sense_buffer);
+	init_SCp(SCpnt);
 	SCpnt->SCp.Message = 0;
 	SCpnt->SCp.Status = 0;
Quoted 10 lines

drivers/scsi/arm/fas216.c request_sense: · line 2115

Quoted 6 lines
drivers/scsi/arm/fas216.c +0 -3
-	SCpnt->request_bufflen = sizeof(SCpnt->sense_buffer);
-	SCpnt->sc_data_direction = DMA_FROM_DEVICE;
-	SCpnt->use_sg = 0;
 	SCpnt->tag = 0;
 	SCpnt->host_scribble = (void *)fas216_rq_sns_done;

So where do we end up setting up the request sense command?

In scsi_eh_prep_cmnd(), when (sense_bytes != 0), called here:

+ scsi_eh_prep_cmnd(SCpnt, &info->ses, NULL, 0, ~0);