Weirdness with DDF arrays (mdadm 3.3)

Weirdness with DDF arrays (mdadm 3.3)

[Francis Moreau]

Hi,

Still testing MD arrays using DDF metadata and find another possible issues :)

I'm creating a new DDF array containing 2 disks. After that
/proc/mdstat looks correct:

# cat /proc/mdstat
Personalities : [raid1]
md124 : active raid1 loop0[1] loop1[0]
      84416 blocks super external:/md125/0 [2/2] [UU]

md125 : inactive loop1[1](S) loop0[0](S)
      65536 blocks super external:ddf

Now I'm stopping the array and restart it by incrementaly adding the 2 disks:
# mdadm --stop /dev/md124
# mdadm --stop /dev/md125
# mdadm -IRs /dev/loop0
# mdadm -IRs /dev/loop1
# cat /proc/mdstat
Personalities : [raid1]
md124 : active (auto-read-only) raid1 loop1[2] loop0[0]
      84416 blocks super external:/md125/0 [2/1] [_U]

md125 : inactive loop1[1](S) loop0[0](S)
      65536 blocks super external:ddf

Parsing mdstat content tells me disk "loop1" have a role number equal
to 2 which is greater than 1 indicating that "loop1" is a spare disk
and the "[_U]" below indicates "loop1" is down".

Why is "loop1" down now ?

I decided to still use the md device by creating a new partition on it:
# fdisk /dev/md124
...
Calling ioctl() to re-read partition table.
Syncing disks.

Now inspecting /proc/mdstat:

# cat /proc/mdstat
Personalities : [raid1]
md124 : active raid1 loop1[2] loop0[0]
      84416 blocks super external:/md125/0 [2/2] [UU]

md125 : inactive loop1[1](S) loop0[0](S)
      65536 blocks super external:ddf

which looks even weirder: "loop1[2]" indicates that the disk is a
spare one whereas "[UU]" tells me the opposite.

Could you tell me if I'm wrong in my interpretation or what's going wrong ?

Thanks
-- 
Francis

Re: Weirdness with DDF arrays (mdadm 3.3)

[Martin Wilck]

Hi Francis, hi Neil,

> Still testing MD arrays using DDF metadata and find another possible issues :)
> 
> I'm creating a new DDF array containing 2 disks. After that
> /proc/mdstat looks correct:
> 
> # cat /proc/mdstat
> Personalities : [raid1]
> md124 : active raid1 loop0[1] loop1[0]
>       84416 blocks super external:/md125/0 [2/2] [UU]
> 
> md125 : inactive loop1[1](S) loop0[0](S)
>       65536 blocks super external:ddf
> 
> Now I'm stopping the array and restart it by incrementaly adding the 2 disks:
> # mdadm --stop /dev/md124
> # mdadm --stop /dev/md125
> # mdadm -IRs /dev/loop0

This is wrong, because -IRs "wills can the mapfile for arrays that are
being incrementally assembled snd will try to start any that are not
already started".

mdadm -IRs will first add /dev/loop0, then see that there is an
incomplete array, and start it.

> # mdadm -IRs /dev/loop1

Now you add /dev/loop1, but as the array is already started, it will be
added as a spare. That's what you see below.

However, there is room for improvement here. The array hasn't been
written to, so even if it is started, it should be possible to re-add
the second disk cleanly.

Looking into that.

Martin


> # cat /proc/mdstat
> Personalities : [raid1]
> md124 : active (auto-read-only) raid1 loop1[2] loop0[0]
>       84416 blocks super external:/md125/0 [2/1] [_U]
> 
> md125 : inactive loop1[1](S) loop0[0](S)
>       65536 blocks super external:ddf
> 
> Parsing mdstat content tells me disk "loop1" have a role number equal
> to 2 which is greater than 1 indicating that "loop1" is a spare disk
> and the "[_U]" below indicates "loop1" is down".
> 
> Why is "loop1" down now ?
> 
> I decided to still use the md device by creating a new partition on it:
> # fdisk /dev/md124
> ...
> Calling ioctl() to re-read partition table.
> Syncing disks.
> 
> Now inspecting /proc/mdstat:
> 
> # cat /proc/mdstat
> Personalities : [raid1]
> md124 : active raid1 loop1[2] loop0[0]
>       84416 blocks super external:/md125/0 [2/2] [UU]
> 
> md125 : inactive loop1[1](S) loop0[0](S)
>       65536 blocks super external:ddf
> 
> which looks even weirder: "loop1[2]" indicates that the disk is a
> spare one whereas "[UU]" tells me the opposite.
> 
> Could you tell me if I'm wrong in my interpretation or what's going wrong ?
> 
> Thanks

