<html><head><meta name="color-scheme" content="light dark"></head><body><pre style="word-wrap: break-word; white-space: pre-wrap;">
From: NeilBrown &lt;neilb@cse.unsw.edu.au&gt;

When we get a pointer and check it is non-null, we should not get the pointer
again but should instead use the pointer we got in the first place...

Signed-off-by: Neil Brown &lt;neilb@cse.unsw.edu.au&gt;
Signed-off-by: Andrew Morton &lt;akpm@osdl.org&gt;
---

 25-akpm/drivers/md/raid1.c |   40 ++++++++++++++++++++++++++++------------
 1 files changed, 28 insertions(+), 12 deletions(-)

diff -puN drivers/md/raid1.c~md-remove-possible-oops-in-md-raid1 drivers/md/raid1.c
--- 25/drivers/md/raid1.c~md-remove-possible-oops-in-md-raid1	2005-02-17 18:00:49.000000000 -0800
+++ 25-akpm/drivers/md/raid1.c	2005-02-17 18:00:49.000000000 -0800
@@ -338,6 +338,7 @@ static int read_balance(conf_t *conf, r1
 	int new_disk = conf-&gt;last_used, disk = new_disk;
 	const int sectors = r1_bio-&gt;sectors;
 	sector_t new_distance, current_distance;
+	mdk_rdev_t *new_rdev, *rdev;
 
 	rcu_read_lock();
 	/*
@@ -345,13 +346,14 @@ static int read_balance(conf_t *conf, r1
 	 * device if no resync is going on, or below the resync window.
 	 * We take the first readable disk when above the resync window.
 	 */
+ retry:
 	if (conf-&gt;mddev-&gt;recovery_cp &lt; MaxSector &amp;&amp;
 	    (this_sector + sectors &gt;= conf-&gt;next_resync)) {
 		/* Choose the first operation device, for consistancy */
 		new_disk = 0;
 
-		while (!conf-&gt;mirrors[new_disk].rdev ||
-		       !conf-&gt;mirrors[new_disk].rdev-&gt;in_sync) {
+		while ((new_rdev=conf-&gt;mirrors[new_disk].rdev) == NULL ||
+		       !new_rdev-&gt;in_sync) {
 			new_disk++;
 			if (new_disk == conf-&gt;raid_disks) {
 				new_disk = -1;
@@ -363,8 +365,8 @@ static int read_balance(conf_t *conf, r1
 
 
 	/* make sure the disk is operational */
-	while (!conf-&gt;mirrors[new_disk].rdev ||
-	       !conf-&gt;mirrors[new_disk].rdev-&gt;in_sync) {
+	while ((new_rdev=conf-&gt;mirrors[new_disk].rdev) == NULL ||
+	       !new_rdev-&gt;in_sync) {
 		if (new_disk &lt;= 0)
 			new_disk = conf-&gt;raid_disks;
 		new_disk--;
@@ -393,18 +395,20 @@ static int read_balance(conf_t *conf, r1
 			disk = conf-&gt;raid_disks;
 		disk--;
 
-		if (!conf-&gt;mirrors[disk].rdev ||
-		    !conf-&gt;mirrors[disk].rdev-&gt;in_sync)
+		if ((rdev=conf-&gt;mirrors[disk].rdev) == NULL ||
+		    !rdev-&gt;in_sync)
 			continue;
 
-		if (!atomic_read(&amp;conf-&gt;mirrors[disk].rdev-&gt;nr_pending)) {
+		if (!atomic_read(&amp;rdev-&gt;nr_pending)) {
 			new_disk = disk;
+			new_rdev = rdev;
 			break;
 		}
 		new_distance = abs(this_sector - conf-&gt;mirrors[disk].head_position);
 		if (new_distance &lt; current_distance) {
 			current_distance = new_distance;
 			new_disk = disk;
+			new_rdev = rdev;
 		}
 	} while (disk != conf-&gt;last_used);
 
@@ -414,7 +418,14 @@ rb_out:
 	if (new_disk &gt;= 0) {
 		conf-&gt;next_seq_sect = this_sector + sectors;
 		conf-&gt;last_used = new_disk;
-		atomic_inc(&amp;conf-&gt;mirrors[new_disk].rdev-&gt;nr_pending);
+		atomic_inc(&amp;new_rdev-&gt;nr_pending);
+		if (!new_rdev-&gt;in_sync) {
+			/* cannot risk returning a device that failed
+			 * before we inc'ed nr_pending
+			 */
+			atomic_dec(&amp;new_rdev-&gt;nr_pending);
+			goto retry;
+		}
 	}
 	rcu_read_unlock();
 
@@ -512,6 +523,7 @@ static int make_request(request_queue_t 
 	r1bio_t *r1_bio;
 	struct bio *read_bio;
 	int i, disks;
+	mdk_rdev_t *rdev;
 
 	/*
 	 * Register the new request and wait if the reconstruction
@@ -585,10 +597,14 @@ static int make_request(request_queue_t 
 	disks = conf-&gt;raid_disks;
 	rcu_read_lock();
 	for (i = 0;  i &lt; disks; i++) {
-		if (conf-&gt;mirrors[i].rdev &amp;&amp;
-		    !conf-&gt;mirrors[i].rdev-&gt;faulty) {
-			atomic_inc(&amp;conf-&gt;mirrors[i].rdev-&gt;nr_pending);
-			r1_bio-&gt;bios[i] = bio;
+		if ((rdev=conf-&gt;mirrors[i].rdev) != NULL &amp;&amp;
+		    !rdev-&gt;faulty) {
+			atomic_inc(&amp;rdev-&gt;nr_pending);
+			if (rdev-&gt;faulty) {
+				atomic_dec(&amp;rdev-&gt;nr_pending);
+				r1_bio-&gt;bios[i] = NULL;
+			} else
+				r1_bio-&gt;bios[i] = bio;
 		} else
 			r1_bio-&gt;bios[i] = NULL;
 	}
_
</pre></body></html>