Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 18 Jun 2007 08:36:51 GMT
From:      Ulf Lilleengen <lulf@FreeBSD.org>
To:        Perforce Change Reviews <perforce@FreeBSD.org>
Subject:   PERFORCE change 121899 for review
Message-ID:  <200706180836.l5I8apKE082969@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=121899

Change 121899 by lulf@lulf_carrot on 2007/06/18 08:35:54

	- Remove old comments, refer to plex instead of subdisk
	- Implement gv_init_plex which handles raid5 array initialization. It
	  uses the same model as parity/rebuild operations in that it is a send
	  and done function that handles issuing and handling finished init
	  BIOs.
	- Update plexstate when a rebuild is complete, and make sure it's
	  possible to take it up after a rebuild (set the CANGOUP bit).
	- Change GV_BIO_SUCCEED BIO flag to GV_BIO_INIT. This must be discussed
	  further, but for now it's needed for the INIT operation since
	  bio_cflags are only 8 bits, and it was unused. This may change in the
	  future though.
	- Update volstate after sync is complete too.
	- Create naming consistency with these new types of functions.

Affected files ...

.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.c#16 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.h#14 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_init.c#8 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_plex.c#13 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_raid5.c#5 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_subr.c#9 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_var.h#13 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_volume.c#6 edit

Differences ...

==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.c#16 (text+ko) ====

@@ -605,8 +605,6 @@
 				newstate = *(int *)ev->arg2;
 				flags = *(int *)ev->arg3;
 				err = gv_set_sd_state(s, newstate, flags);
-				/* XXX: Handle these errors better, provide
-				 * ERROR CODES.*/
 				if (err)
 					printf("VINUM: error setting subdisk "
 					    "state: error code %d\n", err);
@@ -620,8 +618,6 @@
 				newstate = *(int *)ev->arg2;
 				flags = *(int *)ev->arg3;
 				err = gv_set_drive_state(d, newstate, flags);
-				/* XXX: Handle these errors better, provide
-				 * ERROR CODES.*/
 				if (err)
 					printf("VINUM: error setting drive "
 					    "state: error code %d\n", err);
@@ -677,7 +673,7 @@
 					break;
 				}
 				p->synced = 0;
-				gv_send_parity_bio(p, GV_BIO_CHECK |
+				gv_parity_request(p, GV_BIO_CHECK |
 				    GV_BIO_PARITY, 0);
 				break;
 
@@ -691,7 +687,7 @@
 					break;
 				}
 				p->synced = 0;
-				gv_send_parity_bio(p, GV_BIO_CHECK, 0);
+				gv_parity_request(p, GV_BIO_CHECK, 0);
 				break;
 
 			case GV_EVENT_START_PLEX:
@@ -743,7 +739,7 @@
 				err = gv_detach_sd(s, 0);
 				if (err)
 					printf("VINUM: error detaching %s: "
-					    "error code %d\n", p->name, err);
+					    "error code %d\n", s->name, err);
 				break;
 
 			case GV_EVENT_THREAD_EXIT:

==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.h#14 (text+ko) ====

@@ -33,7 +33,6 @@
 void	gv_save_config(struct gv_softc *);
 
 /* geom_vinum_init.c */
-/*void	gv_parityop(struct g_geom *, struct gctl_req *);*/
 void	gv_start_obj(struct g_geom *, struct gctl_req *);
 int	gv_start_plex(struct gv_plex *);
 
@@ -118,7 +117,8 @@
 
 int	gv_stripe_active(struct gv_plex *, struct bio *);
 
-void	gv_send_parity_bio(struct gv_plex *, int, off_t);
+void	gv_init_request(struct gv_sd *, off_t, caddr_t, off_t);
+void	gv_parity_request(struct gv_plex *, int, off_t);
 void	gv_parityop(struct gv_softc *, struct gctl_req *);
 
 #endif /* !_GEOM_VINUM_H_ */

==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_init.c#8 (text+ko) ====

@@ -43,6 +43,7 @@
 static int	gv_start_vol(struct gv_volume *);
 static int	gv_sync(struct gv_volume *);
 static int	gv_rebuild_plex(struct gv_plex *);
+static int	gv_init_plex(struct gv_plex *);
 
 struct gv_sync_args {
 	struct gv_volume *v;
@@ -123,14 +124,13 @@
 	else if (p->org == GV_PLEX_RAID5) {
 		if (p->state == GV_PLEX_DEGRADED)
 			error = gv_rebuild_plex(p);
-/*		else
-			error = gv_init_plex(p);*/
+		else
+			error = gv_init_plex(p);
 	}
 
 	return (error);
 }
 