Re: Weirdness with DDF arrays (mdadm 3.3)

[Francis Moreau]

Hi Martin,

On Sat, Sep 14, 2013 at 4:16 PM, Martin Wilck <mwilck@arcor.de> wrote:
> Hi Francis, hi Neil,
>
>> Still testing MD arrays using DDF metadata and find another possible issues :)
>>
>> I'm creating a new DDF array containing 2 disks. After that
>> /proc/mdstat looks correct:
>>
>> # cat /proc/mdstat
>> Personalities : [raid1]
>> md124 : active raid1 loop0[1] loop1[0]
>>       84416 blocks super external:/md125/0 [2/2] [UU]
>>
>> md125 : inactive loop1[1](S) loop0[0](S)
>>       65536 blocks super external:ddf
>>
>> Now I'm stopping the array and restart it by incrementaly adding the 2 disks:
>> # mdadm --stop /dev/md124
>> # mdadm --stop /dev/md125
>> # mdadm -IRs /dev/loop0
>
> This is wrong, because -IRs "wills can the mapfile for arrays that are
> being incrementally assembled snd will try to start any that are not
> already started".
>
> mdadm -IRs will first add /dev/loop0, then see that there is an
> incomplete array, and start it.
>
>> # mdadm -IRs /dev/loop1
>
> Now you add /dev/loop1, but as the array is already started, it will be
> added as a spare. That's what you see below.

Ah you're right, sorry for the noise, however I still doesn't
understand my last point, please see the end of the email.

>
> However, there is room for improvement here. The array hasn't been
> written to, so even if it is started, it should be possible to re-add
> the second disk cleanly.
>
> Looking into that.
>

thanks

> Martin
>
>
>> # cat /proc/mdstat
>> Personalities : [raid1]
>> md124 : active (auto-read-only) raid1 loop1[2] loop0[0]
>>       84416 blocks super external:/md125/0 [2/1] [_U]
>>
>> md125 : inactive loop1[1](S) loop0[0](S)
>>       65536 blocks super external:ddf
>>
>> Parsing mdstat content tells me disk "loop1" have a role number equal
>> to 2 which is greater than 1 indicating that "loop1" is a spare disk
>> and the "[_U]" below indicates "loop1" is down".
>>
>> Why is "loop1" down now ?
>>
>> I decided to still use the md device by creating a new partition on it:
>> # fdisk /dev/md124
>> ...
>> Calling ioctl() to re-read partition table.
>> Syncing disks.
>>
>> Now inspecting /proc/mdstat:
>>
>> # cat /proc/mdstat
>> Personalities : [raid1]
>> md124 : active raid1 loop1[2] loop0[0]
>>       84416 blocks super external:/md125/0 [2/2] [UU]
>>
>> md125 : inactive loop1[1](S) loop0[0](S)
>>       65536 blocks super external:ddf
>>
>> which looks even weirder: "loop1[2]" indicates that the disk is a
>> spare one whereas "[UU]" tells me the opposite.
>>
>> Could you tell me if I'm wrong in my interpretation or what's going wrong ?

What about the loop1 in spare and [UU] indicating that loop1 is used ?


Thanks
-- 
Francis

Re: Weirdness with DDF arrays (mdadm 3.3)

[Martin Wilck]

