Date: Tue, 14 Aug 2007 12:22:37 GMT From: Ulf Lilleengen <lulf@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 125136 for review Message-ID: <200708141222.l7ECMbDZ019523@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=125136 Change 125136 by lulf@lulf_carrot on 2007/08/14 12:22:18 - Modify rebuild and parity routines to increase providers access counts before starting and after finishing rebuild instead of doing it for each rebuild bio. This caused the rest of gvinum to lock up while rebuild since the topology lock was aquired all the time. - Change output of list to show how far we're in the rebuild process. Affected files ... .. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.c#36 edit .. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_init.c#26 edit .. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_list.c#6 edit .. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_plex.c#28 edit Differences ... ==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.c#36 (text+ko) ==== @@ -745,6 +745,15 @@ break; } p->synced = 0; + g_topology_assert_not(); + g_topology_lock(); + err = gv_access(p->vol_sc->provider, 1, 1, 0); + if (err) { + printf("VINUM: unable to access " + "provider\n"); + break; + } + g_topology_unlock(); gv_parity_request(p, GV_BIO_CHECK | GV_BIO_PARITY, 0); break; @@ -759,6 +768,15 @@ break; } p->synced = 0; + g_topology_assert_not(); + g_topology_lock(); + err = gv_access(p->vol_sc->provider, 1, 1, 0); + if (err) { + printf("VINUM: unable to access " + "provider\n"); + break; + } + g_topology_unlock(); gv_parity_request(p, GV_BIO_CHECK, 0); break; ==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_init.c#26 (text+ko) ==== @@ -221,6 +221,7 @@ { struct gv_drive *d; struct gv_sd *s; + int error; /* XXX: Is this safe? (Allows for mounted rebuild)*/ /* if (gv_provider_is_open(p->vol_sc->provider)) @@ -245,6 +246,15 @@ p->flags |= GV_PLEX_REBUILDING; p->synced = 0; + g_topology_assert_not(); + g_topology_lock(); + error = gv_access(p->vol_sc->provider, 1, 1, 0); + if (error) { + printf("VINUM: unable to access provider\n"); + return (0); + } + g_topology_unlock(); + gv_parity_request(p, GV_BIO_REBUILD, 0); return (0); } ==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_list.c#6 (text+ko) ==== @@ -294,7 +294,9 @@ p->name, (intmax_t)p->size, (intmax_t)p->size / MEGABYTE); sbuf_printf(sb, "\t\tSubdisks: %8d\n", p->sdcount); sbuf_printf(sb, "\t\tState: %s\n", gv_plexstate(p->state)); - if ((p->flags & GV_PLEX_SYNCING) || (p->flags & GV_PLEX_GROWING)) { + if ((p->flags & GV_PLEX_SYNCING) || + (p->flags & GV_PLEX_GROWING) || + (p->flags & GV_PLEX_REBUILDING)) { sbuf_printf(sb, "\t\tSynced: "); sbuf_printf(sb, "%16jd bytes (%d%%)\n", (intmax_t)p->synced, @@ -312,7 +314,9 @@ } else { sbuf_printf(sb, "P %-18s %2s State: ", p->name, gv_plexorg_short(p->org)); - if ((p->flags & GV_PLEX_SYNCING) || (p->flags & GV_PLEX_GROWING)) { + if ((p->flags & GV_PLEX_SYNCING) || + (p->flags & GV_PLEX_GROWING) || + (p->flags & GV_PLEX_REBUILDING)) { sbuf_printf(sb, "S %d%%\t", (int)((p->synced * 100) / p->size)); } else { ==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_plex.c#28 (text+ko) ==== @@ -856,27 +856,15 @@ gv_parity_request(struct gv_plex *p, int flags, off_t offset) { struct bio *bp; - int error; KASSERT(p != NULL, ("gv_parity_request: NULL p")); - /* Make sure we don't have the lock. */ - g_topology_assert_not(); - 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); - gv_access(p->vol_sc->provider, -1, -1, 0); - goto bad; + return; } - g_topology_unlock(); bp->bio_cmd = BIO_WRITE; bp->bio_done = gv_done; @@ -903,9 +891,6 @@ bp->bio_offset = offset; gv_plex_start(p, bp); /* Send it down to the plex. */ - return; -bad: - g_topology_unlock(); } /* @@ -925,12 +910,13 @@ g_free(bp->bio_data); g_destroy_bio(bp); - /* Make sure we don't have the lock. */ - g_topology_assert_not(); - g_topology_lock(); - gv_access(p->vol_sc->provider, -1, -1, 0); - g_topology_unlock(); if (error) { + /* Make sure we don't have the lock. */ + g_topology_assert_not(); + g_topology_lock(); + gv_access(p->vol_sc->provider, -1, -1, 0); + g_topology_unlock(); + if (error == EAGAIN) { printf("VINUM: Parity incorrect at offset 0x%jx\n", (intmax_t)p->synced); @@ -942,11 +928,14 @@ } else { p->synced += p->stripesize; } - printf("VINUM: Parity operation at 0x%jx finished\n", - (intmax_t)p->synced); + if (p->synced >= p->size) { + /* Make sure we don't have the lock. */ + g_topology_assert_not(); + g_topology_lock(); + gv_access(p->vol_sc->provider, -1, -1, 0); + g_topology_unlock(); - if (p->synced >= p->size) { /* We're finished. */ printf("VINUM: Parity operation on %s finished\n", p->name); p->synced = 0; @@ -977,12 +966,12 @@ g_free(bp->bio_data); g_destroy_bio(bp); - /* Make sure we don't have the lock. */ - g_topology_assert_not(); - g_topology_lock(); - gv_access(p->vol_sc->provider, -1, -1, 0); - g_topology_unlock(); if (error) { + g_topology_assert_not(); + g_topology_lock(); + gv_access(p->vol_sc->provider, -1, -1, 0); + g_topology_unlock(); + printf("VINUM: rebuild of %s failed at offset %jd errno: %d\n", p->name, (intmax_t)offset, error); p->flags &= ~GV_PLEX_REBUILDING; @@ -994,6 +983,11 @@ offset += (p->stripesize * (gv_sdcount(p, 1) - 1)); if (offset >= p->size) { /* We're finished. */ + g_topology_assert_not(); + g_topology_lock(); + gv_access(p->vol_sc->provider, -1, -1, 0); + g_topology_unlock(); + printf("VINUM: rebuild of %s finished\n", p->name); gv_save_config(p->vinumconf); p->flags &= ~GV_PLEX_REBUILDING;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200708141222.l7ECMbDZ019523>