[PATCH 0/3] convert flush request to REQ_TYPE_FS instead of BLOCK_PC

[PATCH 0/3] convert flush request to REQ_TYPE_FS instead of BLOCK_PC

[FUJITA Tomonori]

We use REQ_TYPE_BLOCK_PC for flush requests from file
systems. scsi-ml's definition of REQ_TYPE_BLOCK_PC is that we don't
retry PC requests even when we can (e.g. UNIT ATTENTION) and we send
the response to the callers, and expect the callers to decide what
they want.

File systems (and the block layer) don't have the mechnism to retry
requests so file systems hit avoidable I/O errors. 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. flush requests are retried when appropriate.

This patchset can be applied to the block tree's for-2.6.36. It
contains of some changes to scsi so this patchset can't be applied to
scsi-misc.

=
 block/blk-barrier.c       |    4 ++--
 drivers/scsi/scsi_error.c |   19 ++++---------------
 drivers/scsi/sd.c         |   19 +++++++++++++------
 include/linux/bio.h       |    2 ++
 4 files changed, 21 insertions(+), 23 deletions(-)

[PATCH 1/3] block: set up rq->rq_disk properly for flush requests

[FUJITA Tomonori]

q->bar_rq.rq_disk is NULL. Use the rq_disk of the original request
instead.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 block/blk-barrier.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index 7c6f4a7..4646f6d 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -144,7 +144,7 @@ static void queue_flush(struct request_queue *q, unsigned which)
 
 	blk_rq_init(q, rq);
 	rq->cmd_flags = REQ_HARDBARRIER;
-	rq->rq_disk = q->bar_rq.rq_disk;
+	rq->rq_disk = q->orig_bar_rq->rq_disk;
 	rq->end_io = end_io;
 	q->prepare_flush_fn(q, rq);
 
-- 
1.6.5

[PATCH 2/3] block: introduce REQ_FLUSH flag

[FUJITA Tomonori]

SCSI-ml needs a way to mark a request as flush request in
q->prepare_flush_fn because it needs to identify them later (e.g. in
q->request_fn or prep_rq_fn).

queue_flush sets REQ_HARDBARRIER in rq->cmd_flags however the block
layer also sends normal REQ_TYPE_FS requests with REQ_HARDBARRIER. So
SCSI-ml can't use REQ_HARDBARRIER to identify flush requests.

We could change the block layer to clear REQ_HARDBARRIER bit before
sending non flush requests to the lower layers. However, intorudcing
the new flag looks cleaner (surely easier).

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 block/blk-barrier.c |    2 +-
 include/linux/bio.h |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index 4646f6d..ce4e9b8 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -143,7 +143,7 @@ static void queue_flush(struct request_queue *q, unsigned which)
 	}
 
 	blk_rq_init(q, rq);
