On Wednesday May 16, [EMAIL PROTECTED] wrote:
>
> (more patches to come. They will go to Linus, Alan, and linux-raid only).
This is the next one, which actually addresses the "NULL Checking
Bug".
There are two places in the the raid code which allocate memory
without (properly) checking for failure and which are fixed in "ac",
both in raid5.c. One in grow_buffers and one in __check_consistency.
The one in grow_buffers is definately right and included below in a
slightly different form - fewer characters.
The one in __check_consistency is best fixed by simply removing
__check_consistency.
__check_consistency reads stipes at some offset into the array and
checks that parity is correct. This is called once, but the result is
ignored.
Presumably this is a hangover from times gone by when the superblock
didn't have proper versioning and there was no auto-rebuild process.
It is now irrelevant and can go.
There is similar code in raid1.c which should also go.
This patch removes said code.
NeilBrown
--- ./drivers/md/raid5.c 2001/05/16 05:14:39 1.2
+++ ./drivers/md/raid5.c 2001/05/16 05:27:20 1.3
@@ -156,9 +156,9 @@
return 1;
memset(bh, 0, sizeof (struct buffer_head));
init_waitqueue_head(&bh->b_wait);
- page = alloc_page(priority);
- bh->b_data = page_address(page);
- if (!bh->b_data) {
+ if ((page = alloc_page(priority)))
+ bh->b_data = page_address(page);
+ else {
kfree(bh);
return 1;
}
@@ -1256,76 +1256,6 @@
printk("raid5: resync finished.\n");
}
-static int __check_consistency (mddev_t *mddev, int row)
-{
- raid5_conf_t *conf = mddev->private;
- kdev_t dev;
- struct buffer_head *bh[MD_SB_DISKS], *tmp = NULL;
- int i, ret = 0, nr = 0, count;
- struct buffer_head *bh_ptr[MAX_XOR_BLOCKS];
-
- if (conf->working_disks != conf->raid_disks)
- goto out;
- tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
- tmp->b_size = 4096;
- tmp->b_page = alloc_page(GFP_KERNEL);
- tmp->b_data = page_address(tmp->b_page);
- if (!tmp->b_data)
- goto out;
- md_clear_page(tmp->b_data);
- memset(bh, 0, MD_SB_DISKS * sizeof(struct buffer_head *));
- for (i = 0; i < conf->raid_disks; i++) {
- dev = conf->disks[i].dev;
- set_blocksize(dev, 4096);
- bh[i] = bread(dev, row / 4, 4096);
- if (!bh[i])
- break;
- nr++;
- }
- if (nr == conf->raid_disks) {
- bh_ptr[0] = tmp;
- count = 1;
- for (i = 1; i < nr; i++) {
- bh_ptr[count++] = bh[i];
- if (count == MAX_XOR_BLOCKS) {
- xor_block(count, &bh_ptr[0]);
- count = 1;
- }
- }
- if (count != 1) {
- xor_block(count, &bh_ptr[0]);
- }
- if (memcmp(tmp->b_data, bh[0]->b_data, 4096))
- ret = 1;
- }
- for (i = 0; i < conf->raid_disks; i++) {
- dev = conf->disks[i].dev;
- if (bh[i]) {
- bforget(bh[i]);
- bh[i] = NULL;
- }
- fsync_dev(dev);
- invalidate_buffers(dev);
- }
- free_page((unsigned long) tmp->b_data);
-out:
- if (tmp)
- kfree(tmp);
- return ret;
-}
-
-static int check_consistency (mddev_t *mddev)
-{
- if (__check_consistency(mddev, 0))
-/*
- * We are not checking this currently, as it's legitimate to have
- * an inconsistent array, at creation time.
- */
- return 0;
-
- return 0;
-}
-
static int raid5_run (mddev_t *mddev)
{
raid5_conf_t *conf;
@@ -1483,12 +1413,6 @@
if (conf->working_disks != sb->raid_disks) {
printk(KERN_ALERT "raid5: md%d, not all disks are operational --
trying to recover array\n", mdidx(mddev));
start_recovery = 1;
- }
-
- if (!start_recovery && (sb->state & (1 << MD_SB_CLEAN)) &&
- check_consistency(mddev)) {
- printk(KERN_ERR "raid5: detected raid-5 superblock xor inconsistency
-- running resync\n");
- sb->state &= ~(1 << MD_SB_CLEAN);
}
{
--- ./drivers/md/raid1.c 2001/05/16 05:14:39 1.2
+++ ./drivers/md/raid1.c 2001/05/16 05:27:20 1.3
@@ -1448,69 +1448,6 @@
}
}
-/*
- * This will catch the scenario in which one of the mirrors was
- * mounted as a normal device rather than as a part of a raid set.
- *
- * check_consistency is very personality-dependent, eg. RAID5 cannot
- * do this check, it uses another method.
- */
-static int __check_consistency (mddev_t *mddev, int row)
-{
- raid1_conf_t *conf = mddev_to_conf(mddev);
- int disks = MD_SB_DISKS;
- kdev_t dev;
- struct buffer_head *bh = NULL;
- int i, rc = 0;
- char *buffer = NULL;
-
- for (i = 0; i < disks; i++) {
- printk("(checking disk %d)\n",i);
- if (!conf->mirrors[i].operational)
- continue;
- printk("(really checking disk %d)\n",i);
- dev = conf->mirrors[i].dev;
- set_blocksize(dev, 4096);
- if ((bh = bread(dev, row / 4, 4096)) == NULL)
- break;
- if (!buffer) {
- buffer = (char *) __get_free_page(GFP_KERNEL);
- if (!buffer)
- break;
- memcpy(buffer, bh->b_data, 4096);
- } else if (memcmp(buffer, bh->b_data, 4096)) {
- rc = 1;
- break;
- }
- bforget(bh);
- fsync_dev(dev);
- invalidate_buffers(dev);
- bh = NULL;
- }
- if (buffer)
- free_page((unsigned long) buffer);
- if (bh) {
- dev = bh->b_dev;
- bforget(bh);
- fsync_dev(dev);
- invalidate_buffers(dev);
- }
- return rc;
-}
-
-static int check_consistency (mddev_t *mddev)
-{
- if (__check_consistency(mddev, 0))
-/*
- * we do not do this currently, as it's perfectly possible to
- * have an inconsistent array when it's freshly created. Only
- * newly written data has to be consistent.
- */
- return 0;
-
- return 0;
-}
-
#define INVALID_LEVEL KERN_WARNING \
"raid1: md%d: raid level not set to mirroring (%d)\n"
@@ -1541,9 +1478,6 @@
#define NONE_OPERATIONAL KERN_ERR \
"raid1: no operational mirrors for md%d\n"
-#define RUNNING_CKRAID KERN_ERR \
-"raid1: detected mirror differences -- running resync\n"
-
#define ARRAY_IS_ACTIVE KERN_INFO \
"raid1: raid set md%d active with %d out of %d mirrors\n"
@@ -1725,17 +1659,6 @@
start_recovery = 1;
}
- if (!start_recovery && (sb->state & (1 << MD_SB_CLEAN))) {
- /*
- * we do sanity checks even if the device says
- * it's clean ...
- */
- if (check_consistency(mddev)) {
- printk(RUNNING_CKRAID);
- sb->state &= ~(1 << MD_SB_CLEAN);
- }
- }
-
{
const char * name = "raid1d";
@@ -1805,7 +1728,6 @@
#undef OPERATIONAL
#undef SPARE
#undef NONE_OPERATIONAL
-#undef RUNNING_CKRAID
#undef ARRAY_IS_ACTIVE
static int raid1_stop_resync (mddev_t *mddev)
-
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to [EMAIL PROTECTED]