On 09/14/2013 05:25 PM, Francis Moreau wrote:
> Hi Martin,
> 
> On Sat, Sep 14, 2013 at 4:16 PM, Martin Wilck <mwilck@arcor.de> wrote:
>> Hi Francis, hi Neil,
>>
>>> Still testing MD arrays using DDF metadata and find another possible issues :)
>>>
>>> I'm creating a new DDF array containing 2 disks. After that
>>> /proc/mdstat looks correct:
>>>
>>> # cat /proc/mdstat
>>> Personalities : [raid1]
>>> md124 : active raid1 loop0[1] loop1[0]
>>>       84416 blocks super external:/md125/0 [2/2] [UU]
>>>
>>> md125 : inactive loop1[1](S) loop0[0](S)
>>>       65536 blocks super external:ddf
>>>
>>> Now I'm stopping the array and restart it by incrementaly adding the 2 disks:
>>> # mdadm --stop /dev/md124
>>> # mdadm --stop /dev/md125
>>> # mdadm -IRs /dev/loop0
>>
>> This is wrong, because -IRs "wills can the mapfile for arrays that are
>> being incrementally assembled snd will try to start any that are not
>> already started".
>>
>> mdadm -IRs will first add /dev/loop0, then see that there is an
>> incomplete array, and start it.
>>
>>> # mdadm -IRs /dev/loop1
>>
>> Now you add /dev/loop1, but as the array is already started, it will be
>> added as a spare. That's what you see below.
> 
> Ah you're right, sorry for the noise, however I still doesn't
> understand my last point, please see the end of the email.
> 
>>
>> However, there is room for improvement here. The array hasn't been
>> written to, so even if it is started, it should be possible to re-add
>> the second disk cleanly.
>>
>> Looking into that.
>>
> 
> thanks
> 
>> Martin
>>
>>
>>> # cat /proc/mdstat
>>> Personalities : [raid1]
>>> md124 : active (auto-read-only) raid1 loop1[2] loop0[0]
>>>       84416 blocks super external:/md125/0 [2/1] [_U]
>>>
>>> md125 : inactive loop1[1](S) loop0[0](S)
>>>       65536 blocks super external:ddf
>>>
>>> Parsing mdstat content tells me disk "loop1" have a role number equal
>>> to 2 which is greater than 1 indicating that "loop1" is a spare disk
>>> and the "[_U]" below indicates "loop1" is down".
>>>
>>> Why is "loop1" down now ?
>>>
>>> I decided to still use the md device by creating a new partition on it:
>>> # fdisk /dev/md124
>>> ...
>>> Calling ioctl() to re-read partition table.
>>> Syncing disks.
>>>
>>> Now inspecting /proc/mdstat:
>>>
>>> # cat /proc/mdstat
>>> Personalities : [raid1]
>>> md124 : active raid1 loop1[2] loop0[0]
>>>       84416 blocks super external:/md125/0 [2/2] [UU]
>>>
>>> md125 : inactive loop1[1](S) loop0[0](S)
>>>       65536 blocks super external:ddf
>>>
>>> which looks even weirder: "loop1[2]" indicates that the disk is a
>>> spare one whereas "[UU]" tells me the opposite.
>>>
>>> Could you tell me if I'm wrong in my interpretation or what's going wrong ?
> 
> What about the loop1 in spare and [UU] indicating that loop1 is used ?

After you added loop1, the array was in read-auto state. Rebuild doesn't
start in this state.

When you created the partition table, the array went in active state and
was rebuilt. When you looked at mdstat again, the rebuild was already
finished. Therefore you got "[UU]" after that.

Wrt loop1[2], I think you interpret the [2] wrongly. It seems to be the
kernel index of the device somehow. The mdstat parsing code of mdadm
doesn't look at this number. If you look at
/sys/class/block/md124/md/dev-loop*/slot, the number should be correct -
I tried it here.

Perhaps someone else can comment on this, too.

/proc/mdstat can be pretty cryptic. If you are uncertain about anything,
look under /sys.

Martin

> 
> 
> Thanks

"detect reshape on array start" (was Re: Weirdness with DDF arrays (mdadm 3.3))

[Martin Wilck]

Hi Neil,

I wrote:

