On 2003-09-19T12:08:29,
Neil Brown <neilb@cse.unsw.edu.au> said:
Hi Neil,
your patch needs the following two addendums.
Andi Kleen: Locking in md_seq_next is wrong, should happen in
md_seq_start
Andrea & I: Refcounting for the raidautostart feature is wrong.
This is actually a long-standing bug which is catched by your diffs,
which notice the wrong counter and BUG().
What would happen: user-space open()s /dev/md0, issues
ioctl(RAID_AUTOSTART) - which creates all md devices and sets active ==
0 ultimately -, and then close()s /dev/md0, which decrements active to
-1. Oops next time md_make_request gets called.
Patch fixes this and adds safeguard to md_release() already.
Sincerely,
Lars Marowsky-Brée <lmb@suse.de>
--
High Availability & Clustering ever tried. ever failed. no matter.
SuSE Labs try again. fail again. fail better.
Research & Development, SUSE LINUX AG -- Samuel Beckett
--- linux/drivers/md/md.c-o 2003-09-24 09:45:39.000000000 +0000 +++ linux/drivers/md/md.c 2003-09-24 09:47:23.000000000 +0000 @@ -3287,6 +3287,7 @@ if (l > 0x10000) return NULL; + down_read(&all_mddevs_sem); if (!l--) /* header */ return (void*)1; @@ -3304,7 +3305,6 @@ struct list_head *tmp; mddev_t *next_mddev, *mddev = v; - down_read(&all_mddevs_sem); ++*pos; if (v == (void*)2) return NULL;
--- linux-2.4.21/drivers/md/md.c 2003-09-24 10:54:35.000000000 +0200
+++ linux-2.4.21.new/drivers/md/md.c 2003-09-24 10:53:34.000000000 +0200
@@ -60,7 +60,7 @@
#endif
#ifndef MODULE
-static void autostart_arrays (void);
+static void autostart_arrays (kdev_t);
#endif
static mdk_personality_t *pers[MAX_PERSONALITY];
@@ -2753,7 +2753,7 @@
#ifndef MODULE
case RAID_AUTORUN:
err = 0;
- autostart_arrays();
+ autostart_arrays(dev);
goto done;
#endif
@@ -3017,8 +3017,11 @@
{
mddev_t *mddev = kdev_to_mddev(inode->i_rdev);
if (mddev) {
- atomic_dec(&mddev->active);
- atomic_dec(&mddev->active);
+ if (atomic_dec_and_test(&mddev->active))
+ /* Refcount should _never_ drop to zero here! */
+ BUG();
+ else
+ atomic_dec(&mddev->active);
}
return 0;
}
@@ -3923,7 +3926,7 @@
}
-static void autostart_arrays(void)
+static void autostart_arrays(kdev_t countdev)
{
mdk_rdev_t *rdev;
int i;
@@ -3954,7 +3957,7 @@
}
dev_cnt = 0;
- autorun_devices(-1);
+ autorun_devices(countdev);
}
static struct {
@@ -4183,7 +4186,7 @@
if (raid_setup_args.noautodetect)
printk(KERN_INFO "md: Skipping autodetection of RAID arrays. (raid=noautodetect)\n");
else
- autostart_arrays();
+ autostart_arrays(-1);
md_setup_drive();
return 0;
}
Attachment:
pgp00054.pgp
Description: PGP signature
