Date: Thu, 19 May 2016 13:56:34 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: geom@freebsd.org Cc: arch@freebsd.org Subject: Removing Giant asserts from geom Message-ID: <20160519105634.GO89104@kib.kiev.ua>
next in thread | raw e-mail | index | archive | help
I propose to remove asserts
mtx_assert(&Giant, MA_NOTOWNED);
from the geom KPI. The asserts do not serve any purposes, at least not
in the current kernel. They might provided some agenda in the 5.x days,
but now they only force filesystems to add cruft to satisfy GEOM KPI
requirements.
As an example, consider DROP_GIANT/PICKUP_GIANT in the ffs code. VFS is
currently Giant-free, including the mount and unmount path, UFS/FFS are
also Giant-free, but mount and unmount paths have to do the Giant dance
to allow root mount to proceed. This is because startup is executed with
the Giant owned by the thread0.
Giant asserts are even more ridiculous in the geom code since geom cdev
geom.ctl is marked as needing Giant. And there are vestiges of Giant
around calls to add geom kthreads, which is not need by KPI. Not to note
that Giant is already held there due to startup protocol.
Any objections ?
diff --git a/sys/geom/eli/g_eli.c b/sys/geom/eli/g_eli.c
index 403d0b6..422883d 100644
--- a/sys/geom/eli/g_eli.c
+++ b/sys/geom/eli/g_eli.c
@@ -1229,7 +1229,6 @@ g_eli_shutdown_pre_sync(void *arg, int howto)
int error;
mp = arg;
- DROP_GIANT();
g_topology_lock();
LIST_FOREACH_SAFE(gp, &mp->geom, geom, gp2) {
sc = gp->softc;
@@ -1245,7 +1244,6 @@ g_eli_shutdown_pre_sync(void *arg, int howto)
}
}
g_topology_unlock();
- PICKUP_GIANT();
}
static void
diff --git a/sys/geom/geom.h b/sys/geom/geom.h
index bf70d0b..7c1d714 100644
--- a/sys/geom/geom.h
+++ b/sys/geom/geom.h
@@ -369,7 +369,6 @@ g_free(void *ptr)
#define g_topology_lock() \
do { \
- mtx_assert(&Giant, MA_NOTOWNED); \
sx_xlock(&topology_lock); \
} while (0)
diff --git a/sys/geom/geom_event.c b/sys/geom/geom_event.c
index 2ded638..3c2ee49 100644
--- a/sys/geom/geom_event.c
+++ b/sys/geom/geom_event.c
@@ -83,7 +83,6 @@ g_waitidle(void)
{
g_topology_assert_not();
- mtx_assert(&Giant, MA_NOTOWNED);
mtx_lock(&g_eventlock);
while (!TAILQ_EMPTY(&g_events))
diff --git a/sys/geom/geom_kern.c b/sys/geom/geom_kern.c
index dbced0f..9f3f120 100644
--- a/sys/geom/geom_kern.c
+++ b/sys/geom/geom_kern.c
@@ -90,7 +90,6 @@ static void
g_up_procbody(void *arg)
{
- mtx_assert(&Giant, MA_NOTOWNED);
thread_lock(g_up_td);
sched_prio(g_up_td, PRIBIO);
thread_unlock(g_up_td);
@@ -103,7 +102,6 @@ static void
g_down_procbody(void *arg)
{
- mtx_assert(&Giant, MA_NOTOWNED);
thread_lock(g_down_td);
sched_prio(g_down_td, PRIBIO);
thread_unlock(g_down_td);
@@ -116,7 +114,6 @@ static void
g_event_procbody(void *arg)
{
- mtx_assert(&Giant, MA_NOTOWNED);
thread_lock(g_event_td);
sched_prio(g_event_td, PRIBIO);
thread_unlock(g_event_td);
@@ -147,14 +144,12 @@ g_init(void)
g_io_init();
g_event_init();
g_ctl_init();
- mtx_lock(&Giant);
kproc_kthread_add(g_event_procbody, NULL, &g_proc, &g_event_td,
RFHIGHPID, 0, "geom", "g_event");
kproc_kthread_add(g_up_procbody, NULL, &g_proc, &g_up_td,
RFHIGHPID, 0, "geom", "g_up");
kproc_kthread_add(g_down_procbody, NULL, &g_proc, &g_down_td,
RFHIGHPID, 0, "geom", "g_down");
- mtx_unlock(&Giant);
EVENTHANDLER_REGISTER(shutdown_pre_sync, geom_shutdown, NULL,
SHUTDOWN_PRI_FIRST);
}
diff --git a/sys/geom/geom_mbr.c b/sys/geom/geom_mbr.c
index 86ee860..a811e35 100644
--- a/sys/geom/geom_mbr.c
+++ b/sys/geom/geom_mbr.c
@@ -190,7 +190,6 @@ g_mbr_ioctl(struct g_provider *pp, u_long cmd, void *data, int fflag, struct thr
case DIOCSMBR: {
if (!(fflag & FWRITE))
return (EPERM);
- DROP_GIANT();
g_topology_lock();
cp = LIST_FIRST(&gp->consumer);
if (cp->acw == 0) {
@@ -205,7 +204,6 @@ g_mbr_ioctl(struct g_provider *pp, u_long cmd, void *data, int fflag, struct thr
if (opened)
g_access(cp, 0, -1 , 0);
g_topology_unlock();
- PICKUP_GIANT();
return(error);
}
default:
diff --git a/sys/geom/geom_pc98.c b/sys/geom/geom_pc98.c
index 42c9962..f4435cb 100644
--- a/sys/geom/geom_pc98.c
+++ b/sys/geom/geom_pc98.c
@@ -176,7 +176,6 @@ g_pc98_ioctl(struct g_provider *pp, u_long cmd, void *data, int fflag, struct th
case DIOCSPC98: {
if (!(fflag & FWRITE))
return (EPERM);
- DROP_GIANT();
g_topology_lock();
cp = LIST_FIRST(&gp->consumer);
if (cp->acw == 0) {
@@ -191,7 +190,6 @@ g_pc98_ioctl(struct g_provider *pp, u_long cmd, void *data, int fflag, struct th
if (opened)
g_access(cp, 0, -1 , 0);
g_topology_unlock();
- PICKUP_GIANT();
return(error);
}
default:
diff --git a/sys/geom/geom_subr.c b/sys/geom/geom_subr.c
index 54a99bf..8517991 100644
--- a/sys/geom/geom_subr.c
+++ b/sys/geom/geom_subr.c
@@ -247,9 +247,7 @@ g_modevent(module_t mod, int type, void *data)
break;
case MOD_UNLOAD:
g_trace(G_T_TOPOLOGY, "g_modevent(%s, UNLOAD)", mp->name);
- DROP_GIANT();
error = g_unload_class(mp);
- PICKUP_GIANT();
if (error == 0) {
KASSERT(LIST_EMPTY(&mp->geom),
("Unloaded class (%s) still has geom", mp->name));
diff --git a/sys/geom/journal/g_journal.c b/sys/geom/journal/g_journal.c
index 871bd8e4..0678003 100644
--- a/sys/geom/journal/g_journal.c
+++ b/sys/geom/journal/g_journal.c
@@ -2697,7 +2697,6 @@ g_journal_shutdown(void *arg, int howto __unused)
if (panicstr != NULL)
return;
mp = arg;
- DROP_GIANT();
g_topology_lock();
LIST_FOREACH_SAFE(gp, &mp->geom, geom, gp2) {
if (gp->softc == NULL)
@@ -2706,7 +2705,6 @@ g_journal_shutdown(void *arg, int howto __unused)
g_journal_destroy(gp->softc);
}
g_topology_unlock();
- PICKUP_GIANT();
}
/*
@@ -2725,7 +2723,6 @@ g_journal_lowmem(void *arg, int howto __unused)
g_journal_stats_low_mem++;
mp = arg;
- DROP_GIANT();
g_topology_lock();
LIST_FOREACH(gp, &mp->geom, geom) {
sc = gp->softc;
@@ -2756,7 +2753,6 @@ g_journal_lowmem(void *arg, int howto __unused)
break;
}
g_topology_unlock();
- PICKUP_GIANT();
}
static void g_journal_switcher(void *arg);
@@ -2871,7 +2867,6 @@ g_journal_do_switch(struct g_class *classp)
char *mountpoint;
int error, save;
- DROP_GIANT();
g_topology_lock();
LIST_FOREACH(gp, &classp->geom, geom) {
sc = gp->softc;
@@ -2886,7 +2881,6 @@ g_journal_do_switch(struct g_class *classp)
mtx_unlock(&sc->sc_mtx);
}
g_topology_unlock();
- PICKUP_GIANT();
mtx_lock(&mountlist_mtx);
TAILQ_FOREACH(mp, &mountlist, mnt_list) {
@@ -2901,11 +2895,9 @@ g_journal_do_switch(struct g_class *classp)
continue;
/* mtx_unlock(&mountlist_mtx) was done inside vfs_busy() */
- DROP_GIANT();
g_topology_lock();
sc = g_journal_find_device(classp, mp->mnt_gjprovider);
g_topology_unlock();
- PICKUP_GIANT();
if (sc == NULL) {
GJ_DEBUG(0, "Cannot find journal geom for %s.",
@@ -2984,7 +2976,6 @@ next:
sc = NULL;
for (;;) {
- DROP_GIANT();
g_topology_lock();
LIST_FOREACH(gp, &g_journal_class.geom, geom) {
sc = gp->softc;
@@ -3000,7 +2991,6 @@ next:
sc = NULL;
}
g_topology_unlock();
- PICKUP_GIANT();
if (sc == NULL)
break;
mtx_assert(&sc->sc_mtx, MA_OWNED);
diff --git a/sys/geom/mirror/g_mirror.c b/sys/geom/mirror/g_mirror.c
index 91f1367..379f615 100644
--- a/sys/geom/mirror/g_mirror.c
+++ b/sys/geom/mirror/g_mirror.c
@@ -3310,7 +3310,6 @@ g_mirror_shutdown_post_sync(void *arg, int howto)
int error;
mp = arg;
- DROP_GIANT();
g_topology_lock();
g_mirror_shutdown = 1;
LIST_FOREACH_SAFE(gp, &mp->geom, geom, gp2) {
@@ -3329,7 +3328,6 @@ g_mirror_shutdown_post_sync(void *arg, int howto)
g_topology_lock();
}
g_topology_unlock();
- PICKUP_GIANT();
}
static void
diff --git a/sys/geom/mountver/g_mountver.c b/sys/geom/mountver/g_mountver.c
index eafccc8..61375ef 100644
--- a/sys/geom/mountver/g_mountver.c
+++ b/sys/geom/mountver/g_mountver.c
@@ -611,12 +611,10 @@ g_mountver_shutdown_pre_sync(void *arg, int howto)
struct g_geom *gp, *gp2;
mp = arg;
- DROP_GIANT();
g_topology_lock();
LIST_FOREACH_SAFE(gp, &mp->geom, geom, gp2)
g_mountver_destroy(gp, 1);
g_topology_unlock();
- PICKUP_GIANT();
}
static void
diff --git a/sys/geom/raid/g_raid.c b/sys/geom/raid/g_raid.c
index 4885319..e590e35 100644
--- a/sys/geom/raid/g_raid.c
+++ b/sys/geom/raid/g_raid.c
@@ -2462,7 +2462,6 @@ g_raid_shutdown_post_sync(void *arg, int howto)
struct g_raid_volume *vol;
mp = arg;
- DROP_GIANT();
g_topology_lock();
g_raid_shutdown = 1;
LIST_FOREACH_SAFE(gp, &mp->geom, geom, gp2) {
@@ -2477,7 +2476,6 @@ g_raid_shutdown_post_sync(void *arg, int howto)
g_topology_lock();
}
g_topology_unlock();
- PICKUP_GIANT();
}
static void
diff --git a/sys/geom/raid3/g_raid3.c b/sys/geom/raid3/g_raid3.c
index a2ffe53..9b3c483 100644
--- a/sys/geom/raid3/g_raid3.c
+++ b/sys/geom/raid3/g_raid3.c
@@ -3543,7 +3543,6 @@ g_raid3_shutdown_post_sync(void *arg, int howto)
int error;
mp = arg;
- DROP_GIANT();
g_topology_lock();
g_raid3_shutdown = 1;
LIST_FOREACH_SAFE(gp, &mp->geom, geom, gp2) {
@@ -3562,7 +3561,6 @@ g_raid3_shutdown_post_sync(void *arg, int howto)
g_topology_lock();
}
g_topology_unlock();
- PICKUP_GIANT();
}
static void
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160519105634.GO89104>