diff options
author | Konstantin Belousov <kib@FreeBSD.org> | 2007-07-03 17:42:37 +0000 |
---|---|---|
committer | Konstantin Belousov <kib@FreeBSD.org> | 2007-07-03 17:42:37 +0000 |
commit | de10ffa527504dff12f2ba6e4b345dc719f71e6e (patch) | |
tree | cb3718e3a301e214d2cb4e0f64469a55da81c6f6 | |
parent | 7aee5992a5f546d5610dd7c80c22eef7702a83f9 (diff) | |
download | src-de10ffa527504dff12f2ba6e4b345dc719f71e6e.tar.gz src-de10ffa527504dff12f2ba6e4b345dc719f71e6e.zip |
Since rev. 1.199 of sys/kern/kern_conf.c, the thread that calls
destroy_dev() from d_close() cdev method would self-deadlock.
devfs_close() bump device thread reference counter, and destroy_dev()
sleeps, waiting for si_threadcount to reach zero for cdev without
d_purge method.
destroy_dev_sched() could be used instead from d_close(), to
schedule execution of destroy_dev() in another context. The
destroy_dev_sched_drain() function can be used to drain the scheduled
calls to destroy_dev_sched(). Similarly, drain_dev_clone_events() drains
the events clone to make sure no lingering devices are left after
dev_clone event handler deregistered.
make_dev_credf(MAKEDEV_REF) function should be used from dev_clone
event handlers instead of make_dev()/make_dev_cred() to ensure that created
device has reference counter bumped before cdev mutex is dropped inside
make_dev().
Reviewed by: tegge (early versions), njl (programming interface)
Debugging help and testing by: Peter Holm
Approved by: re (kensmith)
Notes
Notes:
svn path=/head/; revision=171181
-rw-r--r-- | sys/fs/devfs/devfs_int.h | 6 | ||||
-rw-r--r-- | sys/fs/devfs/devfs_vnops.c | 13 | ||||
-rw-r--r-- | sys/kern/kern_conf.c | 165 | ||||
-rw-r--r-- | sys/sys/conf.h | 10 |
4 files changed, 176 insertions, 18 deletions
diff --git a/sys/fs/devfs/devfs_int.h b/sys/fs/devfs/devfs_int.h index 2bd7f99da73a..3848ab4c1904 100644 --- a/sys/fs/devfs/devfs_int.h +++ b/sys/fs/devfs/devfs_int.h @@ -47,11 +47,16 @@ struct cdev_priv { u_int cdp_flags; #define CDP_ACTIVE (1 << 0) +#define CDP_SCHED_DTR (1 << 1) u_int cdp_inuse; u_int cdp_maxdirent; struct devfs_dirent **cdp_dirents; struct devfs_dirent *cdp_dirent0; + + TAILQ_ENTRY(cdev_priv) cdp_dtr_list; + void (*cdp_dtr_cb)(void *); + void *cdp_dtr_cb_arg; }; struct cdev *devfs_alloc(void); @@ -62,6 +67,7 @@ void devfs_destroy(struct cdev *dev); extern struct unrhdr *devfs_inos; extern struct mtx devmtx; extern struct mtx devfs_de_interlock; +extern struct sx clone_drain_lock; extern TAILQ_HEAD(cdev_priv_list, cdev_priv) cdevp_list; #endif /* _KERNEL */ diff --git a/sys/fs/devfs/devfs_vnops.c b/sys/fs/devfs/devfs_vnops.c index 0acf99bfdcf6..fcf3b3a42f58 100644 --- a/sys/fs/devfs/devfs_vnops.c +++ b/sys/fs/devfs/devfs_vnops.c @@ -75,6 +75,8 @@ static struct fileops devfs_ops_f; struct mtx devfs_de_interlock; MTX_SYSINIT(devfs_de_interlock, &devfs_de_interlock, "devfs interlock", MTX_DEF); +struct sx clone_drain_lock; +SX_SYSINIT(clone_drain_lock, &clone_drain_lock, "clone events drain lock"); static int devfs_fp_check(struct file *fp, struct cdev **devp, struct cdevsw **dswp) @@ -618,8 +620,19 @@ devfs_lookupx(struct vop_lookup_args *ap, int *dm_unlock) break; cdev = NULL; + DEVFS_DMP_HOLD(dmp); + sx_xunlock(&dmp->dm_lock); + sx_slock(&clone_drain_lock); EVENTHANDLER_INVOKE(dev_clone, td->td_ucred, pname, strlen(pname), &cdev); + sx_sunlock(&clone_drain_lock); + sx_xlock(&dmp->dm_lock); + if (DEVFS_DMP_DROP(dmp)) { + *dm_unlock = 0; + sx_xunlock(&dmp->dm_lock); + devfs_unmount_final(dmp); + return (ENOENT); + } if (cdev == NULL) break; diff --git a/sys/kern/kern_conf.c b/sys/kern/kern_conf.c index 2241575418c6..beacfff3cc9a 100644 --- a/sys/kern/kern_conf.c +++ b/sys/kern/kern_conf.c @@ -39,9 +39,11 @@ __FBSDID("$FreeBSD$"); #include <sys/vnode.h> #include <sys/queue.h> #include <sys/poll.h> +#include <sys/sx.h> #include <sys/ctype.h> #include <sys/tty.h> #include <sys/ucred.h> +#include <sys/taskqueue.h> #include <machine/stdarg.h> #include <fs/devfs/devfs_int.h> @@ -50,9 +52,11 @@ static MALLOC_DEFINE(M_DEVT, "cdev", "cdev storage"); struct mtx devmtx; static void destroy_devl(struct cdev *dev); -static struct cdev *make_dev_credv(struct cdevsw *devsw, int minornr, - struct ucred *cr, uid_t uid, gid_t gid, int mode, const char *fmt, - va_list ap); + +static struct cdev *make_dev_credv(int flags, + struct cdevsw *devsw, int minornr, + struct ucred *cr, uid_t uid, gid_t gid, int mode, const char *fmt, + va_list ap); static struct cdev_priv_list cdevp_free_list = TAILQ_HEAD_INITIALIZER(cdevp_free_list); @@ -143,12 +147,18 @@ struct cdevsw * dev_refthread(struct cdev *dev) { struct cdevsw *csw; + struct cdev_priv *cdp; mtx_assert(&devmtx, MA_NOTOWNED); dev_lock(); csw = dev->si_devsw; - if (csw != NULL) - dev->si_threadcount++; + if (csw != NULL) { + cdp = dev->si_priv; + if ((cdp->cdp_flags & CDP_SCHED_DTR) == 0) + dev->si_threadcount++; + else + csw = NULL; + } dev_unlock(); return (csw); } @@ -157,15 +167,19 @@ struct cdevsw * devvn_refthread(struct vnode *vp, struct cdev **devp) { struct cdevsw *csw; + struct cdev_priv *cdp; mtx_assert(&devmtx, MA_NOTOWNED); csw = NULL; dev_lock(); *devp = vp->v_rdev; if (*devp != NULL) { - csw = (*devp)->si_devsw; - if (csw != NULL) - (*devp)->si_threadcount++; + cdp = (*devp)->si_priv; + if ((cdp->cdp_flags & CDP_SCHED_DTR) == 0) { + csw = (*devp)->si_devsw; + if (csw != NULL) + (*devp)->si_threadcount++; + } } dev_unlock(); return (csw); @@ -554,8 +568,9 @@ prep_cdevsw(struct cdevsw *devsw) dev_unlock(); } -static struct cdev * -make_dev_credv(struct cdevsw *devsw, int minornr, struct ucred *cr, uid_t uid, +struct cdev * +make_dev_credv(int flags, struct cdevsw *devsw, int minornr, + struct ucred *cr, uid_t uid, gid_t gid, int mode, const char *fmt, va_list ap) { struct cdev *dev; @@ -569,6 +584,8 @@ make_dev_credv(struct cdevsw *devsw, int minornr, struct ucred *cr, uid_t uid, dev = devfs_alloc(); dev_lock(); dev = newdev(devsw, minornr, dev); + if (flags & MAKEDEV_REF) + dev_refl(dev); if (dev->si_flags & SI_CHEAPCLONE && dev->si_flags & SI_NAMED) { /* @@ -611,7 +628,7 @@ make_dev(struct cdevsw *devsw, int minornr, uid_t uid, gid_t gid, int mode, va_list ap; va_start(ap, fmt); - dev = make_dev_credv(devsw, minornr, NULL, uid, gid, mode, fmt, ap); + dev = make_dev_credv(0, devsw, minornr, NULL, uid, gid, mode, fmt, ap); va_end(ap); return (dev); } @@ -624,7 +641,23 @@ make_dev_cred(struct cdevsw *devsw, int minornr, struct ucred *cr, uid_t uid, va_list ap; va_start(ap, fmt); - dev = make_dev_credv(devsw, minornr, cr, uid, gid, mode, fmt, ap); + dev = make_dev_credv(0, devsw, minornr, cr, uid, gid, mode, fmt, ap); + va_end(ap); + + return (dev); +} + +struct cdev * +make_dev_credf(int flags, struct cdevsw *devsw, int minornr, + struct ucred *cr, uid_t uid, + gid_t gid, int mode, const char *fmt, ...) +{ + struct cdev *dev; + va_list ap; + + va_start(ap, fmt); + dev = make_dev_credv(flags, devsw, minornr, cr, uid, gid, mode, + fmt, ap); va_end(ap); return (dev); @@ -728,8 +761,10 @@ destroy_devl(struct cdev *dev) LIST_REMOVE(dev, si_list); /* If cdevsw has no more struct cdev *'s, clean it */ - if (LIST_EMPTY(&csw->d_devs)) + if (LIST_EMPTY(&csw->d_devs)) { fini_cdevsw(csw); + wakeup(&csw->d_devs); + } } dev->si_flags &= ~SI_ALIAS; dev->si_refcount--; /* Avoid race with dev_rel() */ @@ -915,21 +950,115 @@ clone_create(struct clonedevs **cdp, struct cdevsw *csw, int *up, struct cdev ** void clone_cleanup(struct clonedevs **cdp) { - struct cdev *dev, *tdev; + struct cdev *dev; + struct cdev_priv *cp; struct clonedevs *cd; cd = *cdp; if (cd == NULL) return; dev_lock(); - LIST_FOREACH_SAFE(dev, &cd->head, si_clone, tdev) { + while (!LIST_EMPTY(&cd->head)) { + dev = LIST_FIRST(&cd->head); + LIST_REMOVE(dev, si_clone); KASSERT(dev->si_flags & SI_CLONELIST, ("Dev %p(%s) should be on clonelist", dev, dev->si_name)); - KASSERT(dev->si_flags & SI_NAMED, - ("Driver has goofed in cloning underways udev %x", dev->si_drv0)); - destroy_devl(dev); + dev->si_flags &= ~SI_CLONELIST; + cp = dev->si_priv; + if (!(cp->cdp_flags & CDP_SCHED_DTR)) { + cp->cdp_flags |= CDP_SCHED_DTR; + KASSERT(dev->si_flags & SI_NAMED, + ("Driver has goofed in cloning underways udev %x", dev->si_drv0)); + destroy_devl(dev); + } } dev_unlock(); free(cd, M_DEVBUF); *cdp = NULL; } + +static TAILQ_HEAD(, cdev_priv) dev_ddtr = + TAILQ_HEAD_INITIALIZER(dev_ddtr); +static struct task dev_dtr_task; + +static void +destroy_dev_tq(void *ctx, int pending) +{ + struct cdev_priv *cp; + struct cdev *dev; + void (*cb)(void *); + void *cb_arg; + + dev_lock(); + while (!TAILQ_EMPTY(&dev_ddtr)) { + cp = TAILQ_FIRST(&dev_ddtr); + dev = &cp->cdp_c; + KASSERT(cp->cdp_flags & CDP_SCHED_DTR, + ("cdev %p in dev_destroy_tq without CDP_SCHED_DTR", cp)); + TAILQ_REMOVE(&dev_ddtr, cp, cdp_dtr_list); + cb = cp->cdp_dtr_cb; + cb_arg = cp->cdp_dtr_cb_arg; + destroy_devl(dev); + dev_unlock(); + dev_rel(dev); + if (cb != NULL) + cb(cb_arg); + dev_lock(); + } + dev_unlock(); +} + +int +destroy_dev_sched_cb(struct cdev *dev, void (*cb)(void *), void *arg) +{ + struct cdev_priv *cp; + + cp = dev->si_priv; + dev_lock(); + if (cp->cdp_flags & CDP_SCHED_DTR) { + dev_unlock(); + return (0); + } + dev_refl(dev); + cp->cdp_flags |= CDP_SCHED_DTR; + cp->cdp_dtr_cb = cb; + cp->cdp_dtr_cb_arg = arg; + TAILQ_INSERT_TAIL(&dev_ddtr, cp, cdp_dtr_list); + dev_unlock(); + taskqueue_enqueue(taskqueue_swi_giant, &dev_dtr_task); + return (1); +} + +int +destroy_dev_sched(struct cdev *dev) +{ + return (destroy_dev_sched_cb(dev, NULL, NULL)); +} + +void +destroy_dev_drain(struct cdevsw *csw) +{ + + dev_lock(); + while (!LIST_EMPTY(&csw->d_devs)) { + msleep(&csw->d_devs, &devmtx, PRIBIO, "devscd", hz/10); + } + dev_unlock(); +} + +void +drain_dev_clone_events(void) +{ + + sx_xlock(&clone_drain_lock); + sx_xunlock(&clone_drain_lock); +} + +static void +devdtr_init(void *dummy __unused) +{ + + TASK_INIT(&dev_dtr_task, 0, destroy_dev_tq, NULL); +} + +SYSINIT(devdtr, SI_SUB_DEVFS, SI_ORDER_SECOND, devdtr_init, NULL); diff --git a/sys/sys/conf.h b/sys/sys/conf.h index c4272dd9b904..7b1a615caead 100644 --- a/sys/sys/conf.h +++ b/sys/sys/conf.h @@ -245,6 +245,10 @@ int clone_create(struct clonedevs **, struct cdevsw *, int *unit, struct cdev ** int count_dev(struct cdev *_dev); void destroy_dev(struct cdev *_dev); +int destroy_dev_sched(struct cdev *dev); +int destroy_dev_sched_cb(struct cdev *dev, void (*cb)(void *), void *arg); +void destroy_dev_drain(struct cdevsw *csw); +void drain_dev_clone_events(void); struct cdevsw *dev_refthread(struct cdev *_dev); struct cdevsw *devvn_refthread(struct vnode *vp, struct cdev **devp); void dev_relthread(struct cdev *_dev); @@ -258,6 +262,12 @@ struct cdev *make_dev(struct cdevsw *_devsw, int _minor, uid_t _uid, gid_t _gid, struct cdev *make_dev_cred(struct cdevsw *_devsw, int _minor, struct ucred *_cr, uid_t _uid, gid_t _gid, int _perms, const char *_fmt, ...) __printflike(7, 8); +#define MAKEDEV_REF 0x1 +#define MAKEDEV_WHTOUT 0x2 +struct cdev *make_dev_credf(int _flags, + struct cdevsw *_devsw, int _minornr, + struct ucred *_cr, uid_t _uid, gid_t _gid, int _mode, + const char *_fmt, ...) __printflike(8, 9); struct cdev *make_dev_alias(struct cdev *_pdev, const char *_fmt, ...) __printflike(2, 3); int dev2unit(struct cdev *_dev); void dev_lock(void); |