All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Riemer <sebastian.riemer@profitbricks.com>
To: Ben Hutchings <ben@decadent.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>, Neil Brown <neilb@suse.de>,
	linux-raid@vger.kernel.org, 696650@bugs.debian.org
Subject: [PATCH v4] md: protect against crash upon fsync on ro array
Date: Tue, 29 Jan 2013 12:19:22 +0100	[thread overview]
Message-ID: <5107B03A.9030109@profitbricks.com> (raw)
In-Reply-To: <1359438326.3367.41.camel@deadeye.wl.decadent.org.uk>

[-- Attachment #1: Type: text/plain, Size: 798 bytes --]

On 29.01.2013 06:45, Ben Hutchings wrote:
> 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?

Good point. But it's better to use "bio_sectors" as bi_size is the
important information. I've seen that DRBD detects empty flushes the
same way.

For testing I've disabled the "set_disk_ro" in "md_set_readonly". Then,
I did direct IO writes on the read-only array and it worked - I've
received "Input/output error" in the user space.

I've attached version 4 of the patch.

Any further objection?

[-- Attachment #2: 0001-md-protect-against-crash-upon-fsync-on-ro-array.txt --]
[-- Type: text/plain, Size: 1408 bytes --]

From adfac4df99edc1a83dced9c732464634d3381a9f Mon Sep 17 00:00:00 2001
From: Sebastian Riemer <sebastian.riemer@profitbricks.com>
Date: Fri, 25 Jan 2013 12:46:59 +0100
Subject: [PATCH v4] 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 return success upon a flush
request but we return -EROFS for other writes.
We detect flush requests by checking if the bio has zero sectors.

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: NeilBrown <neilb@suse.de>
Signed-off-by: Sebastian Riemer <sebastian.riemer@profitbricks.com>
Reported-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/md/md.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 3db3d1b..1e634a6 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, bio_sectors(bio) == 0 ? 0 : -EROFS);
+		return;
+	}
 	smp_rmb(); /* Ensure implications of  'active' are visible */
 	rcu_read_lock();
 	if (mddev->suspended) {
-- 
1.7.1


  reply	other threads:[~2013-01-29 11:19 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-20 18:44 fsync() on read-only RAID triggers BUG Ben Hutchings
2013-01-25 15:09 ` Sebastian Riemer
2013-01-26 19:44   ` Ben Hutchings
2013-01-27 16:39     ` Christoph Hellwig
2013-01-28 10:32       ` [PATCH v2] md: protect against crash upon fsync on ro array Sebastian Riemer
2013-01-28 12:39         ` [PATCH v3] " Sebastian Riemer
2013-01-29  5:45           ` Ben Hutchings
2013-01-29 11:19             ` Sebastian Riemer [this message]
2013-01-29 12:29               ` [PATCH v5] " Paul Menzel
2013-01-31 19:35                 ` Sebastian Riemer
2013-02-04 22:30                   ` NeilBrown

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=5107B03A.9030109@profitbricks.com \
    --to=sebastian.riemer@profitbricks.com \
    --cc=696650@bugs.debian.org \
    --cc=ben@decadent.org.uk \
    --cc=hch@infradead.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=neilb@suse.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.