From owner-p4-projects@FreeBSD.ORG Tue Aug 14 12:22:38 2007 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 64BBD16A421; Tue, 14 Aug 2007 12:22:38 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2104216A41B for ; Tue, 14 Aug 2007 12:22:38 +0000 (UTC) (envelope-from lulf@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 079CC13C45E for ; Tue, 14 Aug 2007 12:22:38 +0000 (UTC) (envelope-from lulf@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.1/8.14.1) with ESMTP id l7ECMbal019526 for ; Tue, 14 Aug 2007 12:22:37 GMT (envelope-from lulf@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.1/8.14.1/Submit) id l7ECMbDZ019523 for perforce@freebsd.org; Tue, 14 Aug 2007 12:22:37 GMT (envelope-from lulf@FreeBSD.org) Date: Tue, 14 Aug 2007 12:22:37 GMT Message-Id: <200708141222.l7ECMbDZ019523@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to lulf@FreeBSD.org using -f From: Ulf Lilleengen To: Perforce Change Reviews Cc: Subject: PERFORCE change 125136 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Aug 2007 12:22:38 -0000 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;