>> > Still testing MD arrays using DDF metadata and find another possible issues :)
>> > 
>> > I'm creating a new DDF array containing 2 disks. After that
>> > /proc/mdstat looks correct:
>> > 
>> > # cat /proc/mdstat
>> > Personalities : [raid1]
>> > md124 : active raid1 loop0[1] loop1[0]
>> >       84416 blocks super external:/md125/0 [2/2] [UU]
>> > 
>> > md125 : inactive loop1[1](S) loop0[0](S)
>> >       65536 blocks super external:ddf
>> > 
>> > Now I'm stopping the array and restart it by incrementaly adding the 2 disks:
>> > # mdadm --stop /dev/md124
>> > # mdadm --stop /dev/md125
>> > # mdadm -IRs /dev/loop0
> This is wrong, because -IRs "wills can the mapfile for arrays that are
> being incrementally assembled snd will try to start any that are not
> already started".
> 
> mdadm -IRs will first add /dev/loop0, then see that there is an
> incomplete array, and start it.
> 
>> > # mdadm -IRs /dev/loop1
> Now you add /dev/loop1, but as the array is already started, it will be
> added as a spare. That's what you see below.
> 
> However, there is room for improvement here. The array hasn't been
> written to, so even if it is started, it should be possible to re-add
> the second disk cleanly.

one major problem here seems to be commit 4867e068. It sets the array
dirty after transition from inactive to e.g. read-auto. That seems to be
wrong to me. From the patch's description

  Raid0: detect reshape on array start

  When raid0 array is takeovered to raid4 for reshape it should be
  possible to detect that array for reshape is monitored now for
  metadata update."

it seems that has been made for is a pretty special case (reshape of
RAID0 to RAID4, or would that affect other target geometries as well?)

For experimenting, I reverted the patch and ran some of the IMSM test
cases for reshape of RAID0 to something else, and they all still succeed.

So, I believe this patch can be reverted; and it should be because the
side effect that once a disk is missing when an array is started, it
can't be cleanly added any more, even if the array was never written to.

I cc'd Adam, the author of that patch, on this mail.

Martin

Re: "detect reshape on array start" (was Re: Weirdness with DDF arrays (mdadm 3.3))

[Martin Wilck]

On 09/14/2013 11:01 PM, Martin Wilck wrote:

> it seems that has been made for is a pretty special case (reshape of
> RAID0 to RAID4, or would that affect other target geometries as well?)
> 
> For experimenting, I reverted the patch and ran some of the IMSM test
> cases for reshape of RAID0 to something else, and they all still succeed.

Meanwhile I ran all test cases for IMSM (09, 11-18) with this patch
reverted; they all succeed.

Martin

[PATCH] Monitor: don't set arrays dirty after transition to read-only

[mwilck]

This patch reverts commit 4867e068. Setting arrays dirty after
transition from inactive to anything else causes unnecessary
meta data writes and may wreak trouble unnecessarily when
a disk was missing during assembly but the array was never
written to.

The reason for 4867e068 was a special situation during reshape
from RAID0 to RAID4. I ran all IMSM test cases with it reverted
and found no regressions, so I believe the reshape logic for
IMSM works fine in mdadm 3.3 also without this.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 monitor.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/monitor.c b/monitor.c
index 742aa19..3b57ed0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -270,13 +270,6 @@ static int read_and_act(struct active_array *a)
 		a->info.resync_start
 		);
 