-#if 0
 int
 gv_start_vol(struct gv_volume *v)
 {
@@ -170,7 +170,6 @@
 
 	return (error);
 }
-#endif
 
 static int
 gv_sync(struct gv_volume *v)
@@ -237,15 +236,18 @@
 	p->flags |= GV_PLEX_SYNCING;
 	p->synced = 0;
 
-	gv_send_parity_bio(p, GV_BIO_REBUILD, 0);
+	gv_parity_request(p, GV_BIO_REBUILD, 0);
 	return (0);
 }
 
-#if 0
 static int
 gv_init_plex(struct gv_plex *p)
 {
+	struct gv_drive *d;
 	struct gv_sd *s;
+	int error;
+	off_t start;
+	caddr_t data;
 
 	KASSERT(p != NULL, ("gv_init_plex: NULL p"));
 
@@ -254,13 +256,32 @@
 			return (EINPROGRESS);
 		gv_set_sd_state(s, GV_SD_INITIALIZING, GV_SETSTATE_FORCE);
 		s->init_size = GV_DFLT_SYNCSIZE;
-		kthread_create(gv_init_td, s, NULL, 0, 0, "gv_init %s",
-		    s->name);
+		start = s->drive_offset + s->initialized;
+		d = s->drive_sc;
+		if (d == NULL) {
+			printf("VINUM: subdisk %s has no drive yet\n", s->name);
+			break;
+		}
+		/* 
+		 * Take the lock here since we need to avoid a race in
+		 * gv_init_request if the BIO is completed before the lock is
+		 * released.
+		 */
+		g_topology_lock();
+		error = g_access(d->consumer, 0, 1, 0);
+		g_topology_unlock();
+		if (error) {
+			printf("VINUM: error accessing consumer when "
+			    "initializing %s\n", s->name);
+			break; /* XXX: Or continue..? */
+		}
+		data = g_malloc(s->init_size, M_WAITOK | M_ZERO);
+		gv_init_request(s, start, data, s->init_size);
 	}
-
 	return (0);
 }
 
+#if 0
 /* This thread is responsible for rebuilding a degraded RAID5 plex. */
 void
 gv_rebuild_td(void *arg)

==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_plex.c#13 (text+ko) ====

@@ -45,8 +45,9 @@
 		    struct gv_raid5_packet *);
 static int	gv_normal_parity(struct gv_plex *, struct bio *,
 		    struct gv_raid5_packet *);
-static void	gv_parity_completed(struct gv_plex *, struct bio *);
-static void	gv_rebuild_completed(struct gv_plex *, struct bio *);
+static void	gv_parity_complete(struct gv_plex *, struct bio *);
+static void	gv_rebuild_complete(struct gv_plex *, struct bio *);
+static void	gv_init_complete(struct gv_plex *, struct bio *);
 static struct bio * gv_plexbuffer(struct gv_plex *, struct bio *, caddr_t,
 			off_t, off_t, int *);
 
@@ -368,10 +369,12 @@
 		/* Hand it over for checking or delivery. */
 		if (pbp->bio_cmd == BIO_WRITE &&
 		    (pbp->bio_cflags & GV_BIO_CHECK)) {
-			gv_parity_completed(p, pbp);
+			gv_parity_complete(p, pbp);
 		} else if (pbp->bio_cmd == BIO_WRITE &&
 		    (pbp->bio_cflags & GV_BIO_REBUILD)) {
-			gv_rebuild_completed(p, pbp);
+			gv_rebuild_complete(p, pbp);
+		} else if (pbp->bio_cflags & GV_BIO_INIT) {
+			gv_init_complete(p, pbp);
 		} else {
 			g_io_deliver(pbp, pbp->bio_error);
 		}
@@ -473,12 +476,12 @@
  * rebuild of degraded plexes as well as user initiated rebuilds/checks.
  */
 void
