Version 1 super block

PATCH In review Mike Tran 2 participants 4 messages Jan 16, 2005 – Jan 24, 2005 MD RAID

lore

Share & tools

lore

Mike Tran

author

2005-01-16 02:50 UTC #0 lore raw

PATCH 1/1 Version 1 super block

Hello Neil,

I found 2 problems in the kernel MD driver:
1) if disk index 0 is used, counting of max_dev is wrong
2) missing checksum calculation

Please consider the attached patch for 2.6.10 kernel. --
Thanks,
Mike T.

linux-2.6.10/drivers/md/md.c +6 -1

@@ -939,9 +939,13 @@
 		sb->resync_offset = cpu_to_le64(0);
 
 	max_dev = 0;
-	ITERATE_RDEV(mddev,rdev2,tmp)
+	i = 0;
+	ITERATE_RDEV(mddev,rdev2,tmp) {
+		i++;
 		if (rdev2->desc_nr > max_dev)
 			max_dev = rdev2->desc_nr;
+	}
+	max_dev = max(max_dev, i);
 	
 	sb->max_dev = cpu_to_le32(max_dev);
 	for (i=0; i<max_dev;i++)

@@ -958,6 +962,7 @@
 	}
 
 	sb->recovery_offset = cpu_to_le64(0); /* not supported yet */
+	sb->sb_csum = calc_sb_1_csum(sb);
 }

Mike Tran

author

2005-01-21 15:59 UTC #1 lore raw

Hello Neil,

I've been wondering if you had a chance to look at this patch. Please let me know.

--
Thanks,
Mike T.

Quoted 38 lines · Mike Tran

Hello Neil,
I found 2 problems in the kernel MD driver:
1) if disk index 0 is used, counting of max_dev is wrong
2) missing checksum calculation
Please consider the attached patch for 2.6.10 kernel. --
Thanks,
Mike T.
______________________________________________________________________
linux-2.6.10/drivers/md/md.c +6 -1
@@ -939,9 +939,13 @@
 		sb->resync_offset = cpu_to_le64(0);
 
 	max_dev = 0;
-	ITERATE_RDEV(mddev,rdev2,tmp)
+	i = 0;
+	ITERATE_RDEV(mddev,rdev2,tmp) {
+		i++;
 		if (rdev2->desc_nr > max_dev)
 			max_dev = rdev2->desc_nr;
+	}
+	max_dev = max(max_dev, i);
 
 	sb->max_dev = cpu_to_le32(max_dev);
 	for (i=0; i<max_dev;i++)
@@ -958,6 +962,7 @@
 	}
 
 	sb->recovery_offset = cpu_to_le64(0); /* not supported yet */
+	sb->sb_csum = calc_sb_1_csum(sb);
 }
 
 

Neil Brown

2005-01-24 01:38 UTC #2 lore raw

Quoted 4 lines · On Friday January 21, [email protected] wrote:

Hello Neil,
I've been wondering if you had a chance to look at this patch. Please let me know.

No I hadn't, but I have now.

Quoted 6 lines · Mike Tran

Quoted 4 lines
Hello Neil,
I found 2 problems in the kernel MD driver:
1) if disk index 0 is used, counting of max_dev is wrong

Yes, but I think that the correct fix is simply to add 1 to max_dev, rather than setting it to max(max_dev, i).
It should really be renamed to "max_devs" (i.e. the maximum number of devices) rather than "max_dev" (i.e. the maximum device number). Possibly "num_slots" wold be an even better name...

Quoted 1 line

Quoted 1 line
2) missing checksum calculation

Yes .... how did I miss that :-(

Quoted 2 lines

Quoted 2 lines
Please consider the attached patch for 2.6.10 kernel.

How about the following?
I'll add a changelog entry and forward it to Andrew shortly.

thanks,
NeilBrown

----------- Diffstat output ------------

 ./drivers/md/md.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

drivers/md/md.c renamed from drivers/md/md.c~current~ +3 -2

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -983,8 +983,8 @@ static void super_1_sync(mddev_t *mddev,
 
 	max_dev = 0;
 	ITERATE_RDEV(mddev,rdev2,tmp)
-		if (rdev2->desc_nr > max_dev)
-			max_dev = rdev2->desc_nr;
+		if (rdev2->desc_nr+1 > max_dev)
+			max_dev = rdev2->desc_nr+1;
 	
 	sb->max_dev = cpu_to_le32(max_dev);
 	for (i=0; i<max_dev;i++)

@@ -1001,6 +1001,7 @@ static void super_1_sync(mddev_t *mddev,
 	}
 
 	sb->recovery_offset = cpu_to_le64(0); /* not supported yet */
+	sb->sb_csum = calc_sb_1_csum(sb);
 }

Mike Tran

author

2005-01-24 19:16 UTC #3 lore raw

Quoted 27 lines · Neil Brown

Quoted 4 lines · On Friday January 21, [email protected] wrote:
Hello Neil,
I've been wondering if you had a chance to look at this patch. Please let me know.
No I hadn't, but I have now.
Quoted 6 lines · Mike Tran
Quoted 4 lines
Hello Neil,
I found 2 problems in the kernel MD driver:
1) if disk index 0 is used, counting of max_dev is wrong
Yes, but I think that the correct fix is simply to add 1 to max_dev, rather than setting it to max(max_dev, i).
It should really be renamed to "max_devs" (i.e. the maximum number of devices) rather than "max_dev" (i.e. the maximum device number). Possibly "num_slots" wold be an even better name...
Quoted 1 line
Quoted 1 line
2) missing checksum calculation
Yes .... how did I miss that :-(
Quoted 1 line
Quoted 1 line

Quoted 6 lines

Quoted 1 line
Quoted 1 line
Please consider the attached patch for 2.6.10 kernel.
How about the following?
I'll add a changelog entry and forward it to Andrew shortly.

sounds good.

Thanks,
Mike T.