From 5376d97a97f0654a22509676652f1dd1e0b27370 Mon Sep 17 00:00:00 2001 From: phk Date: Mon, 19 Sep 2005 06:55:27 +0000 Subject: Fix configuration locking in MD. Remove md_mtx. Remove GIANT from the mdctl device driver and avoid DROP_GIANT, PICKUP_GIANT and geom events since we can call into GEOM directly now. Pick up Giant around vn_close(). Apply an exclusive sx around mdctls ioctl and preloading to protect lists etc.. Don't initialize our lock (md_mtx or md_sx) from a SYSINIT when there is a perfectly good pair of _fini/_init functions to do it from. Prune any final fractional sector from the mediasize to keep GEOM happy. Cleanups: Unify MDIOVERSION check in (x)mdctlioctl() Add pointer to start() routine to softc to eliminate a switch{} Inline guts of mddetach(). Always pass error pointer to mdnew(), simplify implementation. --- sys/dev/md/md.c | 196 +++++++++++++++++++++++--------------------------------- 1 file changed, 80 insertions(+), 116 deletions(-) (limited to 'sys/dev/md') diff --git a/sys/dev/md/md.c b/sys/dev/md/md.c index 230f93b..6c40191 100644 --- a/sys/dev/md/md.c +++ b/sys/dev/md/md.c @@ -68,6 +68,7 @@ #include #include #include +#include #include #include #include @@ -112,14 +113,12 @@ static g_access_t g_md_access; static int mdunits; static struct cdev *status_dev = 0; -static struct mtx md_mtx; -MTX_SYSINIT(md_mtx, &md_mtx, "md", MTX_DEF); +static struct sx md_sx; static d_ioctl_t mdctlioctl; static struct cdevsw mdctl_cdevsw = { .d_version = D_VERSION, - .d_flags = D_NEEDGIANT, .d_ioctl = mdctlioctl, .d_name = MD_NAME, }; @@ -166,6 +165,7 @@ struct md_s { struct proc *procp; struct g_geom *gp; struct g_provider *pp; + int (*start)(struct md_s *sc, struct bio *bp); /* MD_MALLOC related fields */ struct indir *indir; @@ -184,8 +184,6 @@ struct md_s { vm_object_t object; }; -static int mddestroy(struct md_s *sc, struct thread *td); - static struct indir * new_indir(u_int shift) { @@ -385,8 +383,6 @@ g_md_start(struct bio *bp) wakeup(sc); } - - static int mdstart_malloc(struct md_s *sc, struct bio *bp) { @@ -481,6 +477,7 @@ mdstart_vnode(struct md_s *sc, struct bio *bp) struct iovec aiov; struct mount *mp; + mtx_assert(&Giant, MA_OWNED); /* * VNODE I/O * @@ -652,17 +649,16 @@ md_kthread(void *arg) } for (;;) { + if (sc->flags & MD_SHUTDOWN) { + sc->procp = NULL; + wakeup(&sc->procp); + if (hasgiant) + mtx_unlock(&Giant); + kthread_exit(0); + } mtx_lock(&sc->queue_mtx); bp = bioq_takefirst(&sc->bio_queue); if (!bp) { - if (sc->flags & MD_SHUTDOWN) { - mtx_unlock(&sc->queue_mtx); - sc->procp = NULL; - wakeup(&sc->procp); - if (hasgiant) - mtx_unlock(&Giant); - kthread_exit(0); - } msleep(sc, &sc->queue_mtx, PRIBIO | PDROP, "mdwait", 0); continue; } @@ -677,25 +673,7 @@ md_kthread(void *arg) else error = EOPNOTSUPP; } else { - switch (sc->type) { - case MD_MALLOC: - error = mdstart_malloc(sc, bp); - break; - case MD_PRELOAD: - error = mdstart_preload(sc, bp); - break; - case MD_VNODE: - mtx_assert(&Giant, MA_OWNED); - error = mdstart_vnode(sc, bp); - mtx_assert(&Giant, MA_OWNED); - break; - case MD_SWAP: - error = mdstart_swap(sc, bp); - break; - default: - panic("Impossible md(type)"); - break; - } + error = sc->start(sc, bp); } if (error != -1) { @@ -710,12 +688,10 @@ mdfind(int unit) { struct md_s *sc; - mtx_lock(&md_mtx); LIST_FOREACH(sc, &md_softc_list, list) { if (sc->unit == unit) break; } - mtx_unlock(&md_mtx); return (sc); } @@ -725,40 +701,31 @@ mdnew(int unit, int *errp) struct md_s *sc, *sc2; int error, max = -1; - sc = (struct md_s *)malloc(sizeof *sc, M_MD, M_WAITOK | M_ZERO); - bioq_init(&sc->bio_queue); - mtx_init(&sc->queue_mtx, "md bio queue", NULL, MTX_DEF); - mtx_lock(&md_mtx); + *errp = 0; LIST_FOREACH(sc2, &md_softc_list, list) { - if (sc2->unit == unit) { - mtx_unlock(&md_mtx); - mtx_destroy(&sc->queue_mtx); - free(sc, M_MD); - if (errp != NULL) - *errp = EBUSY; + if (unit == sc2->unit) { + *errp = EBUSY; return (NULL); } - if (sc2->unit > max) + if (unit == -1 && sc2->unit > max) max = sc2->unit; } if (unit == -1) unit = max + 1; + sc = (struct md_s *)malloc(sizeof *sc, M_MD, M_WAITOK | M_ZERO); + bioq_init(&sc->bio_queue); + mtx_init(&sc->queue_mtx, "md bio queue", NULL, MTX_DEF); sc->unit = unit; sprintf(sc->name, "md%d", unit); LIST_INSERT_HEAD(&md_softc_list, sc, list); - mtx_unlock(&md_mtx); error = kthread_create(md_kthread, sc, &sc->procp, 0, 0,"%s", sc->name); - if (error != 0) { - mtx_lock(&md_mtx); - LIST_REMOVE(sc, list); - mtx_unlock(&md_mtx); - mtx_destroy(&sc->queue_mtx); - free(sc, M_MD); - if (errp != NULL) - *errp = error; - return (NULL); - } - return (sc); + if (error == 0) + return (sc); + LIST_REMOVE(sc, list); + mtx_destroy(&sc->queue_mtx); + free(sc, M_MD); + *errp = error; + return (NULL); } static void @@ -768,7 +735,6 @@ mdinit(struct md_s *sc) struct g_geom *gp; struct g_provider *pp; - DROP_GIANT(); g_topology_lock(); gp = g_new_geomf(&g_md_class, "md%d", sc->unit); gp->softc = sc; @@ -779,7 +745,6 @@ mdinit(struct md_s *sc) sc->pp = pp; g_error_provider(pp, 0); g_topology_unlock(); - PICKUP_GIANT(); } /* @@ -930,23 +895,16 @@ mdcreate_vnode(struct md_s *sc, struct md_ioctl *mdio, struct thread *td) return (0); } -static void -md_zapit(void *p, int cancel) -{ - if (cancel) - return; - g_wither_geom(p, ENXIO); -} - static int mddestroy(struct md_s *sc, struct thread *td) { - GIANT_REQUIRED; if (sc->gp) { sc->gp->softc = NULL; - g_waitfor_event(md_zapit, sc->gp, M_WAITOK, sc->gp, NULL); + g_topology_lock(); + g_wither_geom(sc->gp, ENXIO); + g_topology_unlock(); sc->gp = NULL; sc->pp = NULL; } @@ -955,9 +913,12 @@ mddestroy(struct md_s *sc, struct thread *td) while (sc->procp != NULL) tsleep(&sc->procp, PRIBIO, "mddestroy", hz / 10); mtx_destroy(&sc->queue_mtx); - if (sc->vnode != NULL) + if (sc->vnode != NULL) { + mtx_lock(&Giant); (void)vn_close(sc->vnode, sc->flags & MD_READONLY ? FREAD : (FREAD|FWRITE), sc->cred, td); + mtx_unlock(&Giant); + } if (sc->cred != NULL) crfree(sc->cred); if (sc->object != NULL) @@ -967,9 +928,7 @@ mddestroy(struct md_s *sc, struct thread *td) if (sc->uma) uma_zdestroy(sc->uma); - mtx_lock(&md_mtx); LIST_REMOVE(sc, list); - mtx_unlock(&md_mtx); free(sc, M_MD); return (0); } @@ -980,8 +939,6 @@ mdcreate_swap(struct md_s *sc, struct md_ioctl *mdio, struct thread *td) vm_ooffset_t npage; int error; - GIANT_REQUIRED; - /* * Range check. Disallow negative sizes or any size less then the * size of a page. Then round to a page. @@ -1020,29 +977,9 @@ mdcreate_swap(struct md_s *sc, struct md_ioctl *mdio, struct thread *td) return (error); } -static int -mddetach(int unit, struct thread *td) -{ - struct md_s *sc; - - sc = mdfind(unit); - if (sc == NULL) - return (ENOENT); - if (sc->opencount != 0 && !(sc->flags & MD_FORCE)) - return (EBUSY); - switch(sc->type) { - case MD_VNODE: - case MD_SWAP: - case MD_MALLOC: - case MD_PRELOAD: - return (mddestroy(sc, td)); - default: - return (EOPNOTSUPP); - } -} static int -mdctlioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td) +xmdctlioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td) { struct md_ioctl *mdio; struct md_s *sc; @@ -1052,6 +989,10 @@ mdctlioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread printf("mdctlioctl(%s %lx %p %x %p)\n", devtoname(dev), cmd, addr, flags, td); + mdio = (struct md_ioctl *)addr; + if (mdio->md_version != MDIOVERSION) + return (EINVAL); + /* * We assert the version number in the individual ioctl * handlers instead of out here because (a) it is possible we @@ -1059,11 +1000,9 @@ mdctlioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread * mdio, and (b) the correct return value for an unknown ioctl * is ENOIOCTL, not EINVAL. */ - mdio = (struct md_ioctl *)addr; + error = 0; switch (cmd) { case MDIOCATTACH: - if (mdio->md_version != MDIOVERSION) - return (EINVAL); switch (mdio->md_type) { case MD_MALLOC: case MD_PRELOAD: @@ -1090,15 +1029,19 @@ mdctlioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread error = EDOOFUS; switch (sc->type) { case MD_MALLOC: + sc->start = mdstart_malloc; error = mdcreate_malloc(sc, mdio); break; case MD_PRELOAD: + sc->start = mdstart_preload; error = mdcreate_preload(sc, mdio); break; case MD_VNODE: + sc->start = mdstart_vnode; error = mdcreate_vnode(sc, mdio, td); break; case MD_SWAP: + sc->start = mdstart_swap; error = mdcreate_swap(sc, mdio, td); break; } @@ -1106,17 +1049,24 @@ mdctlioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread mddestroy(sc, td); return (error); } + + /* Prune off any residual fractional sector */ + i = sc->mediasize % sc->sectorsize; + sc->mediasize -= i; + mdinit(sc); return (0); case MDIOCDETACH: - if (mdio->md_version != MDIOVERSION) - return (EINVAL); if (mdio->md_mediasize != 0 || mdio->md_options != 0) return (EINVAL); - return (mddetach(mdio->md_unit, td)); + + sc = mdfind(mdio->md_unit); + if (sc == NULL) + return (ENOENT); + if (sc->opencount != 0 && !(sc->flags & MD_FORCE)) + return (EBUSY); + return (mddestroy(sc, td)); case MDIOCQUERY: - if (mdio->md_version != MDIOVERSION) - return (EINVAL); sc = mdfind(mdio->md_unit); if (sc == NULL) return (ENOENT); @@ -1124,16 +1074,11 @@ mdctlioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread mdio->md_options = sc->flags; mdio->md_mediasize = sc->mediasize; mdio->md_sectorsize = sc->sectorsize; - if (sc->type == MD_VNODE) { + if (sc->type == MD_VNODE) error = copyout(sc->file, mdio->md_file, strlen(sc->file) + 1); - if (error != 0) - return (error); - } - return (0); + return (error); case MDIOCLIST: - if (mdio->md_version != MDIOVERSION) - return (EINVAL); i = 1; LIST_FOREACH(sc, &md_softc_list, list) { if (i == MDNPAD - 1) @@ -1146,15 +1091,26 @@ mdctlioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread default: return (ENOIOCTL); }; - return (ENOIOCTL); +} + +static int +mdctlioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td) +{ + int error; + + sx_xlock(&md_sx); + error = xmdctlioctl(dev, cmd, addr, flags, td); + sx_xunlock(&md_sx); + return (error); } static void md_preloaded(u_char *image, size_t length) { struct md_s *sc; + int error; - sc = mdnew(-1, NULL); + sc = mdnew(-1, &error); if (sc == NULL) return; sc->type = MD_PRELOAD; @@ -1162,6 +1118,7 @@ md_preloaded(u_char *image, size_t length) sc->sectorsize = DEV_BSIZE; sc->pl_ptr = image; sc->pl_len = length; + sc->start = mdstart_preload; #ifdef MD_ROOT if (sc->unit == 0) rootdevnames[0] = "ufs:/dev/md0"; @@ -1179,15 +1136,19 @@ g_md_init(struct g_class *mp __unused) unsigned len; mod = NULL; + sx_init(&md_sx, "MD config lock"); g_topology_unlock(); #ifdef MD_ROOT_SIZE - md_preloaded(mfs_root, MD_ROOT_SIZE*1024); + sx_xlock(&md_sx); + md_preloaded(mfs_root, MD_ROOT_SIZE * 1024); + sx_xunlock(&md_sx); #endif + /* XXX: are preload_* static or do they need Giant ? */ while ((mod = preload_search_next_name(mod)) != NULL) { name = (char *)preload_search_info(mod, MODINFO_NAME); - type = (char *)preload_search_info(mod, MODINFO_TYPE); if (name == NULL) continue; + type = (char *)preload_search_info(mod, MODINFO_TYPE); if (type == NULL) continue; if (strcmp(type, "md_image") && strcmp(type, "mfs_root")) @@ -1198,7 +1159,9 @@ g_md_init(struct g_class *mp __unused) len = *(size_t *)c; printf("%s%d: Preloaded image <%s> %d bytes at %p\n", MD_NAME, mdunits, name, len, ptr); + sx_xlock(&md_sx); md_preloaded(ptr, len); + sx_xunlock(&md_sx); } status_dev = make_dev(&mdctl_cdevsw, MAXMINOR, UID_ROOT, GID_WHEEL, 0600, MDCTL_NAME); @@ -1209,6 +1172,7 @@ static void g_md_fini(struct g_class *mp __unused) { + sx_destroy(&md_sx); if (status_dev != NULL) destroy_dev(status_dev); } -- cgit v1.1