-	if (a->curr_state > inactive &&
-	    a->prev_state == inactive) {
-		/* array has been started
-		 * possible that container operation has to be completed
-		 */
-		a->container->ss->set_array_state(a, 0);
-	}
 	if ((a->curr_state == bad_word || a->curr_state <= inactive) &&
 	    a->prev_state > inactive) {
 		/* array has been stopped */
-- 
1.7.3.4

Re: Weirdness with DDF arrays (mdadm 3.3)

[Martin Wilck]

On 09/15/2013 02:52 PM, Francis Moreau wrote:
> Hello Martin,
> 
> On Sat, Sep 14, 2013 at 10:11 PM, Martin Wilck <mwilck@arcor.de> wrote:
>> On 09/14/2013 05:25 PM, Francis Moreau wrote:
> 
> [...]
> 
>>>>>
>>>>> which looks even weirder: "loop1[2]" indicates that the disk is a
>>>>> spare one whereas "[UU]" tells me the opposite.
>>>>>
>>>>> Could you tell me if I'm wrong in my interpretation or what's going wrong ?
>>>
>>> What about the loop1 in spare and [UU] indicating that loop1 is used ?
>>
>> After you added loop1, the array was in read-auto state. Rebuild doesn't
>> start in this state.
>>
>> When you created the partition table, the array went in active state and
>> was rebuilt. When you looked at mdstat again, the rebuild was already
>> finished. Therefore you got "[UU]" after that.
>>
>> Wrt loop1[2], I think you interpret the [2] wrongly. It seems to be the
>> kernel index of the device somehow. The mdstat parsing code of mdadm
>> doesn't look at this number. If you look at
>> /sys/class/block/md124/md/dev-loop*/slot, the number should be correct -
>> I tried it here.
> 
> Well I think I interpreted the numbers the way it's described here :
> 
> https://raid.wiki.kernel.org/index.php/Mdstat#md_device_line

That description is not quite correct. The number in brackets [2] means
the index of the disk in the meta data (for DDF, that's the index in the
"physical disks" table of the container). That number isn't very
meaningful except for the meta data itself.

The logical disk index is represented by the "slot" attribute in sysfs.

See e.g.
http://lxr.missinglinkelectronics.com/linux+*/drivers/md/md.h#L79

The number displayed in /proc/mdstat is "desc_nr", while the number that
actually matters is "raid_disk".

Regards
Martin

> Thanks

Re: Weirdness with DDF arrays (mdadm 3.3)

[Francis Moreau]

Hi Martin,

On Sun, Sep 15, 2013 at 9:46 PM, Martin Wilck <mwilck@arcor.de> wrote:
> On 09/15/2013 02:52 PM, Francis Moreau wrote:
>> Hello Martin,
>>
>> On Sat, Sep 14, 2013 at 10:11 PM, Martin Wilck <mwilck@arcor.de> wrote:
>>> On 09/14/2013 05:25 PM, Francis Moreau wrote:
>>
>> [...]
>>
>>>>>>
>>>>>> which looks even weirder: "loop1[2]" indicates that the disk is a
>>>>>> spare one whereas "[UU]" tells me the opposite.
>>>>>>
>>>>>> Could you tell me if I'm wrong in my interpretation or what's going wrong ?
>>>>
>>>> What about the loop1 in spare and [UU] indicating that loop1 is used ?
>>>
>>> After you added loop1, the array was in read-auto state. Rebuild doesn't
>>> start in this state.
>>>
>>> When you created the partition table, the array went in active state and
>>> was rebuilt. When you looked at mdstat again, the rebuild was already
>>> finished. Therefore you got "[UU]" after that.
>>>
>>> Wrt loop1[2], I think you interpret the [2] wrongly. It seems to be the
>>> kernel index of the device somehow. The mdstat parsing code of mdadm
>>> doesn't look at this number. If you look at
>>> /sys/class/block/md124/md/dev-loop*/slot, the number should be correct -
>>> I tried it here.
>>
>> Well I think I interpreted the numbers the way it's described here :
>>
>> https://raid.wiki.kernel.org/index.php/Mdstat#md_device_line
>
> That description is not quite correct. The number in brackets [2] means
> the index of the disk in the meta data (for DDF, that's the index in the
> "physical disks" table of the container). That number isn't very
> meaningful except for the meta data itself.
>
> The logical disk index is represented by the "slot" attribute in sysfs.
>
> See e.g.
> http://lxr.missinglinkelectronics.com/linux+*/drivers/md/md.h#L79
>
> The number displayed in /proc/mdstat is "desc_nr", while the number that
> actually matters is "raid_disk".

Maybe the description in the wiki is correct but there's a bun in the
kernel which displays the wrong number ?

If "desc_nr" isn't meaningful, I don't see the point to show it in /proc/mdstat.

Thanks
-- 
Francis

Re: Weirdness with DDF arrays (mdadm 3.3)

[Martin Wilck]

On 09/16/2013 03:47 PM, Francis Moreau wrote:
[...]

>>>> Wrt loop1[2], I think you interpret the [2] wrongly. It seems to be the
>>>> kernel index of the device somehow. The mdstat parsing code of mdadm
>>>> doesn't look at this number. If you look at
>>>> /sys/class/block/md124/md/dev-loop*/slot, the number should be correct -
>>>> I tried it here.
>>>
>>> Well I think I interpreted the numbers the way it's described here :
>>>
>>> https://raid.wiki.kernel.org/index.php/Mdstat#md_device_line
>>
>> That description is not quite correct. The number in brackets [2] means
>> the index of the disk in the meta data (for DDF, that's the index in the
>> "physical disks" table of the container). That number isn't very
>> meaningful except for the meta data itself.
>>
>> The logical disk index is represented by the "slot" attribute in sysfs.
>>
>> See e.g.
>> http://lxr.missinglinkelectronics.com/linux+*/drivers/md/md.h#L79
>>
>> The number displayed in /proc/mdstat is "desc_nr", while the number that
>> actually matters is "raid_disk".
> 
> Maybe the description in the wiki is correct but there's a bun in the
> kernel which displays the wrong number ?
>
> If "desc_nr" isn't meaningful, I don't see the point to show it in
> /proc/mdstat.

Well, we could propose to change the value displayed by the kernel. The
question is whether the kernel maintainers would allow that, because it
would change the kernel-userspace API. mdadm itself doesn't look at this
number, but other tools might, so there is some small but non-zero risk
of breaking something somewhere.

Let's wait what others have to say.

Martin

Re: Weirdness with DDF arrays (mdadm 3.3)

[Francis Moreau]

On Mon, Sep 16, 2013 at 7:02 PM, Martin Wilck <mwilck@arcor.de> wrote:
> On 09/16/2013 03:47 PM, Francis Moreau wrote:
> [...]
>
>>>>> Wrt loop1[2], I think you interpret the [2] wrongly. It seems to be the
>>>>> kernel index of the device somehow. The mdstat parsing code of mdadm
>>>>> doesn't look at this number. If you look at
>>>>> /sys/class/block/md124/md/dev-loop*/slot, the number should be correct -
>>>>> I tried it here.
>>>>
>>>> Well I think I interpreted the numbers the way it's described here :
>>>>
>>>> https://raid.wiki.kernel.org/index.php/Mdstat#md_device_line
>>>
>>> That description is not quite correct. The number in brackets [2] means
>>> the index of the disk in the meta data (for DDF, that's the index in the
>>> "physical disks" table of the container). That number isn't very
>>> meaningful except for the meta data itself.
>>>
>>> The logical disk index is represented by the "slot" attribute in sysfs.
>>>
>>> See e.g.
>>> http://lxr.missinglinkelectronics.com/linux+*/drivers/md/md.h#L79
>>>
>>> The number displayed in /proc/mdstat is "desc_nr", while the number that
>>> actually matters is "raid_disk".
>>
>> Maybe the description in the wiki is correct but there's a bun in the
>> kernel which displays the wrong number ?
>>
>> If "desc_nr" isn't meaningful, I don't see the point to show it in
>> /proc/mdstat.
>
> Well, we could propose to change the value displayed by the kernel. The
> question is whether the kernel maintainers would allow that, because it
> would change the kernel-userspace API. mdadm itself doesn't look at this
> number, but other tools might, so there is some small but non-zero risk
> of breaking something somewhere.

Well I was assuming that the API was described by:
https://raid.wiki.kernel.org/index.php/Mdstat#md_device_line rather
than looking at the source code. I haven't found other documentation
for a description of /proc/mdstat BTW.

Plus the fact that "desc_nr" isn't really meaningful.

>
> Let's wait what others have to say.
>

Agreed.

Thanks.
-- 
Francis