Re: misc scsi midlayer updates

2014-03-31 Thread Christoph Hellwig
Hi Boaz,

 Hi Christoph
 
 Many years ago I have sent these exact patches but no-one cared Including
 me I guess.

I didn't remember your older patches, sorry.

 I think my:
   scsi_lib: Remove that __scsi_release_buffers contraption
 Is more elegant, is layered better and is smaller code. (please
 consider my version)

I very much disagree - the bidi code uses a separate request for it's payload,
uses separate functions to set it up at the low-level so mirroring it with
a separate teardown makes sense.  This also avoids having to do any bidi
check at all in the fast path.

 Also the first patch is some more cleanup you'd like.

Doesn't look bad, but not that importan either.

 The main patch of collapsing  scsi_end_request is basically the same.

I like the goto version better beause it avoids additional duplication from
inside the switch and the bidi path, but it should be functionally equivalent.


 Please note the 4th patch which is a real BUG, titled:
   scsi_lib: Can't RETRY scsi_cmnd if some bytes were completed

That fix seems very hard to read due to the arithmetic comparism on the enum
value.  The way I try to understand it is that you never want to retry
if ((an error happened)  (bytes were completed)) but the explanation
should be expanded.

 [Your patches have been tested within my MQ testing right?]

Yes.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: misc scsi midlayer updates

2014-03-30 Thread Boaz Harrosh
On 03/27/2014 06:14 PM, Christoph Hellwig wrote:
 Various patches from the scsi multiqueue development that make sense on their
 own.
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

Hi Christoph

Many years ago I have sent these exact patches but no-one cared Including
me I guess.

(one instance is here, I sent it several times)
http://comments.gmane.org/gmane.linux.scsi/63347

Please note the 4th patch which is a real BUG, titled:
scsi_lib: Can't RETRY scsi_cmnd if some bytes were completed

I think my:
scsi_lib: Remove that __scsi_release_buffers contraption
Is more elegant, is layered better and is smaller code. (please
consider my version)

Also the first patch is some more cleanup you'd like.

The main patch of collapsing  scsi_end_request is basically the same.

Funny that I've just rebased an exactly 2 years ago version and it
rebased cleanly. So here they are for your reference.

[PATCH 1/4] scsi_lib: request_queue is only needed inside
[PATCH 2/4] scsi_lib: Remove that __scsi_release_buffers contraption
[PATCH 3/4] scsi_lib: Collapse scsi_end_request into only user
[PATCH 4/4] scsi_lib: BUG: Can't RETRY scsi_cmnd if some bytes were

[Your patches have been tested within my MQ testing right?]

Thanks for pushing on this
Boaz

From 9809b82bbb9813370738ac00400c01d61b0c51b4 Mon Sep 17 00:00:00 2001
From: Boaz Harrosh bharr...@panasas.com
Date: Mon, 4 Jan 2010 10:45:56 +0200
Subject: [PATCH 1/4] scsi_lib: request_queue is only needed inside
 scsi_requeue_command

This is a pure cleanup. Just starting the engines for things
to come.

Signed-off-by: Boaz Harrosh bharr...@panasas.com
---
 drivers/scsi/scsi_lib.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 62ec84b..dd834ca 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -491,10 +491,11 @@ void scsi_requeue_run_queue(struct work_struct *work)
  *		sector.
  * Notes:	Upon return, cmd is a stale pointer.
  */
-static void scsi_requeue_command(struct request_queue *q, struct scsi_cmnd *cmd)
+static void scsi_requeue_command(struct scsi_cmnd *cmd)
 {
 	struct scsi_device *sdev = cmd-device;
 	struct request *req = cmd-request;
+	struct request_queue *q = req-q;
 	unsigned long flags;
 
 	/*
@@ -565,7 +566,6 @@ static void __scsi_release_buffers(struct scsi_cmnd *, int);
 static struct scsi_cmnd *scsi_end_request(struct scsi_cmnd *cmd, int error,
 	  int bytes, int requeue)
 {
-	struct request_queue *q = cmd-device-request_queue;
 	struct request *req = cmd-request;
 
 	/*
@@ -584,7 +584,7 @@ static struct scsi_cmnd *scsi_end_request(struct scsi_cmnd *cmd, int error,
  * queue, and goose the queue again.
  */
 scsi_release_buffers(cmd);
-scsi_requeue_command(q, cmd);
+scsi_requeue_command(cmd);
 cmd = NULL;
 			}
 			return cmd;
@@ -779,7 +779,6 @@ static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
 void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 {
 	int result = cmd-result;
-	struct request_queue *q = cmd-device-request_queue;
 	struct request *req = cmd-request;
 	int error = 0;
 	struct scsi_sense_hdr sshdr;
@@ -1003,7 +1002,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 			scsi_print_command(cmd);
 		}
 		if (blk_end_request_err(req, error))
-			scsi_requeue_command(q, cmd);
+			scsi_requeue_command(cmd);
 		else
 			scsi_next_command(cmd);
 		break;
@@ -1012,7 +1011,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 		 * A new command will be prepared and issued.
 		 */
 		scsi_release_buffers(cmd);
-		scsi_requeue_command(q, cmd);
+		scsi_requeue_command(cmd);
 		break;
 	case ACTION_RETRY:
 		/* Retry the same command immediately */
-- 
1.8.5.3

From aac579b02c7d3665665a1f1f7c064a82b70b873a Mon Sep 17 00:00:00 2001
From: Boaz Harrosh bharr...@panasas.com
Date: Mon, 4 Jan 2010 12:46:06 +0200
Subject: [PATCH 2/4] scsi_lib: Remove that __scsi_release_buffers contraption

Remove do_bidi_check by checking and nullifying cmd-request
when blk_end_* indicates the request is no longer valid.

Signed-off-by: Boaz Harrosh bharr...@panasas.com
---
 drivers/scsi/scsi_lib.c | 41 +
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index dd834ca..c4cdc6a 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -539,8 +539,6 @@ void scsi_run_host_queues(struct Scsi_Host *shost)
 		scsi_run_queue(sdev-request_queue);
 }
 
-static void __scsi_release_buffers(struct scsi_cmnd *, int);
-
 /*
  * Function:scsi_end_request()
  *
@@ -591,11 +589,12 @@ static struct scsi_cmnd *scsi_end_request(struct scsi_cmnd *cmd, int error,
 		}
 	}
 
+	

misc scsi midlayer updates

2014-03-27 Thread Christoph Hellwig
Various patches from the scsi multiqueue development that make sense on their
own.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html