Thread (5 messages) 5 messages, 3 authors, 2017-02-14

Re: [PATCH --resend 1/2] md: disable WRITE SAME if it fails for linear/raid0

From: Shaohua Li <shli@kernel.org>
Date: 2017-02-14 00:04:19

On Tue, Feb 14, 2017 at 10:42:32AM +1100, Neil Brown wrote:
On Mon, Feb 13 2017, Shaohua Li wrote:
quoted
This makes md do the same thing as dm for write same IO failure. Please
see 7eee4ae(dm: disable WRITE SAME if it fails) for details why we need
this.

Also reported here: https://bugzilla.kernel.org/show_bug.cgi?id=118581

Signed-off-by: Shaohua Li <redacted>
---
 drivers/md/linear.c |  6 +++++-
 drivers/md/md.c     | 45 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/md/md.h     |  2 ++
 drivers/md/raid0.c  |  6 +++++-
 4 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index 26a73b2..bebc834 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -291,7 +291,11 @@ static void linear_make_request(struct mddev *mddev, struct bio *bio)
 				trace_block_bio_remap(bdev_get_queue(split->bi_bdev),
 						      split, disk_devt(mddev->gendisk),
 						      bio_sector);
-			generic_make_request(split);
+			if (bio_op(split) == REQ_OP_WRITE_SAME)
+				generic_make_request(md_writesame_setup(mddev,
+								split));
+			else
+				generic_make_request(split);
 		}
 	} while (split != bio);
 	return;
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 13020e5..7354f0b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -312,6 +312,51 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
 	return BLK_QC_T_NONE;
 }
 
+struct md_writesame_data {
+	struct bio *orig_bio;
+	struct mddev *mddev;
+	struct bio cloned_bio;
+};
+
+static void md_writesame_endio(struct bio *bio)
+{
+	struct md_writesame_data *data = bio->bi_private;
+
+	if (bio->bi_error == -EREMOTEIO &&
+	    !bdev_get_queue(bio->bi_bdev)->limits.max_write_same_sectors)
+		data->mddev->queue->limits.max_write_same_sectors = 0;
What would be *really* nice is if a block device could send a
reconfigure message to its 'holder' (bd_holder).  This could include
device size changes and, for this, changes to max_write_same_sectors.
There are probably other changes that can usefully be propagated.

But for this patch, wouldn't it be easier, and maybe more efficient, to
do

  if (bio_op(split) == REQ_OP_WRITE_SAME &&
      !bdev_get_queue(bio->bi_bdev)->limits.max_write_same_sectors)
           mddev->queue->limits.max_write_same_sectors = 0;
  generic_make_request(split);

???
If there is some reason that can't work, then the patch as it stands
look OK to me.
So we don't disable writesame in the first IO error and do it until a new
writesame comes? Good idea and much simpler! Let me check if it works.

Thanks,
Shaohua
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help