On Fri, 19 Apr 2013 15:09:21 -0500 Jonathan Brassow <jbrassow@xxxxxxxxxx>
wrote:
> MD: Do not increment resync_mismatches unless MD_RECOVERY_REQUESTED
>
> resync_mismatches is used to display the number of differences that
> have been found or repaired during a scrubbing operation. It is not
> meant to count anything during resync or repair operations. (How
> much sense does it make to find resync_mismatches populated after an
> initial synchronization of the array? After cleaning-up an unclean
> shutdown? After [re]integrating a device into an existing array?)
> The incrementing of the variable must be restricted to when the user
> initiates a scrubbing operation (i.e. "check" or "repair").
How do you know what it is "meant" to do? :-)
While it might not be particularly useful, I see no point in hiding
information, and no desire to change what mismatch_cnt reports.
It may well be useful to somehow report what the real meaning for
mismatch_cnt is. i.e. to report what the last sync_action was.
Then any use-space tool that reported mismatch_cnt, could adjust according to
whether the last operation was check/repair or resync/recovery/reshape. etc.
So if you were to provide a patch which adds "last_sync_action" or similar,
I'd certainly consider that.
Thanks,
NeilBrown
>
> Signed-off-by: Jonathan Brassow <jbrassow@xxxxxxxxxx>
>
> Index: linux-upstream/drivers/md/raid1.c
> ===================================================================
> --- linux-upstream.orig/drivers/md/raid1.c
> +++ linux-upstream/drivers/md/raid1.c
> @@ -1878,7 +1878,8 @@ static int process_checks(struct r1bio *
> }
> } else
> j = 0;
> - if (j >= 0)
> + if ((j >= 0) &&
> + (test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery)))
> atomic64_add(r1_bio->sectors, &mddev->resync_mismatches);
> if (j < 0 || (test_bit(MD_RECOVERY_CHECK, &mddev->recovery)
> && test_bit(BIO_UPTODATE, &sbio->bi_flags))) {
> Index: linux-upstream/drivers/md/raid10.c
> ===================================================================
> --- linux-upstream.orig/drivers/md/raid10.c
> +++ linux-upstream/drivers/md/raid10.c
> @@ -2071,7 +2071,10 @@ static void sync_request_write(struct md
> break;
> if (j == vcnt)
> continue;
> - atomic64_add(r10_bio->sectors, &mddev->resync_mismatches);
> +
> + if (test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery))
> + atomic64_add(r10_bio->sectors,
> + &mddev->resync_mismatches);
> if (test_bit(MD_RECOVERY_CHECK, &mddev->recovery))
> /* Don't fix anything. */
> continue;
> Index: linux-upstream/drivers/md/raid5.c
> ===================================================================
> --- linux-upstream.orig/drivers/md/raid5.c
> +++ linux-upstream/drivers/md/raid5.c
> @@ -2989,7 +2989,10 @@ static void handle_parity_checks5(struct
> */
> set_bit(STRIPE_INSYNC, &sh->state);
> else {
> - atomic64_add(STRIPE_SECTORS, &conf->mddev->resync_mismatches);
> + if (test_bit(MD_RECOVERY_REQUESTED,
> + &conf->mddev->recovery))
> + atomic64_add(STRIPE_SECTORS,
> + &conf->mddev->resync_mismatches);
> if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery))
> /* don't try to repair!! */
> set_bit(STRIPE_INSYNC, &sh->state);
> @@ -3141,7 +3144,10 @@ static void handle_parity_checks6(struct
> */
> }
> } else {
> - atomic64_add(STRIPE_SECTORS, &conf->mddev->resync_mismatches);
> + if (test_bit(MD_RECOVERY_REQUESTED,
> + &conf->mddev->recovery))
> + atomic64_add(STRIPE_SECTORS,
> + &conf->mddev->resync_mismatches);
> if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery))
> /* don't try to repair!! */
> set_bit(STRIPE_INSYNC, &sh->state);
>
Attachment:
signature.asc
Description: PGP signature
