PERFORCE change 123208 for review

Sonja Milicic smilicic at FreeBSD.org
Mon Jul 9 16:25:05 UTC 2007


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

Change 123208 by smilicic at tanarri_marilith on 2007/07/09 16:24:02

	fixed a bug with putting worker thread to sleep

Affected files ...

.. //depot/projects/soc2007/smilicic_glog/sys/geom/log/glog.c#5 edit

Differences ...

==== //depot/projects/soc2007/smilicic_glog/sys/geom/log/glog.c#5 (text+ko) ====

@@ -66,6 +66,7 @@
 static int g_log_access(struct g_provider *pp, int dr, int dw, int de);
 static int g_log_event_sink_init(struct g_log_softc *sc, 
 	    struct g_log_event_sink *es, void (*func)(void*), char* name);
+static void g_log_event_sink_destroy(struct g_log_event_sink *es); 
 static void g_log_worker(void *args); 
 static int g_log_post_event(struct g_log_event_sink *es, u_int type, 
 	    u_int flags, void* data1, int data2);
@@ -167,7 +168,7 @@
 	
 	/*create geom for log*/
 	gp = g_new_geomf(mp, "%s.log", prov);
-        G_LOG_DEBUG(0, "Creating geom %s", gp->name);
+	G_LOG_DEBUG(0, "Creating geom %s", gp->name);
 
 	gp->start = g_log_start;
 	gp->spoiled = g_log_orphan;
@@ -196,16 +197,19 @@
 	g_error_provider(pp_log, 0);
 	sc->sc_prov_log = pp_log;
 	
-	if (g_log_event_sink_init(sc, &sc->sc_events, g_log_worker, "events") !=0)
+	if (g_log_event_sink_init(sc, &sc->sc_events, g_log_worker, "events") !=0){
 		*err=4;
+		g_log_event_sink_destroy(&sc->sc_events);
+	}
 	/*open file*/
 	sc->sc_vn = glog_open_file(file, FWRITE | O_TRUNC | O_CREAT);
 	sc->sc_geom_log = gp;
-        gp->softc = sc;
+	gp->softc = sc;
 	
 	return gp;
 }
 
+/*initialize sink thread*/
 static int
 g_log_event_sink_init(struct g_log_softc *sc, struct g_log_event_sink *es, 
 	    void (*func)(void*), char* name)
@@ -222,6 +226,18 @@
 	return 0;
 }
 
+/* destroy sink thread */
+static void
+g_log_event_sink_destroy(struct g_log_event_sink *es) 
+{
+    g_log_post_event(es, GLOG_EVSTOP, GLOG_FLAG_WAKEUP_SC, NULL, 0);
+    while (es->worker_thread != NULL) /* wait for the worker thread to die */
+        tsleep(&es->worker_thread, PRIBIO, "wrkoff", hz/5);
+    mtx_destroy(&es->eventq_mtx);
+    bzero(es, sizeof(*es));
+}
+
+/*process ctl requests*/
 static void
 g_log_ctlreq(struct gctl_req *req, struct g_class *mp, const char *verb)
 {
@@ -310,32 +326,35 @@
 g_log_stop(struct g_geom *gp, int force)
 {
 	struct g_log_softc *sc;
-        struct g_provider *pp_disk, *pp_log;
+	struct g_provider *pp_disk, *pp_log;
 	struct g_consumer *cp_disk;
 	sc=gp->softc;
-        
+	
        	g_topology_assert();
 
 	if (sc==NULL)
-                return (ENXIO);
-        
-        pp_log = sc->sc_prov_log;
+		return (ENXIO);
+	
+	pp_log = sc->sc_prov_log;
 	pp_disk = sc->sc_prov_disk;
 	cp_disk = sc->sc_cons_disk;
 	
-        if (pp_log != NULL && (pp_log->acr != 0 || pp_log->acw !=0 || pp_log->ace != 0)){
-                if (force)
-                        G_LOG_DEBUG(0, "Device %s is still open.", pp_log->name);
-                else {
-                        G_LOG_DEBUG(1, "Device %s is still open(r%d, w%d, e%d)",
-                            pp_log->name,pp_log->acr,pp_log->acw,pp_log->ace);
-                        return (EBUSY);
-                }
-        }
+	if (pp_log != NULL && (pp_log->acr != 0 || pp_log->acw !=0 || pp_log->ace != 0)){
+		if (force)
+			G_LOG_DEBUG(0, "Device %s is still open.", pp_log->name);
+		else {
+			G_LOG_DEBUG(1, "Device %s is still open(r%d, w%d, e%d)",
+			    pp_log->name,pp_log->acr,pp_log->acw,pp_log->ace);
+			return (EBUSY);
+		}
+	}
 	/*close log file*/
-        G_LOG_DEBUG(0, "Closing log file.");
+	G_LOG_DEBUG(0, "Closing log file.");
 	glog_close_file(sc->sc_vn, FWRITE);
 	
+	/*destory worker thread*/
+	g_log_event_sink_destroy(&sc->sc_events);
+	
 	/*clean up memory*/
 	G_LOG_DEBUG(0,"cleaning up mem.");
 	g_orphan_provider(pp_log, ENXIO);
@@ -349,12 +368,12 @@
 	
 	/*destroy geom*/
 	if (pp_log == NULL || (pp_log->acr == 0 && pp_log->acw == 0 && pp_log->ace == 0))
-                G_LOG_DEBUG(0, "Device %s destroyed.", gp->name);
-        
+		G_LOG_DEBUG(0, "Device %s destroyed.", gp->name);
+	
 	g_wither_geom(gp, ENXIO);
 	
-        G_LOG_DEBUG(0, "Really destroyed %s.", gp->name);
-        return 0;
+	G_LOG_DEBUG(0, "Really destroyed %s.", gp->name);
+	return 0;
 }
 
 static int
@@ -387,46 +406,36 @@
 		err = g_access(cp, dr ,dw, de);
 		if (err == 0)
 			continue;
-                G_LOG_DEBUG(0, "loop access");
+		G_LOG_DEBUG(0, "loop access");
 	}
 	return err;
 }
 
