On Tue, 28 May 2013 23:12:38 +0300 Alexander Lyakas <alex.bolshoy@xxxxxxxxx>
wrote:
> Hi Neil,
> I have found why recovery is triggered again and again.
>
> After initial recovery aborts, raid1d() calls md_check_recovery(),
> which calls remove_and_add_spares().
> So remove_and_add_spares wants to eject the rebuilding drive from the
> array, but cannot because rdev->nr_pending>0:
> if (rdev->raid_disk >= 0 &&
> !test_bit(Blocked, &rdev->flags) &&
> (test_bit(Faulty, &rdev->flags) ||
> ! test_bit(In_sync, &rdev->flags)) &&
> atomic_read(&rdev->nr_pending)==0) {
> if (mddev->pers->hot_remove_disk(
> ...
>
> So instead it goes ahead and triggers another recovery in the
> following condition (by incrementing "spares"):
> rdev_for_each(rdev, mddev) {
> if (rdev->raid_disk >= 0 &&
> !test_bit(In_sync, &rdev->flags) &&
> !test_bit(Faulty, &rdev->flags))
> spares++;
>
> So another recovery starts, aborts, is triggered again, aborts and so forth.
>
> Because conditions:
> test_bit(Faulty, &rdev->flags) || ! test_bit(In_sync, &rdev->flags)
> and
> !test_bit(In_sync, &rdev->flags) && !test_bit(Faulty, &rdev->flags))
> can both be true at the same time.
>
> Problem is that IOs keep coming in, because MD does not fail them as I
> discussed in the previous email. So nr_pending never gets to zero, or
> it takes it a long time to get to zero (totally racy), and we keep
> triggering and aborting recovery.
>
> Can we fix remove_and_add_spares() the following way:
>
> "if a drive qualifies for being removed from array, but cannot be
> removed because of nr_pending, don't try to trigger another recovery
> on it by returning doing spares++".
I'm not sure... While that approach would address this particular problem
I'm not convinced that it would always be correct.
The 'recovery_disabled' field is supposed to stop this sort of thing, but it
doesn't get activated here because it only stops the adding of devices to the
array, and we never add a device.
I would probably remove the nr_pending test from remove_add_and_spares and
leave it to the personality routine to do that test (I think they already do).
Then get ->hot_remove_disk to have a particular error status which means
"don't try any recovery", which is returned if the ->recovery_disabled fields
match.
Then remove_and_add_spares need to check for this return value and act
accordingly.
Something like that.
Do you think that would work?
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
