On Wednesday August 15, maccetta@xxxxxxxxxxxxxxxxxx wrote:
> Neil Brown writes:
> > On Wednesday August 15, maccetta@xxxxxxxxxxxxxxxxxx wrote:
> > >
> > > There are already files like /sys/block/md_d0/md/dev-sdb/errors in /sys
> > > which would be very convenient to consult but according to the kernel
> > > driver implementation the error counts reported there are apparently
> > > for corrected errors and not relevant for read errors during a "check"
> > > operation.
> > >
> >
> > When 'check' hits a read error, an attempt is made to 'correct' it by
> > over-writing with correct data. This will either increase the
> > 'errors' count or fail the drive completely.
> >
> > What 'check' doesn't do (and 'repair' does) it react when it find that
> > successful reads of all drives (in a raid1) do not match.
> >
> > So just use the 'errors' number - it is exactly what you want.
>
> This happens in our old friend sync_request_write()? I'm dealing with
Yes, that would be the place.
> simulated errors and will dig further to make sure that is not perturbing
> the results but I don't see any 'errors' effect. This is with our
> patched 2.6.20 raid1.c. The logic doesn't seem to be any different in
> 2.6.22 from what I can tell, though.
>
> This fragment
>
> if (j < 0 || test_bit(MD_RECOVERY_CHECK, &mddev->recovery)) {
> sbio->bi_end_io = NULL;
> rdev_dec_pending(conf->mirrors[i].rdev, mddev);
> } else {
> /* fixup the bio for reuse */
> ...
> }
>
> looks suspicously like any correction attempt for 'check' is being
> short-circuited to me, regardless of whether or not there was a read
> error. Actually, even if the rewrite was not being short-circuited,
> I still don't see the path that would update 'corrected_errors' in this
> case. There are only two raid1.c sites that touch 'corrected_errors', one
> is in fix_read_errors() and the other is later in sync_request_write().
> With my limited understanding of how this all works, neither of these
> paths would seem to apply here.
hmmm.... yes....
I guess I was thinking of the RAID5 code rather than the RAID1 code.
It doesn't do the right thing does it?
Maybe this patch is what we need. I think it is right.
Thanks,
NeilBrown
Signed-off-by: Neil Brown <neilb@xxxxxxx>
### Diffstat output
./drivers/md/raid1.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff .prev/drivers/md/raid1.c ./drivers/md/raid1.c
--- .prev/drivers/md/raid1.c 2007-08-16 10:29:58.000000000 +1000
+++ ./drivers/md/raid1.c 2007-08-17 12:07:35.000000000 +1000
@@ -1260,7 +1260,8 @@ static void sync_request_write(mddev_t *
j = 0;
if (j >= 0)
mddev->resync_mismatches += r1_bio->sectors;
- if (j < 0 || test_bit(MD_RECOVERY_CHECK, &mddev->recovery)) {
+ if (j < 0 || (test_bit(MD_RECOVERY_CHECK, &mddev->recovery)
+ && text_bit(BIO_UPTODATE, &sbio->bi_flags))) {
sbio->bi_end_io = NULL;
rdev_dec_pending(conf->mirrors[i].rdev, mddev);
} else {
-
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