-gv_send_parity_bio(struct gv_plex *p, int flags, off_t offset)
+gv_parity_request(struct gv_plex *p, int flags, off_t offset)
 {
 	struct bio *bp;
 	int error;
 
-	KASSERT(p != NULL, ("gv_send_parity_bio: NULL p"));
+	KASSERT(p != NULL, ("gv_parity_request: NULL p"));
 
 	/* Make sure we don't have the lock. */
 	g_topology_assert_not();
@@ -531,10 +534,113 @@
 }
 
 /*
+ * Handle a finished initialization BIO.
+ */
+static void
+gv_init_complete(struct gv_plex *p, struct bio *bp)
+{
+	struct gv_drive *d;
+	struct g_consumer *cp;
+	struct gv_sd *s;
+	off_t start, length;
+	caddr_t data;
+	int error;
+
+	s = bp->bio_caller1;
+	start = bp->bio_offset;
+	length = bp->bio_length;
+	error = bp->bio_error;
+	data = bp->bio_data;
+
+	KASSERT(s != NULL, ("gv_init_complete: NULL s"));
+	d = s->drive_sc;
+	KASSERT(d != NULL, ("gv_init_complete: NULL d"));
+	cp = d->consumer;
+	KASSERT(cp != NULL, ("gv_init_complete: NULL cp"));
+
+	g_destroy_bio(bp);
+
+	/*
+	 * First we need to find out if it was okay, and abort if it's not.
+	 * Then we need to free previous buffers, find out the correct subdisk,
+	 * as well as getting the correct starting point and length of the BIO.
+	 */
+	if (start >= s->drive_offset + s->size) {
+		/* Free the data we initialized. */
+		if (data != NULL)
+			g_free(data);
+		g_topology_assert_not();
+		g_topology_lock();
+		g_access(cp, 0, -1, 0);
+		g_topology_unlock();
+		if (error) {
+			gv_set_sd_state(s, GV_SD_STALE, GV_SETSTATE_FORCE |
+			    GV_SETSTATE_CONFIG);
+		} else {
+			gv_set_sd_state(s, GV_SD_UP, GV_SETSTATE_CONFIG);
+			s->initialized = 0;
+			printf("VINUM: subdisk '%s' init: finished "
+			    "successfully\n", s->name);
+		}
+		return;
+	}
+	s->initialized += length;
+	start += length;
+	gv_init_request(s, start, data, length);
+}
+
+/*
+ * Create an initialization BIO and send it off to the consumer. Assume that
+ * we're given initialization data as parameter.
+ */
+void
+gv_init_request(struct gv_sd *s, off_t start, caddr_t data, off_t length)
+{
+	struct gv_drive *d;
+	struct g_consumer *cp;
+	struct bio *bp, *cbp;
+
+	KASSERT(s != NULL, ("gv_init_request: NULL s"));
+	d = s->drive_sc;
+	KASSERT(d != NULL, ("gv_init_request: NULL d"));
+	cp = d->consumer;
+	KASSERT(cp != NULL, ("gv_init_request: NULL cp"));
+
+	bp = g_new_bio();
+	if (bp == NULL) {
+		printf("VINUM: subdisk '%s' init: write failed at offset %jd"
+		    " (drive offset %jd); out of memory\n", s->name,
+		    (intmax_t)s->initialized, (intmax_t)start);
+		return; /* XXX: Error codes. */
+	}
+	bp->bio_cmd = BIO_WRITE;
+	bp->bio_data = data;
+	bp->bio_done = gv_done;
+	bp->bio_error = 0;
+	bp->bio_length = length;
+	bp->bio_cflags |= GV_BIO_INIT;
+	bp->bio_offset = start;
+	bp->bio_caller1 = s;
+
+	/* Then ofcourse, we have to clone it. */
+	cbp = g_clone_bio(bp);
+	if (cbp == NULL) {
+		printf("VINUM: subdisk '%s' init: write failed at offset %jd"
+		    " (drive offset %jd); out of memory\n", s->name,
+		    (intmax_t)s->initialized, (intmax_t)start);
+		return; /* XXX: Error codes. */
+	}
+	cbp->bio_done = gv_done;
+	cbp->bio_caller1 = s;
+	/* Send it off to the consumer. */
+	g_io_request(cbp, cp);
+}
+
+/*
  * Handle a finished parity write.
  */
 static void
-gv_parity_completed(struct gv_plex *p, struct bio *bp)
+gv_parity_complete(struct gv_plex *p, struct bio *bp)
 {
 	int error, flags;
 
@@ -576,15 +682,16 @@
 	}
 
 	/* Send down next. It will determine if we need to itself. */
