dm-raid: check events in super_validate

If an LVM raid1 recovery is interrupted by deactivating the LV, when the LV is reactivated it comes up with both members in sync--the recovery never completes.

I've been trying to figure out how to fix this. Does this approach look okay? I'm not sure what else to use to determine that a member disk is out of sync. It looks like if disk_recovery_offset in the superblock were updated during the recovery, that would also cause it to resume after interruption--but MD skips the recovery target disk when writing superblocks, so this doesn't work.

Comments?

Thanks,

Nate Dailey
Stratus Technologies

diff -Nupr linux-3.12.9.orig/drivers/md/dm-raid.c
linux-3.12.9/drivers/md/dm-raid.c
--- linux-3.12.9.orig/drivers/md/dm-raid.c 2014-02-01
08:46:51.088086299 -0500
+++ linux-3.12.9/drivers/md/dm-raid.c 2014-02-01 09:02:06.657149550 -0500

drivers/md/dm-raid.c 2014-02-01 09:02:06.657149550 -0500 renamed from drivers/md/dm-raid.c 2014-02-01 08:46:51.088086299 -0500 +15 -1

@@ -1042,6 +1042,21 @@ static int super_validate(struct mddev *
          rdev->recovery_offset = le64_to_cpu(sb->disk_recovery_offset);
          if (rdev->recovery_offset != MaxSector)
              clear_bit(In_sync, &rdev->flags);
+        else if (!test_bit(Faulty, &rdev->flags)) {
+            uint64_t events_sb;
+
+            /*
+             * Trigger recovery if events is out-of-date.
+             */
+            events_sb = le64_to_cpu(sb->events);
+            if (events_sb < mddev->events) {
+                DMINFO("Force recovery on out-of-date device #%d.",
+                       rdev->raid_disk);
+                clear_bit(In_sync, &rdev->flags);
+                rdev->saved_raid_disk = rdev->raid_disk;
+                rdev->recovery_offset = 0;
+            }
+        }
      }
 
      /*
 
--

To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 4 lines · On Sat, 1 Feb 2014 09:35:20 -0500 Nate Dailey <[email protected]> wrote:

If an LVM raid1 recovery is interrupted by deactivating the LV, when the LV is reactivated it comes up with both members in sync--the recovery never completes.

Quoted 8 lines

I've been trying to figure out how to fix this. Does this approach look okay? I'm not sure what else to use to determine that a member disk is out of sync. It looks like if disk_recovery_offset in the superblock were updated during the recovery, that would also cause it to resume after interruption--but MD skips the recovery target disk when writing superblocks, so this doesn't work.

Comments?

I know it is confusing, but this should really have gone to dm-devel rather than linux-raid, to make sure Jon Brassow see it (hi Jon!).

Setting recovery_offset to 0 certainly looks wrong, it should be set to
sb->disk_recovery_offset
like the code just above your change.
Why does the code there not meet your need.

Jon: can you help?

Quoted 17 lines · NeilBrown

Quoted 4 lines · On Sat, 1 Feb 2014 09:35:20 -0500 Nate Dailey <[email protected]> wrote:

If an LVM raid1 recovery is interrupted by deactivating the LV, when the LV is reactivated it comes up with both members in sync--the recovery never completes.

Quoted 8 lines

I've been trying to figure out how to fix this. Does this approach look okay? I'm not sure what else to use to determine that a member disk is out of sync. It looks like if disk_recovery_offset in the superblock were updated during the recovery, that would also cause it to resume after interruption--but MD skips the recovery target disk when writing superblocks, so this doesn't work.

Comments?

I know it is confusing, but this should really have gone to dm-devel rather than linux-raid, to make sure Jon Brassow see it (hi Jon!).

Quoted 6 lines

Setting recovery_offset to 0 certainly looks wrong, it should be set to
sb->disk_recovery_offset
like the code just above your change.
Why does the code there not meet your need.

Jon: can you help?

Sure, thanks for forwarding.

Could you describe first how you are creating the problem?

When I create a RAID1 LV, deactivate it, and reactivate it; I don't see it skip the sync. Also, if I replace a single drive and cycle the LV, I don't see it skip the sync. What steps am I missing?

brassow

Here's what I've done to reproduce this:

- remove a disk containing one leg of an LVM raid1 mirror
- do enough IO that a lengthy recovery will be required
- insert the removed disk
- let recovery begin, but deactivate the LV before it completes
- activate the LV

This is the point where the recovery should start back up, but it doesn't. I haven't tried this in a few weeks, but am happy to try it again if it would help.

Quoted 8 lines · Nate Dailey

Here's what I've done to reproduce this:

- remove a disk containing one leg of an LVM raid1 mirror
- do enough IO that a lengthy recovery will be required
- insert the removed disk
- let recovery begin, but deactivate the LV before it completes
- activate the LV

Quoted 1 line

This is the point where the recovery should start back up, but it doesn't. I haven't tried this in a few weeks, but am happy to try it again if it would help.

Confirmed (test output below). I'll get started on this. This code can be a bit tricky and I've been away from it for a while. It will take me a bit to re-familiarize myself with it and review your patch.

Quoted 4 lines · Brassow Jonathan

Quoted 2 lines · Nate Dailey

Here's what I've done to reproduce this:

Quoted 8 lines

Quoted 7 lines

- remove a disk containing one leg of an LVM raid1 mirror
- do enough IO that a lengthy recovery will be required
- insert the removed disk
- let recovery begin, but deactivate the LV before it completes
- activate the LV

This is the point where the recovery should start back up, but it doesn't. I haven't tried this in a few weeks, but am happy to try it again if it would help.

Confirmed (test output below). I'll get started on this. This code can be a bit tricky and I've been away from it for a while. It will take me a bit to re-familiarize myself with it and review your patch.

I've tested this again with the latest code from upstream (kernel 3.14.0-rc4) and I cannot reproduce the problem there. I'll see if I can find the last non-working version...

brassow

Interesting. I had tried this with the latest stable kernel.org kernel (as of a month or so ago) and still hit it.

I'll mention that the initial resync on creating the raid1 can be interrupted okay; the problem only happens after that completes, and a disk is removed and re-added.

Nate

Quoted 18 lines · Brassow Jonathan

Quoted 4 lines · Brassow Jonathan

Quoted 2 lines · Nate Dailey

Here's what I've done to reproduce this:

Quoted 8 lines

Quoted 7 lines

- remove a disk containing one leg of an LVM raid1 mirror
- do enough IO that a lengthy recovery will be required
- insert the removed disk
- let recovery begin, but deactivate the LV before it completes
- activate the LV

This is the point where the recovery should start back up, but it doesn't. I haven't tried this in a few weeks, but am happy to try it again if it would help.

Confirmed (test output below). I'll get started on this. This code can be a bit tricky and I've been away from it for a while. It will take me a bit to re-familiarize myself with it and review your patch.

I've tested this again with the latest code from upstream (kernel 3.14.0-rc4) and I cannot reproduce the problem there. I'll see if I can find the last non-working version...

brassow

The commit that fixed the problem was:
f466722ca614edcd14f3337373f33132117c7612
f466722 md: Change handling of save_raid_disk and metadata update during recover

I don't know if this is safe to independently apply to previous kernels or not yet. Neil, do you know?

I have applied it directly to the 3.13 kernel and it solves the problem. However, applying it to the 3.12 kernel does not solve the problem. There is obviously something else I'm missing or the fuzziness of the patch on 3.12 is causing things to not behave as expected.