Thread (5 messages) 5 messages, 2 authors, 2014-03-17

Re: [PATCH 2/2] md: Add support for RAID-1 consistency check feature

From: NeilBrown <hidden>
Date: 2014-03-17 23:10:00
Also in: lkml

On Mon, 17 Mar 2014 16:00:05 +0100 Ralph Mueck [off-list ref] wrote:
quoted hunk ↗ jump to hunk
This patch introduces a consistency check feature for level-1 RAID
arrays that have been created with the md driver.
When enabled, every read request is duplicated and initiated for each
member of the RAID array. All read blocks are compared with their
corresponding blocks on the other array members. If the check fails for
a block, the block is not handed over, but an error code is returned
instead.
As mentioned in the cover letter, the implementation still has some 
unresolved issues.

Signed-off-by: Ralph Mueck <redacted>
Signed-off-by: Matthias Oefelein <redacted>

---
 drivers/md/raid1.c | 252 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 250 insertions(+), 2 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 4a6ca1c..8c64f9a 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -37,6 +37,7 @@
 #include <linux/module.h>
 #include <linux/seq_file.h>
 #include <linux/ratelimit.h>
+#include <linux/gfp.h>
 #include "md.h"
 #include "raid1.h"
 #include "bitmap.h"
@@ -257,6 +258,109 @@ static void call_bio_endio(struct r1bio *r1_bio)
 	}
 }
 
+/* The safe_read version of the raid_end_bio_io() function */
+/* On a read request, we issue requests to all available disks.
+ * Data is returned only if all discs contain the same data
+ */
+static void _endio(struct r1bio *r1_bio)
+{
+	struct bio *bio = r1_bio->master_bio;
+	int done;
+	struct r1conf *conf = r1_bio->mddev->private;
+	sector_t start_next_window = r1_bio->start_next_window;
+	sector_t bi_sector = bio->bi_iter.bi_sector;
+	int disk;
+	struct md_rdev *rdev;
+	int i;
+	struct page *dragptr = NULL;
+	int already_copied = 0;	/* we want to copy the data only once */
+
+	for (disk = 0 ; disk < conf->raid_disks * 2 ; disk++) {
+		struct bio *p = NULL;
+		struct bio *s = NULL;
+		
+		rcu_read_lock();
+		rdev = rcu_dereference(conf->mirrors[disk].rdev);
+		rcu_read_unlock();
You cannot drop rcu_read_lock until you take a reference to rdev, or stop
using it.

+
+		if (r1_bio->bios[disk] == IO_BLOCKED
+			|| rdev == NULL
+			|| test_bit(Unmerged, &rdev->flags)
+			|| test_bit(Faulty, &rdev->flags)) {
+			continue;
+		}
+
+		/* bio_for_each_segment is broken. at least here.. */
+		/* iterate over linked bios */
+		for (p = r1_bio->master_bio, s = r1_bio->bios[disk];
+		     (p != NULL) && (s != NULL);
+		     p = p->bi_next, s = s->bi_next) {
+			/* compare the pages read */
+			for (i = 0; i < r1_bio->bios[disk]->bi_vcnt; i++) {
+				if (dragptr) { /* dragptr points to the previous page */
+					if(memcmp(page_address(r1_bio->bios[disk]->bi_io_vec[0].bv_page),
+						page_address(dragptr),
+						(r1_bio->bios[disk]->bi_io_vec[0].bv_len))) {
+						set_bit(R1BIO_ReadError, &r1_bio->state);
+						clear_bit(R1BIO_Uptodate, &r1_bio->state);
+					}
+				}
+				dragptr = r1_bio->bios[disk]->bi_io_vec[0].bv_page;
+			}
This doesn't make any sense to me at all.  You use 'i' to loop bi_vnt times,
but never use 'i' or change any other variable in that loop (except dragptr
which is always set to the same value).

And you use "bi_next", but next set up any linkage through bi_next.

Confused.

NeilBrown

Attachments

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