linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: linux-scsi@vger.kernel.org, linux-block@vger.kernel.org
Cc: jiangyiwen@huawei.com, snitzer@redhat.com,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Subject: [PATCH] block/sd: Return -EREMOTEIO when WRITE SAME and DISCARD are disabled
Date: Thu,  4 Feb 2016 01:48:03 -0500	[thread overview]
Message-ID: <1454568483-11298-1-git-send-email-martin.petersen@oracle.com> (raw)
In-Reply-To: <20160204042528.GA15599@redhat.com>

When a storage device rejects a WRITE SAME command we will disable write
same functionality for the device and return -EREMOTEIO to the block
layer. -EREMOTEIO will in turn prevent DM from retrying the I/O and/or
failing the path.

Yiwen Jiang discovered a small race where WRITE SAME requests issued
simultaneously would cause -EIO to be returned. This happened because
any requests being prepared after WRITE SAME had been disabled for the
device caused us to return BLKPREP_KILL. The latter caused the block
layer to return -EIO upon completion.

To overcome this we introduce BLKPREP_INVALID which indicates that this
is an invalid request for the device. blk_peek_request() is modified to
return -EREMOTEIO in that case.

Reported-by: Yiwen Jiang <jiangyiwen@huawei.com>
Suggested-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

---

I contemplated making blk_peek_request() use rq->errors to decide what
to return up the stack. However, I cringed at mixing errnos and SCSI
midlayer status information in the same location.
---
 block/blk-core.c       | 6 ++++--
 drivers/scsi/sd.c      | 4 ++--
 include/linux/blkdev.h | 9 ++++++---
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 33e2f62d5062..ee833af2f892 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2446,14 +2446,16 @@ struct request *blk_peek_request(struct request_queue *q)
 
 			rq = NULL;
 			break;
-		} else if (ret == BLKPREP_KILL) {
+		} else if (ret == BLKPREP_KILL || ret == BLKPREP_INVALID) {
+			int err = ret == BLKPREP_INVALID ? -EREMOTEIO : -EIO;
+
 			rq->cmd_flags |= REQ_QUIET;
 			/*
 			 * Mark this request as started so we don't trigger
 			 * any debug logic in the end I/O path.
 			 */
 			blk_start_request(rq);
-			__blk_end_request_all(rq, -EIO);
+			__blk_end_request_all(rq, err);
 		} else {
 			printk(KERN_ERR "%s: bad return=%d\n", __func__, ret);
 			break;
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index ec163d08f6c3..6e841c6da632 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -761,7 +761,7 @@ static int sd_setup_discard_cmnd(struct scsi_cmnd *cmd)
 		break;
 
 	default:
-		ret = BLKPREP_KILL;
+		ret = BLKPREP_INVALID;
 		goto out;
 	}
 
@@ -839,7 +839,7 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
 	int ret;
 
 	if (sdkp->device->no_write_same)
-		return BLKPREP_KILL;
+		return BLKPREP_INVALID;
 
 	BUG_ON(bio_offset(bio) || bio_iovec(bio).bv_len != sdp->sector_size);
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c70e3588a48c..e990d181625a 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -680,9 +680,12 @@ static inline bool blk_write_same_mergeable(struct bio *a, struct bio *b)
 /*
  * q->prep_rq_fn return values
  */
-#define BLKPREP_OK		0	/* serve it */
-#define BLKPREP_KILL		1	/* fatal error, kill */
-#define BLKPREP_DEFER		2	/* leave on queue */
+enum {
+	BLKPREP_OK,		/* serve it */
+	BLKPREP_KILL,		/* fatal error, kill, return -EIO */
+	BLKPREP_INVALID,	/* invalid command, kill, return -EREMOTEIO */
+	BLKPREP_DEFER,		/* leave on queue */
+};
 
 extern unsigned long blk_max_low_pfn, blk_max_pfn;
 
-- 
2.7.0


  parent reply	other threads:[~2016-02-04  6:48 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-04  2:08 [dm-devel] [PATCH] dm-mpath: fix a tiny case which can cause an infinite loop jiangyiwen
2016-02-04  3:24 ` Mike Snitzer
2016-02-04  3:49   ` jiangyiwen
2016-02-04  4:25     ` Mike Snitzer
2016-02-04  5:03       ` Martin K. Petersen
2016-02-04  6:48       ` Martin K. Petersen [this message]
2016-02-04  9:16         ` [PATCH] block/sd: Return -EREMOTEIO when WRITE SAME and DISCARD are disabled jiangyiwen
2016-02-05  3:13           ` Martin K. Petersen
2016-02-05  3:39             ` jiangyiwen
2016-02-16 12:14             ` jiangyiwen
2016-02-18  0:22               ` Martin K. Petersen
2016-02-04 14:14         ` Hannes Reinecke
2016-02-04 16:36         ` Ewan Milne
2016-02-05  3:16           ` Martin K. Petersen
2016-02-04  8:25       ` dm-mpath: fix a tiny case which can cause an infinite loop jiangyiwen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1454568483-11298-1-git-send-email-martin.petersen@oracle.com \
    --to=martin.petersen@oracle.com \
    --cc=jiangyiwen@huawei.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=snitzer@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).