PERFORCE change 125136 for review

[Ulf Lilleengen]

http://perforce.freebsd.org/chv.cgi?CH=125136

Change 125136 by lulf at 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;