Re: Raid5 race patch (fwd)

[Neil Brown <neilb@cse.unsw.edu.au>]

On Friday March 15, neilb@cse.unsw.edu.au wrote:
> 
> I have even tried to duplicate your results. ...

Ok, here's another try.  I managed to get a deadlock when creating an
array.  Your problem could well be related.


Could you try this patch please?  It changes all_mddevs_rem into a r/w
semaphore.


Thanks,
NeilBrown

--- ./drivers/md/md.c	2002/03/15 04:05:03	1.3
+++ ./drivers/md/md.c	2002/03/15 04:07:37
@@ -34,6 +34,7 @@
 #include <linux/sysctl.h>
 #include <linux/raid/xor.h>
 #include <linux/devfs_fs_kernel.h>
+#include <linux/rwsem.h>
 
 #include <linux/init.h>
 
@@ -149,7 +150,7 @@
  *     increase ->active or take a lock
  */
 static MD_LIST_HEAD(all_mddevs);
-static DECLARE_MUTEX(all_mddevs_sem);
+static DECLARE_RWSEM(all_mddevs_sem);
 
 /*
  * The mapping between kdev and mddev is not necessary a simple
@@ -164,13 +165,13 @@
 	mddev_t *mddev;
 	if (MAJOR(dev) != MD_MAJOR)
 		BUG();
-	down(&all_mddevs_sem);
+	down_read(&all_mddevs_sem);
         mddev = mddev_map[MINOR(dev)].mddev;
 	if (mddev &&  !mddev->dying)
 		atomic_inc(&mddev->active);
 	else
 		mddev = NULL;
-	up(&all_mddevs_sem);
+	up_read(&all_mddevs_sem);
 	return mddev;
 }
 
@@ -179,7 +180,7 @@
 	mddev_t *mddev;
 	if (MAJOR(dev) != MD_MAJOR)
 		BUG();
-	down(&all_mddevs_sem);
+	down_read(&all_mddevs_sem);
         mddev = mddev_map[MINOR(dev)].mddev;
 	if (mddev) {
 		if (mddev->dying)
@@ -187,7 +188,7 @@
 		else
 			lock_mddev(mddev);
 	}
-	up(&all_mddevs_sem);
+	up_read(&all_mddevs_sem);
 	return mddev;
 }
 static inline mddev_t * kdev_to_mddev_lock_interruptible (kdev_t dev, int *err)
@@ -195,7 +196,7 @@
 	mddev_t *mddev;
 	if (MAJOR(dev) != MD_MAJOR)
 		BUG();
-	down(&all_mddevs_sem);
+	down_read(&all_mddevs_sem);
         mddev = mddev_map[MINOR(dev)].mddev;
 	*err = 0;
 	if (mddev) {
@@ -205,7 +206,7 @@
 		} else
 			*err = lock_mddev_interruptible(mddev);
 	}
-	up(&all_mddevs_sem);
+	up_read(&all_mddevs_sem);
 	return mddev;
 }
 
@@ -286,10 +287,10 @@
 	 * personalities can create additional mddevs
 	 * if necessary.
 	 */
-	down(&all_mddevs_sem);
+	down_write(&all_mddevs_sem);
 	add_mddev_mapping(mddev, dev, 0);
 	md_list_add(&mddev->all_mddevs, &all_mddevs);
-	up(&all_mddevs_sem);
+	up_write(&all_mddevs_sem);
 
 	MOD_INC_USE_COUNT;
 
@@ -798,10 +799,10 @@
 	md_hd_struct[mdidx(mddev)].nr_sects = 0;
 
 
-	down(&all_mddevs_sem);
+	down_write(&all_mddevs_sem);
 	del_mddev_mapping(mddev, MKDEV(MD_MAJOR, mdidx(mddev)));
 	md_list_del(&mddev->all_mddevs);
-	up(&all_mddevs_sem);
+	up_write(&all_mddevs_sem);
 	kfree(mddev);
 	MOD_DEC_USE_COUNT;
 }
@@ -872,7 +873,7 @@
 	printk("md:	**********************************\n");
 	printk("md:	* <COMPLETE RAID STATE PRINTOUT> *\n");
 	printk("md:	**********************************\n");
-/*	down(&all_mddevs_sem); */
+	down_read(&all_mddevs_sem);
 	ITERATE_MDDEV/*_LOCK*/(mddev,tmp) {
 		printk("md%d: ", mdidx(mddev));
 
@@ -888,7 +889,7 @@
 		ITERATE_RDEV(mddev,rdev,tmp2)
 			print_rdev(rdev);
 	}
-/*	up(&all_mddevs_sem); */
+	up_read(&all_mddevs_sem);
 	printk("md:	**********************************\n");
 	printk("\n");
 }
@@ -3252,7 +3253,7 @@
 	else
 		sz += sprintf(page+sz, "%d sectors\n", read_ahead[MD_MAJOR]);
 
-	down(&all_mddevs_sem);
+	down_read(&all_mddevs_sem);
 	ITERATE_MDDEV_LOCK(mddev,tmp) {
 		sz += sprintf(page + sz, "md%d : %sactive", mdidx(mddev),
 						mddev->pers ? "" : "in");
@@ -3297,7 +3298,7 @@
 		}
 		sz += sprintf(page + sz, "\n");
 	}
-	up(&all_mddevs_sem);
+	up_read(&all_mddevs_sem);
 	sz += status_unused(page + sz);
 
 	return sz;
@@ -3429,7 +3430,7 @@
 
 recheck:
 	serialize = 0;
-	down(&all_mddevs_sem);
+	down_read(&all_mddevs_sem);
 	ITERATE_MDDEV(mddev2,tmp) {
 		if (mddev2 == mddev)
 			continue;
@@ -3441,7 +3442,7 @@
 			break;
 		}
 	}
-	up(&all_mddevs_sem);
+	up_read(&all_mddevs_sem);
 	if (serialize) {
 		interruptible_sleep_on(&resync_wait);
 		if (md_signal_pending(current)) {
@@ -3590,7 +3591,7 @@
 	printk(KERN_INFO "md: recovery thread got woken up ...\n");
 
  restart:
-	down(&all_mddevs_sem);
+	down_read(&all_mddevs_sem);
 	ITERATE_MDDEV_LOCK(mddev,tmp) {
 		sb = mddev->sb;
 		if (!sb)
@@ -3619,7 +3620,7 @@
 		if (mddev->pers->diskop(mddev, &spare, DISKOP_SPARE_WRITE))
 			continue;
 		unlock_mddev(mddev);
-		up(&all_mddevs_sem);
+		up_read(&all_mddevs_sem);
 		down(&mddev->recovery_sem);
 		mddev->recovery_running = 1;
 		err = md_do_sync(mddev, spare);
@@ -3671,7 +3672,7 @@
 		unlock_mddev(mddev);
 		goto restart;
 	}
-	up(&all_mddevs_sem);
+	up_read(&all_mddevs_sem);
 	printk(KERN_INFO "md: recovery thread finished ...\n");
 
 }

-
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html