-	rq->cmd_flags = REQ_HARDBARRIER;
+	rq->cmd_flags = REQ_HARDBARRIER | REQ_FLUSH;
 	rq->rq_disk = q->orig_bar_rq->rq_disk;
 	rq->end_io = end_io;
 	q->prepare_flush_fn(q, rq);
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 4d379c8..331bd0a 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -152,6 +152,7 @@ enum rq_flag_bits {
 	__REQ_SYNC,		/* request is sync (sync write or read) */
 	__REQ_META,		/* metadata io request */
 	__REQ_DISCARD,		/* request to discard sectors */
+	__REQ_FLUSH,		/* request for cache flush */
 	__REQ_NOIDLE,		/* don't anticipate more IO after this one */
 
 	/* bio only flags */
@@ -187,6 +188,7 @@ enum rq_flag_bits {
 #define REQ_SYNC		(1 << __REQ_SYNC)
 #define REQ_META		(1 << __REQ_META)
 #define REQ_DISCARD		(1 << __REQ_DISCARD)
+#define REQ_FLUSH		(1 << __REQ_FLUSH)
 #define REQ_NOIDLE		(1 << __REQ_NOIDLE)
 
 #define REQ_FAILFAST_MASK \
-- 
1.6.5

Re: [PATCH 2/3] block: introduce REQ_FLUSH flag

[Christoph Hellwig]

> +++ b/include/linux/bio.h
> @@ -152,6 +152,7 @@ enum rq_flag_bits {
>  	__REQ_SYNC,		/* request is sync (sync write or read) */
>  	__REQ_META,		/* metadata io request */
>  	__REQ_DISCARD,		/* request to discard sectors */
> +	__REQ_FLUSH,		/* request for cache flush */

As we only used this in the request it should go into the struct request
only section at the end of the flags.

Re: [PATCH 2/3] block: introduce REQ_FLUSH flag

[FUJITA Tomonori]

On Sun, 27 Jun 2010 05:52:24 -0400
Christoph Hellwig <hch@infradead.org> wrote:

> > +++ b/include/linux/bio.h
> > @@ -152,6 +152,7 @@ enum rq_flag_bits {
> >  	__REQ_SYNC,		/* request is sync (sync write or read) */
> >  	__REQ_META,		/* metadata io request */
> >  	__REQ_DISCARD,		/* request to discard sectors */
> > +	__REQ_FLUSH,		/* request for cache flush */
> 
> As we only used this in the request it should go into the struct request
> only section at the end of the flags.

Ah, thanks.

Here's the updated patch.

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH 2/3] block: introduce REQ_FLUSH flag

SCSI-ml needs a way to mark a request as flush request in
q->prepare_flush_fn because it needs to identify them later (e.g. in
q->request_fn or prep_rq_fn).

queue_flush sets REQ_HARDBARRIER in rq->cmd_flags however the block
layer also sends normal REQ_TYPE_FS requests with REQ_HARDBARRIER. So
SCSI-ml can't use REQ_HARDBARRIER to identify flush requests.

We could change the block layer to clear REQ_HARDBARRIER bit before
sending non flush requests to the lower layers. However, intorudcing
the new flag looks cleaner (surely easier).

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 block/blk-barrier.c |    2 +-
 include/linux/bio.h |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index 4646f6d..ce4e9b8 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -143,7 +143,7 @@ static void queue_flush(struct request_queue *q, unsigned which)
 	}
 
 	blk_rq_init(q, rq);
-	rq->cmd_flags = REQ_HARDBARRIER;
+	rq->cmd_flags = REQ_HARDBARRIER | REQ_FLUSH;
 	rq->rq_disk = q->orig_bar_rq->rq_disk;
 	rq->end_io = end_io;
 	q->prepare_flush_fn(q, rq);
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 4d379c8..f655b54 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -174,6 +174,7 @@ enum rq_flag_bits {
 	__REQ_ALLOCED,		/* request came from our alloc pool */
 	__REQ_COPY_USER,	/* contains copies of user pages */
 	__REQ_INTEGRITY,	/* integrity metadata has been remapped */
+	__REQ_FLUSH,		/* request for cache flush */
 	__REQ_IO_STAT,		/* account I/O stat */
 	__REQ_MIXED_MERGE,	/* merge of different types, fail separately */
 	__REQ_NR_BITS,		/* stops here */
@@ -213,6 +214,7 @@ enum rq_flag_bits {
 #define REQ_ALLOCED		(1 << __REQ_ALLOCED)
 #define REQ_COPY_USER		(1 << __REQ_COPY_USER)
 #define REQ_INTEGRITY		(1 << __REQ_INTEGRITY)
+#define REQ_FLUSH		(1 << __REQ_FLUSH)
 #define REQ_IO_STAT		(1 << __REQ_IO_STAT)
 #define REQ_MIXED_MERGE		(1 << __REQ_MIXED_MERGE)
 
-- 
1.6.5

[PATCH 3/3] use REQ_TYPE_FS for flush request

[FUJITA Tomonori]

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>
---
 drivers/scsi/scsi_error.c |   19 ++++---------------
 drivers/scsi/sd.c         |   19 +++++++++++++------
 2 files changed, 17 insertions(+), 21 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 86da819..d447726 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -488,11 +488,18 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 	int ret, host_dif;
 	unsigned char protect;
 
-	/*
-	 * Discard request come in as REQ_TYPE_FS but we turn them into
-	 * block PC requests to make life easier.
-	 */
-	if (rq->cmd_flags & REQ_DISCARD) {
+	if (rq->cmd_flags & REQ_FLUSH) {
+		/*
+		 * Flush request is REQ_TYPE_FS but we initialize them
+		 * as REQ_TYPE_BLOCK_PC.
+		 */
+		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
+		goto out;
+	} else if (rq->cmd_flags & REQ_DISCARD) {
+		/*
+		 * Discard request come in as REQ_TYPE_FS but we turn
+		 * them into block PC requests to make life easier.
+		 */
 		ret = scsi_setup_discard_cmnd(sdp, rq);
 		goto out;
 	} else if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
@@ -1046,7 +1053,7 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
 
 static void sd_prepare_flush(struct request_queue *q, struct request *rq)
 {
-	rq->cmd_type = REQ_TYPE_BLOCK_PC;
+	rq->cmd_type = REQ_TYPE_FS;
 	rq->timeout = SD_TIMEOUT;
 	rq->retries = SD_MAX_RETRIES;
 	rq->cmd[0] = SYNCHRONIZE_CACHE;
-- 
1.6.5