-	gv_send_parity_bio(p, flags, p->synced);
+	gv_parity_request(p, flags, p->synced);
 }
 
 /*
  * Handle a finished plex rebuild bio.
  */
 static void
-gv_rebuild_completed(struct gv_plex *p, struct bio *bp)
+gv_rebuild_complete(struct gv_plex *p, struct bio *bp)
 {
+	struct gv_sd *s;
 	int error, flags;
 	off_t offset;
 
@@ -617,11 +724,14 @@
 		gv_save_config(p->vinumconf);
 		p->flags &= ~GV_PLEX_SYNCING;
 		p->synced = 0;
+		/* Try to up all subdisks. */
+		LIST_FOREACH(s, &p->subdisks, in_plex)
+			gv_update_sd_state(s);
 		return;
 	}
 
 	/* Send down next. It will determine if we need to itself. */
-	gv_send_parity_bio(p, flags, offset);
+	gv_parity_request(p, flags, offset);
 }
 
 void

==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_raid5.c#5 (text+ko) ====

@@ -267,6 +267,8 @@
 
 		printf("GEOM_VINUM: sd %s is reviving\n", broken->name);
 		gv_set_sd_state(broken, GV_SD_REVIVING, GV_SETSTATE_FORCE);
+		/* Set this bit now, but should be set at end. */
+		broken->flags |= GV_SD_CANGOUP;
 		break;
 
 	case GV_SD_REVIVING:

==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_subr.c#9 (text+ko) ====


==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_var.h#13 (text+ko) ====

@@ -112,7 +112,7 @@
 #define	GV_BIO_MALLOC	0x02
 #define	GV_BIO_ONHOLD	0x04
 #define	GV_BIO_SYNCREQ	0x08
-#define	GV_BIO_SUCCEED	0x10
+#define	GV_BIO_INIT	0x10
 #define	GV_BIO_REBUILD	0x20
 #define	GV_BIO_CHECK	0x40
 #define	GV_BIO_PARITY	0x80

==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_volume.c#6 (text+ko) ====

@@ -41,7 +41,7 @@
 #include <geom/vinum/geom_vinum_var.h>
 #include <geom/vinum/geom_vinum.h>
 
-static void	gv_sync_completed(struct gv_plex *, struct bio *);
+static void	gv_sync_complete(struct gv_plex *, struct bio *);
 
 void
 gv_volume_start(struct gv_softc *sc, struct bio *bp)
@@ -132,7 +132,7 @@
 		pbp->bio_inbed++;
 		if (pbp->bio_children == pbp->bio_inbed) {
 			if (pbp->bio_cflags & GV_BIO_SYNCREQ)
-				gv_sync_completed(p, pbp);
+				gv_sync_complete(p, pbp);
 			else {
 				/* 
 				 * If completed is a multiple of a number less
@@ -175,9 +175,10 @@
  * Handle a finished plex sync bio.
  */
 static void
-gv_sync_completed(struct gv_plex *to, struct bio *bp)
+gv_sync_complete(struct gv_plex *to, struct bio *bp)
 {
 	struct gv_plex *from, *p;
+	struct gv_sd *s;
 	struct gv_volume *v;
 	int err;
 
@@ -200,6 +201,9 @@
 			printf("VINUM: syncing of %s from %s completed\n",
 			    to->name, from->name);
 			to->flags &= ~GV_PLEX_SYNCING;
+			/* Update our state. */
+			LIST_FOREACH(s, &to->subdisks, in_plex)
+				gv_update_sd_state(s);
 		} else {
 			err = gv_sync_request(from, to, bp->bio_offset +
 			    bp->bio_length, bp->bio_length, BIO_READ, NULL);
@@ -225,8 +229,8 @@
 }
 
 int
-gv_sync_request(struct gv_plex *from, struct gv_plex *to, off_t offset, off_t
-    length, int type, caddr_t data)
+gv_sync_request(struct gv_plex *from, struct gv_plex *to, off_t offset,
+    off_t length, int type, caddr_t data)
 {
 	struct bio *bp;
 
@@ -247,9 +251,6 @@
 	bp->bio_cflags |= GV_BIO_MALLOC; /* Free on the next run. */
 	bp->bio_data = data;
 
-/*	printf("Sending next bio:\n ");
-	g_print_bio(bp);
-	printf("\n");*/
 	/* Send down next. */
 	gv_plex_start(from, bp);
 	return (0);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200706180836.l5I8apKE082969>