On 06/12/2013 07:34 AM, Martin K. Petersen wrote:
>>>>>> "hpa" == H Peter Anvin <hpa@xxxxxxxxx> writes:
>
> hpa> The second question is if we should disable WRITE SAME for raid1/10
> hpa> (what about raid0?) for 3.10/stable or if your patch really is
> hpa> sufficient... "just adds another heuristic" makes me nervous.
>
> I think we should disable 1+10 in stable until we get the recovery
> scenario sorted out.
>
> I don't believe there are any problems with raid0.
>
How does this look?
-hpa
>From ac28be1574a6187f4f26bd75217059bf17b13560 Mon Sep 17 00:00:00 2001
From: "H. Peter Anvin" <hpa@xxxxxxxxx>
Date: Wed, 12 Jun 2013 07:37:43 -0700
Subject: [PATCH] raid1,10: Disable WRITE SAME until a recovery strategy is in
place
There are cases where the kernel will believe that the WRITE SAME
command is supported by a block device which does not, in fact,
support WRITE SAME. This currently happens for SATA drivers behind a
SAS controller, but there are probably a hundred other ways that can
happen, including drive firmware bugs.
After receiving an error for WRITE SAME the block layer will retry the
request as a plain write of zeroes, but mdraid will consider the
failure as fatal and consider the drive failed. This has the effect
that all the mirrors containing a specific set of data are each
offlined in very rapid succession resulting in data loss.
However, just bouncing the request back up to the block layer isn't
ideal either, because the whole initial request-retry sequence should
be inside the write bitmap fence, which probably means that md needs
to do its own conversion of WRITE SAME to write zero.
Until the failure scenario has been sorted out, disable WRITE SAME for
raid1 and raid10.
Signed-off-by: H. Peter Anvin <hpa@xxxxxxxxxxxxxxx>
---
drivers/md/raid1.c | 4 ++--
drivers/md/raid10.c | 3 +--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 5595118..914ca0a 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2780,8 +2780,8 @@ static int run(struct mddev *mddev)
return PTR_ERR(conf);
if (mddev->queue)
- blk_queue_max_write_same_sectors(mddev->queue,
- mddev->chunk_sectors);
+ blk_queue_max_write_same_sectors(mddev->queue, 0);
+
rdev_for_each(rdev, mddev) {
if (!mddev->gendisk)
continue;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 59d4daa..807ace8 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3609,8 +3609,7 @@ static int run(struct mddev *mddev)
if (mddev->queue) {
blk_queue_max_discard_sectors(mddev->queue,
mddev->chunk_sectors);
- blk_queue_max_write_same_sectors(mddev->queue,
- mddev->chunk_sectors);
+ blk_queue_max_write_same_sectors(mddev->queue, 0);
blk_queue_io_min(mddev->queue, chunk_size);
if (conf->geo.raid_disks % conf->geo.near_copies)
blk_queue_io_opt(mddev->queue, chunk_size * conf->geo.raid_disks);
--
1.7.11.7
