On Mon, 2013-01-28 at 13:39 +0100, Sebastian Riemer wrote:
On 28.01.2013 11:32, Sebastian Riemer wrote:
O.K., then I hope Neil applies the attached patch. I've changed the
return value to success.
This is also something for linux-stable and should apply to many kernel
versions without an issue.
I've tried to race with continuous fsyncs against continuous mdadm -o
/dev/md0; mdadm -w /dev/md0; in parallel but couldn't break it.
Therefore, no additional locking is required and this part can be put
directly before the RCU locking stuff and the suspended handling.
I've attached version 3 of the patch.
From fe0357344877c9b9cc623fd582a4e0670e448317 Mon Sep 17 00:00:00 2001
From: Sebastian Riemer sebastian.rie...@profitbricks.com
Date: Fri, 25 Jan 2013 12:46:59 +0100
Subject: [PATCH v3] md: protect against crash upon fsync on ro array
If an fsync occurrs on a read-only array, we need to send a
completion for the IO and may not increment the active IO count.
Otherwise, we hit a bug trace and can't stop the MD array anymore.
By advice of Christoph Hellwig we silently return success.
Cc: Christoph Hellwig h...@infradead.org
Cc: Ben Hutchings b...@decadent.org.uk
Cc: NeilBrown ne...@suse.de
Signed-off-by: Sebastian Riemer sebastian.rie...@profitbricks.com
Reported-by: Ben Hutchings b...@decadent.org.uk
---
drivers/md/md.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 3db3d1b..6ba20f7 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -307,6 +307,10 @@ static void md_make_request(struct request_queue
*q, struct bio *bio)
bio_io_error(bio);
return;
}
+ if (mddev-ro == 1 unlikely(rw == WRITE)) {
+ bio_endio(bio, 0);
+ return;
+ }
smp_rmb(); /* Ensure implications of 'active' are visible */
rcu_read_lock();
if (mddev-suspended) {
I'm slightly uneasy about returning 0 for all writes to a read-only
deivce, because if someone ever fails to check the read-only flag
elsewhere this could turn into silent data loss. Does this work:
BUG_ON(bio_segments(bio));
Or should it be:
bio_endio(bio, bio_segments(bio) == 0 ? 0 : -EROFS);
to make the error survivable?
Ben.
--
Ben Hutchings
If more than one person is responsible for a bug, no one is at fault.
signature.asc
Description: This is a digitally signed message part
