Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 21 Jun 2016 20:18:19 +0000 (UTC)
From:      "Kenneth D. Merry" <ken@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r302069 - head/sys/geom
Message-ID:  <201606212018.u5LKIJ7C034784@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: ken
Date: Tue Jun 21 20:18:19 2016
New Revision: 302069
URL: https://svnweb.freebsd.org/changeset/base/302069

Log:
  Fix a bug that caused da(4) instances to hang around after the underlying
  device is gone.
  
  The problem was that when disk_gone() is called, if the GEOM disk
  creation process has not yet happened, the withering process
  couldn't start.
  
  We didn't record any state in the GEOM disk code, and so the d_gone()
  callback to the da(4) driver never happened.
  
  The solution is to track the state of the creation process, and
  initiate the withering process from g_disk_create() if the disk is
  being created.
  
  This change does add fields to struct disk, and so I have bumped
  DISK_VERSION.
  
  geom_disk.c:	Track where we are in the disk creation process,
  		and check to see whether our underlying disk has
  		gone away or not.
  
  		In disk_gone(), set a new d_goneflag variable that
  		g_disk_create() can check to see if it needs to
  		clean up the disk instance.
  
  geom_disk.h:    Add a mutex to struct disk (for internal use) disk
  		init level, and a gone flag.
  
  		Bump DISK_VERSION because the size of struct disk has
  		changed and fields have been added at the beginning.
  
  Sponsored by:	Spectra Logic
  Approved by:	re (marius)

Modified:
  head/sys/geom/geom_disk.c
  head/sys/geom/geom_disk.h

Modified: head/sys/geom/geom_disk.c
==============================================================================
--- head/sys/geom/geom_disk.c	Tue Jun 21 20:15:30 2016	(r302068)
+++ head/sys/geom/geom_disk.c	Tue Jun 21 20:18:19 2016	(r302069)
@@ -669,6 +669,22 @@ g_disk_create(void *arg, int flag)
 		return;
 	g_topology_assert();
 	dp = arg;
+
+	mtx_lock(&dp->d_mtx);
+	dp->d_init_level = DISK_INIT_START;
+
+	/*
+	 * If the disk has already gone away, we can just stop here and
+	 * call the user's callback to tell him we've cleaned things up.
+	 */
+	if (dp->d_goneflag != 0) {
+		mtx_unlock(&dp->d_mtx);
+		if (dp->d_gone != NULL)
+			dp->d_gone(dp);
+		return;
+	}
+	mtx_unlock(&dp->d_mtx);
+
 	sc = g_malloc(sizeof(*sc), M_WAITOK | M_ZERO);
 	mtx_init(&sc->start_mtx, "g_disk_start", NULL, MTX_DEF);
 	mtx_init(&sc->done_mtx, "g_disk_done", NULL, MTX_DEF);
@@ -704,6 +720,21 @@ g_disk_create(void *arg, int flag)
 	pp->private = sc;
 	dp->d_geom = gp;
 	g_error_provider(pp, 0);
+
+	mtx_lock(&dp->d_mtx);
+	dp->d_init_level = DISK_INIT_DONE;
+
+	/*
+	 * If the disk has gone away at this stage, start the withering
+	 * process for it.
+	 */
+	if (dp->d_goneflag != 0) {
+		mtx_unlock(&dp->d_mtx);
+		g_wither_provider(pp, ENXIO);
+		return;
+	}
+	mtx_unlock(&dp->d_mtx);
+
 }
 
 /*
@@ -754,6 +785,9 @@ g_disk_destroy(void *ptr, int flag)
 		dp->d_geom = NULL;
 		g_wither_geom(gp, ENXIO);
 	}
+
+	mtx_destroy(&dp->d_mtx);
+
 	g_free(dp);
 }
 
@@ -817,6 +851,12 @@ disk_create(struct disk *dp, int version
 		    dp->d_sectorsize, DEVSTAT_ALL_SUPPORTED,
 		    DEVSTAT_TYPE_DIRECT, DEVSTAT_PRIORITY_MAX);
 	dp->d_geom = NULL;
+
+	snprintf(dp->d_mtx_name, sizeof(dp->d_mtx_name), "%s%ddlk",
+		 dp->d_name, dp->d_unit);
+	mtx_init(&dp->d_mtx, dp->d_mtx_name, NULL, MTX_DEF);
+	dp->d_init_level = DISK_INIT_NONE;
+
 	g_disk_ident_adjust(dp->d_ident, sizeof(dp->d_ident));
 	g_post_event(g_disk_create, dp, M_WAITOK, dp, NULL);
 }
@@ -838,6 +878,30 @@ disk_gone(struct disk *dp)
 	struct g_geom *gp;
 	struct g_provider *pp;
 
+	mtx_lock(&dp->d_mtx);
+	dp->d_goneflag = 1;
+
+	/*
+	 * If we're still in the process of creating this disk (the
+	 * g_disk_create() function is still queued, or is in
+	 * progress), the init level will not yet be DISK_INIT_DONE.
+	 *
+	 * If that is the case, g_disk_create() will see d_goneflag
+	 * and take care of cleaning things up.
+	 *
+	 * If the disk has already been created, we default to
+	 * withering the provider as usual below.
+	 *
+	 * If the caller has not set a d_gone() callback, he will
+	 * not be any worse off by returning here, because the geom
+	 * has not been fully setup in any case.
+	 */
+	if (dp->d_init_level < DISK_INIT_DONE) {
+		mtx_unlock(&dp->d_mtx);
+		return;
+	}
+	mtx_unlock(&dp->d_mtx);
+
 	gp = dp->d_geom;
 	if (gp != NULL) {
 		pp = LIST_FIRST(&gp->provider);

Modified: head/sys/geom/geom_disk.h
==============================================================================
--- head/sys/geom/geom_disk.h	Tue Jun 21 20:15:30 2016	(r302068)
+++ head/sys/geom/geom_disk.h	Tue Jun 21 20:18:19 2016	(r302069)
@@ -60,11 +60,21 @@ typedef	int	disk_ioctl_t(struct disk *, 
 struct g_geom;
 struct devstat;
 
+typedef enum {
+	DISK_INIT_NONE,
+	DISK_INIT_START,
+	DISK_INIT_DONE
+} disk_init_level;
+
 struct disk {
 	/* Fields which are private to geom_disk */
 	struct g_geom		*d_geom;
 	struct devstat		*d_devstat;
+	int			d_goneflag;
 	int			d_destroyed;
+	struct mtx		d_mtx;
+	char			d_mtx_name[24];
+	disk_init_level		d_init_level;
 
 	/* Shared fields */
 	u_int			d_flags;
@@ -125,7 +135,8 @@ int disk_resize(struct disk *dp, int fla
 #define DISK_VERSION_02		0x5856105b
 #define DISK_VERSION_03		0x5856105c
 #define DISK_VERSION_04		0x5856105d
-#define DISK_VERSION		DISK_VERSION_04
+#define DISK_VERSION_05		0x5856105e
+#define DISK_VERSION		DISK_VERSION_05
 
 #endif /* _KERNEL */
 #endif /* _GEOM_GEOM_DISK_H_ */

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