Re: [RFC PATCHSET block#for-2.6.36-post] block: convert to REQ_FLUSH/FUA

[FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>]

On Wed, 18 Aug 2010 09:26:27 -0500
James Bottomley <James.Bottomley@suse.de> wrote:

> On Wed, 2010-08-18 at 11:53 +0200, Christoph Hellwig wrote:
> > On Mon, Aug 16, 2010 at 06:51:58PM +0200, Tejun Heo wrote:
> > > * scsi_error.c for some reason tests REQ_HARDBARRIER.  I think this
> > >   part can be dropped altogether but am not sure.
> > 
> > That was introduced by commit 77a4229719e511a0d38d9c355317ae1469adeb54
> > 
> > [SCSI] Retry commands with UNIT_ATTENTION sense codes to fix ext3/ext4 I/O error
> > 
> > which just went in this May.  Given that we now send barriers and
> > discards as FS requests I think it should be reverted, preferably
> > already during this cycle, in which the patches to send these requests
> > as FS type were added.
> 
> Right ... it was the quickest hack to get around the fact that we expect
> the caller to do error handling for BLOCK_PC requests and none of the
> block discard and barrier code expects to do this.
> 
> Once they all go as REQ_TYPE_FS, this code can be removed.

I think that I reverted it with the discard and flush conversion. No
idea why the code exists in the current git.


commit e96f6abe02fc3320d669985443e8c68ff8e83294
Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date:   Fri Jul 9 09:38:26 2010 +0900

scsi-ml uses REQ_TYPE_BLOCK_PC for flush requests from file
systems. The definition of REQ_TYPE_BLOCK_PC is that we don't retry
requests even when we can (e.g. UNIT ATTENTION) and we send the
response to the callers (then the callers can decide what they want).
We need a workaround such as the commit
77a4229719e511a0d38d9c355317ae1469adeb54 to retry BLOCK_PC flush
requests. We will need the similar workaround for discard requests too
since SCSI-ml handle them as BLOCK_PC internally.

This uses REQ_TYPE_FS for flush requests from file systems instead of
REQ_TYPE_BLOCK_PC.

scsi-ml retries only REQ_TYPE_FS requests that have data to
transfer when we can retry them (e.g. UNIT_ATTENTION). However, we
also need to retry REQ_TYPE_FS requests without data because the
callers don't.

This also changes scsi_check_sense() to retry all the REQ_TYPE_FS
requests when appropriate. Thanks to scsi_noretry_cmd(),
REQ_TYPE_BLOCK_PC requests don't be retried as before.

Note that basically, this reverts the commit
77a4229719e511a0d38d9c355317ae1469adeb54 since now we use REQ_TYPE_FS
for flush requests.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
---
 drivers/scsi/scsi_error.c |   19 ++++---------------
 drivers/scsi/sd.c         |    2 --
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 1b88af8..2768bf6 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -307,20 +307,7 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 		    (sshdr.asc == 0x04) && (sshdr.ascq == 0x02))
 			return FAILED;
 
-		if (scmd->request->cmd_flags & REQ_HARDBARRIER)
-			/*
-			 * barrier requests should always retry on UA
-			 * otherwise block will get a spurious error
-			 */
-			return NEEDS_RETRY;
-		else
-			/*
-			 * for normal (non barrier) commands, pass the
-			 * UA upwards for a determination in the
-			 * completion functions
-			 */
-			return SUCCESS;
-
+		return NEEDS_RETRY;
 		/* these three are not supported */
 	case COPY_ABORTED:
 	case VOLUME_OVERFLOW:
@@ -1336,7 +1323,9 @@ int scsi_noretry_cmd(struct scsi_cmnd *scmd)
 		 * assume caller has checked sense and determinted
 		 * the check condition was retryable.
 		 */
-		return (scmd->request->cmd_flags & REQ_FAILFAST_DEV);
+		if (scmd->request->cmd_flags & REQ_FAILFAST_DEV ||
+		    scmd->request->cmd_type == REQ_TYPE_BLOCK_PC)
+			return 1;
 	}
 
 	return 0;
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index e63b85a..108daea 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -477,8 +477,6 @@ static int scsi_setup_discard_cmnd(struct scsi_device *sdp, struct request *rq)
 
 static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
 {
-	/* for now, we use REQ_TYPE_BLOCK_PC. */
-	rq->cmd_type = REQ_TYPE_BLOCK_PC;
 	rq->timeout = SD_TIMEOUT;
 	rq->retries = SD_MAX_RETRIES;
 	rq->cmd[0] = SYNCHRONIZE_CACHE;
-- 
1.6.5