From owner-svn-src-head@freebsd.org Tue Jun 21 20:18:20 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5F254AC5AE4; Tue, 21 Jun 2016 20:18:20 +0000 (UTC) (envelope-from ken@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 21C6F12BE; Tue, 21 Jun 2016 20:18:20 +0000 (UTC) (envelope-from ken@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u5LKIJcF034786; Tue, 21 Jun 2016 20:18:19 GMT (envelope-from ken@FreeBSD.org) Received: (from ken@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u5LKIJ7C034784; Tue, 21 Jun 2016 20:18:19 GMT (envelope-from ken@FreeBSD.org) Message-Id: <201606212018.u5LKIJ7C034784@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: ken set sender to ken@FreeBSD.org using -f From: "Kenneth D. Merry" Date: Tue, 21 Jun 2016 20:18:19 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r302069 - head/sys/geom X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Jun 2016 20:18:20 -0000 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_ */