On Mon, May 17, 2010 at 5:23 PM, Neil Brown <neilb@xxxxxxx> wrote:
> On Mon, 17 May 2010 14:11:42 -0400
> Doug Ledford <dledford@xxxxxxxxxx> wrote:
>
>> On 05/11/2010 04:53 PM, Neil Brown wrote:
>> > Theoretically, when the spares are one behind the active array and we need to
>> > update them all, we should update the spares first, then the rest. If we
>> > don't and there is a crash at the wrong time, some spares could be 2 events
>> > behind the most recent device. However that is a fairly unlikely race to
>> > lose and the cost is only having a spare device fall out of the array, which
>> > is quite easy to put back it, that I might not worry to much about it.
>> >
>> > So if you haven't seen a patch to fix this in a week or two, please remind me.
>>
>> They're spares. They don't need to have an in-sync generation count.
>> Change the semantics so that spares have a 0 generation count and only
>> when they've been activated does their count get brought into sync with
>> the rest of the array. The only thing needed to make that work is to
>> not kick them on assembly because their generation count doesn't match,
>> but that should be trivial (and sane) to do based on the fact that if
>> they are a spare, they don't contain data, so the generation count
>> really is meaningless.
>>
>
> Yes.
> However current kernels will kick them on assembly and so I cannot make
> future kernels leave the event count at zero, else people who try a new
> kernel then revert to an old might find there spares drop off ... and could
> well not be happy about that.
>
> But I can certainly relax the requirement that the event count on a spare be
> current now. Maybe in a few years I can then stop updating the event count
> on spares completely....
>
> NeilBrown
>
> From 589af914c85ebb8205ba315a1e78e76ec7bd9e99 Mon Sep 17 00:00:00 2001
> From: NeilBrown <neilb@xxxxxxx>
> Date: Tue, 18 May 2010 10:17:09 +1000
> Subject: [PATCH] md: don't insist on valid event count for spare devices.
>
> Devices which know that they are spares do not really need to have
> an event count that matches the rest of the array, so there are no
> data-in-sync issues. It is enough that the uuid matches.
> So remove the requirement that the event count is up-to-date.
>
> We currently still write out and event count on spares, but this
> allows us in a year or 3 to stop doing that completely.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 9ef21d9..26b3d28 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -1070,10 +1070,13 @@ static int super_90_validate(mddev_t *mddev, mdk_rdev_t *rdev)
> mddev->bitmap_info.default_offset;
>
> } else if (mddev->pers == NULL) {
> - /* Insist on good event counter while assembling */
> + /* Insist on good event counter while assembling, except
> + * for spares (which don't need an event count) */
> ++ev1;
> - if (ev1 < mddev->events)
> - return -EINVAL;
> + if (sb->disks[rdev->desc_nr].state & (
> + (1<<MD_DISK_SYNC) | (1 << MD_DISK_ACTIVE)))
> + if (ev1 < mddev->events)
> + return -EINVAL;
> } else if (mddev->bitmap) {
> /* if adding to array with a bitmap, then we can accept an
> * older device ... but not too old.
> @@ -1469,10 +1472,14 @@ static int super_1_validate(mddev_t *mddev, mdk_rdev_t *rdev)
> }
>
> } else if (mddev->pers == NULL) {
> - /* Insist of good event counter while assembling */
> + /* Insist of good event counter while assembling, except for
> + * spares (which don't need an event count) */
> ++ev1;
> - if (ev1 < mddev->events)
> - return -EINVAL;
> + if (rdev->desc_nr >= 0 &&
> + rdev->desc_nr < le32_to_cpu(sb->max_dev) &&
> + le16_to_cpu(sb->dev_roles[rdev->desc_nr]) < 0xfffe)
> + if (ev1 < mddev->events)
> + return -EINVAL;
> } else if (mddev->bitmap) {
> /* If adding to array with a bitmap, then we can accept an
> * older device, but not too old.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
You should make it a sysfs controllable option to keep the counter
updated on spare discs or not. The default can be to keep it within
one as you currently are patching it to. Then later you can change
the default to the higher performance setting. Better, the presence
of that file can be used to test if the kernel supports it, and if it
doesn't initramfs scripts can react accordingly.
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
