Date: Fri, 25 May 2007 13:34:21 GMT From: Ulf Lilleengen <lulf@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 120381 for review Message-ID: <200705251334.l4PDYL1R048890@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=120381 Change 120381 by lulf@lulf_carrot on 2007/05/25 13:34:04 - Add error handling to the parity checking. - Call gv_access before we issue bio's since we need to actually get permission to read/write to the consumers. - Reset access counters when calling gv_parity_completed. - Add topology locking where needed. Might change the scheme for this later. - Make gv_access availiable to other functions. Affected files ... .. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.c#11 edit .. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.h#9 edit .. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_plex.c#6 edit Differences ... ==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.c#11 (text+ko) ==== @@ -116,7 +116,7 @@ mtx_unlock(&sc->queue_mtx); } -static int +int gv_access(struct g_provider *pp, int dr, int dw, int de) { struct g_geom *gp; ==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.h#9 (text+ko) ==== @@ -96,6 +96,7 @@ void gv_drive_lost(struct gv_softc *, struct gv_drive *); void gv_setup_objects(struct gv_softc *); void gv_start(struct bio *); +int gv_access(struct g_provider *, int, int, int); void gv_done(struct bio *); void gv_volume_start(struct gv_softc *, struct bio *); ==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_plex.c#6 (text+ko) ==== @@ -367,11 +367,12 @@ pbp->bio_inbed++; if (pbp->bio_inbed == pbp->bio_children) { + printf("Error is %d\n", pbp->bio_error); /* XXX: Should this be done here? Probably to be done later, but * just for testing. */ - if (pbp->bio_cmd == BIO_WRITE && (pbp->bio_cflags & GV_BIO_PARITY)) { + if (pbp->bio_cmd == BIO_WRITE && + (pbp->bio_cflags & GV_BIO_CHECK)) { /* Schedule a new bio down to rebuild parity. */ - printf("Finished with rebuild at %lld\n", p->synced); gv_parity_completed(p, pbp); } else g_io_deliver(pbp, pbp->bio_error); @@ -477,20 +478,30 @@ gv_issue_next_parity_bio(struct gv_plex *p, int rebuild) { struct bio *bp; + int error; KASSERT(p != NULL, ("gv_issue_next_parity_bio: NULL p")); + g_topology_lock(); + error = gv_access(p->vol_sc->provider, 1, 1, 0); + if (error) { + printf("VINUM: unable to access provider\n"); + goto bad; + } bp = g_new_bio(); if (bp == NULL) { printf("VINUM: rebuild of %s failed creating bio: " "out of memory\n", p->name); - return; + gv_access(p->vol_sc->provider, -1, -1, 0); + goto bad; } + g_topology_unlock(); bp->bio_cmd = BIO_WRITE; bp->bio_done = NULL; bp->bio_data = g_malloc(p->stripesize, M_WAITOK | M_ZERO); + bp->bio_error = 0; /* bp->bio_cflags |= GV_BIO_REBUILD;*/ bp->bio_cflags |= GV_BIO_CHECK; if (rebuild) @@ -498,22 +509,16 @@ bp->bio_cflags |= GV_BIO_MALLOC; bp->bio_length = p->stripesize; - if (p->synced >= p->size) { - /* We're finished. */ - printf("VINUM: rebuild of %s finished\n", p->name); - g_free(bp->bio_data); - g_destroy_bio(bp); - p->synced = 0; - return; - } - /* We still have more parity to build. */ bp->bio_offset = p->synced; /* Now we need to find the correct subdisk to send it to. */ - p->synced += p->stripesize; - printf("VINUM: Issuing bio with offset %lld\n", bp->bio_offset); + printf("VINUM: Issuing bio at offset 0x%jx\n", + (intmax_t)bp->bio_offset); gv_plex_start(p, bp); /* Send it down to the plex. */ + return; +bad: + g_topology_unlock(); } /* @@ -530,14 +535,33 @@ g_free(bp->bio_data); g_destroy_bio(bp); + g_topology_lock(); + gv_access(p->vol_sc->provider, -1, -1, 0); + g_topology_unlock(); /* Clean up what we allocated. */ /* XXX: Check for error type. */ if (error) { - printf("VINUM: Parity operation error on %s at %lld\n", p->name, - p->synced); + if (error == EAGAIN) { + printf("VINUM: Parity incorrect at offset 0x%jx\n", + (intmax_t)p->synced); + if (!rebuild) + return; + } + printf("VINUM: Parity check on %s failed at 0x%jx errno %d\n", + p->name, (intmax_t)p->synced, error); + } else { + p->synced += p->stripesize; + } + printf("VINUM: Parity operation at 0x%jx finished\n", + (intmax_t)p->synced); + + + if (p->synced >= p->size) { + /* We're finished. */ + printf("VINUM: Parity operation on %s finished\n", p->name); + p->synced = 0; return; } - printf("VINUM: Parity operation at %lld finished\n", p->synced); /* Send down next. It will determine if we need to itself. */ gv_issue_next_parity_bio(p, rebuild);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200705251334.l4PDYL1R048890>