Re: v3.5 regression in IMSM support

[NeilBrown <neilb@xxxxxxx>]

On Tue, 17 Jul 2012 04:05:26 -0500 Brian Downing <bdowning@xxxxxxxxx> wrote:

> I've noticed a regression in IMSM metadata support in v3.5-rc kernels.
> I have a two-disk laptop that I run an IMSM RAID0 on so I can dual-boot
> Windows:
> 
> :; sudo mdadm --detail-platform
>        Platform : Intel(R) Matrix Storage Manager
>         Version : 8.0.0.1039
>     RAID Levels : raid0 raid1
>     Chunk Sizes : 4k 8k 16k 32k 64k 128k
>     2TB volumes : not supported
>       2TB disks : not supported
>       Max Disks : 4
>     Max Volumes : 2 per array, 2 per controller
>  I/O Controller : /sys/devices/pci0000:00/0000:00:1f.2 (SATA)
> 
> :; cat /proc/mdstat 
> Personalities : [raid0] 
> md126 : active raid0 sda[1] sdb[0]
>       625137664 blocks super external:/md127/0 128k chunks
>       
> md127 : inactive sdb[1](S) sda[0](S)
>       4520 blocks super external:imsm
>        
> unused devices: <none>
> 
> What's happened in v3.5 is that it's no longer possible to incrementally
> assemble this array, as for example udev rules do.  This breaks my
> initramfs, and I wind up at a recovery prompt.  'mdadm --assemble --scan'
> is still able to bring up the array enough to get lvm going, though I
> think it's still a little messed up; note the 0 length metadata partition
> reported here:
> 
> :; cat mdstat.bad 
> Personalities : [raid0] 
> md126 : active raid0 sda[1] sdb[0]
>       625137664 blocks super external:/md127/0 128k chunks
>       
> md127 : inactive sdb[1](S) sda[0](S)
>       0 blocks super external:imsm
>        
> unused devices: <none>
> 
> Manually running the incremental mdadm looks like this:
> 
> # mdadm -I /dev/sda
> [   22.514509] md: bind<sda>
> mdadm: failed to add /dev/sda to /dev/md/imsm0: Invalid argument.
> [   22.516151] md: md127 stopped.
> [   22.516234] md: unbind<sda>
> [   22.536399] md: export_rdev(sda)
> 
> The same "mdadm: ... Invalid argument." messages print out from the
> --assemble --scan, yet the array comes up (enough to mount, anyway).
> 
> I bisected down to this commit:
> 
>     commit c6563a8c38fde3c1c7fc925a10bde3ca20799301
>     Author: NeilBrown <neilb@xxxxxxx>
>     Date:   Mon May 21 09:27:00 2012 +1000
> 
>         md: add possibility to change data-offset for devices.

Thank you so much for a very thorough bug report!!

Fix is below - please test and confirm that it fixes your symptoms.

NeilBrown

From a8969e09fd854e9ff302e443c6a185505c3a6624 Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@xxxxxxx>
Date: Wed, 18 Jul 2012 10:34:18 +1000
Subject: [PATCH] md: fix bug in handling of new_data_offset

commit c6563a8c38fde3c1c7fc925a10bde3ca20799301
    md: add possibility to change data-offset for devices.

introduced a 'new_data_offset' attribute which should normally
be the same as 'data_offset', but can be explicitly set to a different
value to allow a reshape operation to move the data.

Unfortunately when the 'data_offset' is explicitly set through
sysfs, the new_data_offset is not also set, so the two would become
out-of-sync incorrectly.

One result of this is that trying to set the 'size' after the
'data_offset' would fail because it is not permitted to set the size
when the 'data_offset' and 'new_data_offset' are different - as that
can be confusing.
Consequently when mdadm tried to do this while assembling an IMSM
array it would fail.

This bug was introduced in 3.5-rc1.

Reported-by: Brian Downing <bdowning@xxxxxxxxx>
Bisected-by: Brian Downing <bdowning@xxxxxxxxx>
Signed-off-by: NeilBrown <neilb@xxxxxxx>

diff --git a/drivers/md/md.c b/drivers/md/md.c
index bdc5c7b..775688e 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2884,6 +2884,7 @@ offset_store(struct md_rdev *rdev, const char *buf, size_t len)
 		 * can be sane */
 		return -EBUSY;
 	rdev->data_offset = offset;
+	rdev->new_data_offset = offset;
 	return len;
 }

Attachment: signature.asc
Description: PGP signature