-/*puts worker thread to sleep if there's nothing for it to do*/
-static void
-g_log_worker_sleep(struct g_log_softc *sc)
-{
-	if (g_log_no_events(&sc->sc_events)){
-		G_LOG_DEBUG(0, "putting worker to sleep");
-		tsleep(sc, PRIBIO, "glogidle", hz);
-	}
-}
-
 /*worker thread*/
 static void
 g_log_worker(void *args) 
 {
 	struct g_log_event_sink *es;
 	struct g_log_softc *sc;
-	struct g_log_event *ev;
+	struct g_log_event *ev = NULL;
 	struct bio *bp;
+
 	es = args;
-	
-	if (es == NULL)
-		panic("No event sink!");
+	KASSERT(es != NULL, ("%s: event_sink is null", __func__));
 	sc = es->sc;
-	if (sc == NULL)
-		panic("No softc!");
+	KASSERT(sc != NULL, ("%s: softc is null", __func__));
 	
 	while (1){
-                G_LOG_DEBUG(0,"working...");
-		ev = g_log_get_event(&sc->sc_events);
-		while (ev == NULL) 
-			g_log_worker_sleep(sc);
+		G_LOG_DEBUG(0, "working...");
+		if (!g_log_no_events(es))
+			ev = g_log_get_event(es);
+		else
+			goto sleep;
+		G_LOG_DEBUG(0, "event: %d", ev->type);
 		bp = ev->data1;
 		switch (ev->type) {
 		case GLOG_EVCOMMIT:
-			break;
+			break; 
 		case GLOG_EVROLLBACK:
 			break;
 		case GLOG_EVREAD:
@@ -437,9 +446,14 @@
 			break;
 		case GLOG_EVSTOP:
 			break;
+		default:
+			G_LOG_DEBUG(0, "unhandled event %d", ev->type);
 		}
 		free(ev,M_GLOG);
 	}
+sleep:		G_LOG_DEBUG(0, "putting worker to sleep");
+		    tsleep(es, PRIBIO, "glogidle", hz);
+		
 }
 /* adds event to event queue */
 static int
@@ -465,7 +479,7 @@
 	mtx_lock(&es->eventq_mtx);
 	TAILQ_INSERT_TAIL(&es->eventq, ev, linkage);
 	mtx_unlock(&es->eventq_mtx);
-
+	G_LOG_DEBUG (0, "posted event %d", ev->type);
 	if ( (flags & GLOG_FLAG_WAKEUP_SC) != 0){
 		G_LOG_DEBUG(0, "waking worker");
 		wakeup(es);
@@ -475,30 +489,32 @@
 }
 
 /*gets next event from event queue*/
-static struct g_log_event*
+/*BUG - causes a panic when there are no events*/
+static struct g_log_event *
 g_log_get_event(struct g_log_event_sink *es) 
 {
-    struct g_log_softc *sc;
-    struct g_log_event *ev;
+	struct g_log_softc *sc;
+	struct g_log_event *ev;
 
-    KASSERT(es != NULL, ("%s: event_sink is null", __func__));
-    sc = es->sc;
-    KASSERT(sc != NULL, ("%s: softc is null", __func__));
-    
-    mtx_lock(&es->eventq_mtx);
-    ev = TAILQ_FIRST(&es->eventq);
-    if (ev != NULL)
-	TAILQ_REMOVE(&es->eventq, ev, linkage);
-    mtx_unlock(&es->eventq_mtx);
-
-    return ev;
+	KASSERT(es != NULL, ("%s: event_sink is null", __func__));
+	sc = es->sc;
+	KASSERT(sc != NULL, ("%s: softc is null", __func__));
+	if (g_log_no_events(es))
+		G_LOG_DEBUG(0, "no events");
+	mtx_lock(&es->eventq_mtx);
+	ev = TAILQ_FIRST(&es->eventq);
+	if (ev != NULL)
+		TAILQ_REMOVE(&es->eventq, ev, linkage);
+	mtx_unlock(&es->eventq_mtx);
+	
+	return ev;
 }
 
 /*is the event queue empty?*/
 static int
 g_log_no_events(struct g_log_event_sink *es) 
 {
-    return TAILQ_EMPTY(&es->eventq);
+	return TAILQ_EMPTY(&es->eventq);
 }
 
 /*writes data to log file*/
@@ -508,21 +524,19 @@
 	struct g_log_softc *sc;
 	void *data;
 	int err;
-        G_LOG_DEBUG(0, "write request");
+	G_LOG_DEBUG(0, "write request");
 	sc = bp->bio_to->geom->softc;
 	data = bp->bio_data;
 	err = glog_write_file(sc->sc_vn, data, sizeof(data), 0);
 	if (err != 0)
-                G_LOG_DEBUG(0, "write error");
-
-	
+		G_LOG_DEBUG(0, "write error");
 }
 
-/*reads data*/
+/*reads data from log file and/or disk*/
 static void
 g_log_read(struct bio *bp)
 {
-       uprintf("Got a read request.");
+       G_LOG_DEBUG(0,"Got a read request.");
 	
 }
 
@@ -551,16 +565,14 @@
 static struct g_log_softc *
 g_log_find(struct g_class *mp, const char *name)
 {
-    struct g_geom *gp;
+	struct g_geom *gp;
 
-    G_LOG_DEBUG(DBG_DEBUG, "%s: %s", __func__, name);
-    LIST_FOREACH(gp, &mp->geom, geom) {
-        if (strcmp(gp->name, name) == 0)
-            return (gp->softc);
-	G_LOG_DEBUG(0, "loop log_find");
-
-    }
-    return (NULL);
+	G_LOG_DEBUG(DBG_DEBUG, "%s: %s", __func__, name);
+	LIST_FOREACH(gp, &mp->geom, geom) {
+	if (strcmp(gp->name, name) == 0)
+		return (gp->softc);
+	}
+	return (NULL);
 }
 
 static void
@@ -580,11 +592,10 @@
 
 	force = gctl_get_paraml(req, "force", sizeof(int));
 	
-        sc = g_log_find(mp, prov);
-        if (sc != NULL)
-                g_log_stop(sc->sc_geom_log, *force);
-        else 
-                panic("Softc is null in ctl_destroy!");
+	sc = g_log_find(mp, prov);
+	KASSERT (sc != NULL, ("Softc is null in %s!", __func__));
+	g_log_stop(sc->sc_geom_log, *force);
+	
 }
 /*XML*/
 static void
@@ -612,8 +623,8 @@
 		G_LOG_DEBUG(0, "error=%d", sc->sc_prov_log->error);
 		sbuf_printf(sb, "</State>\n");
 	}
-        
-        G_LOG_DEBUG(0, "xmldump");
+	
+	G_LOG_DEBUG(0, "xmldump");
 }
 		    
 /* Convert verb to number */
@@ -633,3 +644,4 @@
 };
 
 DECLARE_GEOM_CLASS(g_log_class, g_log);
+


More information about the p4-projects mailing list