Date: Mon, 4 Sep 2006 10:40:31 GMT From: "Alex Unleashed" <unledev@gmail.com> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/102335: [devfs] sx vnode deadlock Message-ID: <200609041040.k84AeVMh091025@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/102335; it has been noted by GNATS.
From: "Alex Unleashed" <unledev@gmail.com>
To: bug-followup@freebsd.org
Cc:
Subject: Re: kern/102335: [devfs] sx vnode deadlock
Date: Mon, 4 Sep 2006 12:32:17 +0200
------=_Part_100777_6803680.1157365937363
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Konstantin Belousov created the following patch against -CURRENT noting that
devfs from 6.X should be replaced by -CURRENT soon enough. This patch trades
the deadlock for a race at unmount time.
Index: devfs.h
===================================================================
RCS file: /usr/local/arch/ncvs/src/sys/fs/devfs/devfs.h,v
retrieving revision 1.29
diff -u -r1.29 devfs.h
--- devfs.h 12 Apr 2006 12:17:29 -0000 1.29
+++ devfs.h 1 Sep 2006 11:00:34 -0000
@@ -163,7 +163,7 @@
void devfs_rules_apply(struct devfs_mount *dm, struct devfs_dirent *de);
void devfs_rules_cleanup (struct devfs_mount *dm);
int devfs_rules_ioctl(struct devfs_mount *dm, u_long cmd, caddr_t data,
struct thread *td);
-int devfs_allocv (struct devfs_dirent *de, struct mount *mp, struct vnode
**vpp, struct thread *td);
+int devfs_allocv (struct devfs_dirent *de, struct mount *mp, struct vnode
**vpp, int *dm_lock, struct thread *td);
void devfs_delete(struct devfs_mount *dm, struct devfs_dirent *de);
void devfs_populate (struct devfs_mount *dm);
void devfs_cleanup (struct devfs_mount *dm);
Index: devfs_vfsops.c
===================================================================
RCS file: /usr/local/arch/ncvs/src/sys/fs/devfs/devfs_vfsops.c,v
retrieving revision 1.50
diff -u -r1.50 devfs_vfsops.c
--- devfs_vfsops.c 17 Jul 2006 09:07:01 -0000 1.50
+++ devfs_vfsops.c 1 Sep 2006 11:00:34 -0000
@@ -135,9 +135,11 @@
int error;
struct vnode *vp;
struct devfs_mount *dmp;
+ int dm_lock;
dmp = VFSTODEVFS(mp);
- error = devfs_allocv(dmp->dm_rootdir, mp, &vp, td);
+ dm_lock = 0;
+ error = devfs_allocv(dmp->dm_rootdir, mp, &vp, &dm_lock, td);
if (error)
return (error);
vp->v_vflag |= VV_ROOT;
Index: devfs_vnops.c
===================================================================
RCS file: /usr/local/arch/ncvs/src/sys/fs/devfs/devfs_vnops.c,v
retrieving revision 1.133
diff -u -r1.133 devfs_vnops.c
--- devfs_vnops.c 17 Jul 2006 09:07:01 -0000 1.133
+++ devfs_vnops.c 1 Sep 2006 11:00:34 -0000
@@ -126,20 +126,26 @@
}
int
-devfs_allocv(struct devfs_dirent *de, struct mount *mp, struct vnode **vpp,
struct thread *td)
+devfs_allocv(struct devfs_dirent *de, struct mount *mp, struct vnode **vpp,
+ int *dm_unlock, struct thread *td)
{
int error;
struct vnode *vp;
struct cdev *dev;
+ struct devfs_mount *dmp;
KASSERT(td == curthread, ("devfs_allocv: td != curthread"));
+ dmp = VFSTODEVFS(mp);
loop:
-
mtx_lock(&devfs_de_interlock);
vp = de->de_vnode;
if (vp != NULL) {
VI_LOCK(vp);
mtx_unlock(&devfs_de_interlock);
+ if (*dm_unlock) {
+ sx_xunlock(&dmp->dm_lock);
+ *dm_unlock = 0;
+ }
if (vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, td))
goto loop;
*vpp = vp;
@@ -182,6 +188,10 @@
vp->v_data = de;
de->de_vnode = vp;
mtx_unlock(&devfs_de_interlock);
+ if (*dm_unlock) {
+ sx_xunlock(&dmp->dm_lock);
+ *dm_unlock = 0;
+ }
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);
#ifdef MAC
mac_associate_vnode_devfs(mp, de, vp);
@@ -456,7 +466,7 @@
}
static int
-devfs_lookupx(struct vop_lookup_args *ap)
+devfs_lookupx(struct vop_lookup_args *ap, int *dm_unlock)
{
struct componentname *cnp;
struct vnode *dvp, **vpp;
@@ -507,7 +517,7 @@
de = TAILQ_FIRST(&dd->de_dlist); /* "." */
de = TAILQ_NEXT(de, de_list); /* ".." */
de = de->de_dir;
- error = devfs_allocv(de, dvp->v_mount, vpp, td);
+ error = devfs_allocv(de, dvp->v_mount, vpp, dm_unlock, td);
vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td);
return (error);
}
@@ -564,7 +574,7 @@
return (0);
}
}
- error = devfs_allocv(de, dvp->v_mount, vpp, td);
+ error = devfs_allocv(de, dvp->v_mount, vpp, dm_unlock, td);
return (error);
}
@@ -573,11 +583,14 @@
{
int j;
struct devfs_mount *dmp;
+ int dm_unlock;
dmp = VFSTODEVFS(ap->a_dvp->v_mount);
+ dm_unlock = 1;
sx_xlock(&dmp->dm_lock);
- j = devfs_lookupx(ap);
- sx_xunlock(&dmp->dm_lock);
+ j = devfs_lookupx(ap, &dm_unlock);
+ if (dm_unlock == 1)
+ sx_xunlock(&dmp->dm_lock);
return (j);
}
@@ -589,6 +602,7 @@
struct thread *td;
struct devfs_dirent *dd, *de;
struct devfs_mount *dmp;
+ int dm_unlock;
int error;
/*
@@ -600,6 +614,7 @@
dvp = ap->a_dvp;
dmp = VFSTODEVFS(dvp->v_mount);
sx_xlock(&dmp->dm_lock);
+ dm_unlock = 1;
cnp = ap->a_cnp;
vpp = ap->a_vpp;
@@ -620,9 +635,10 @@
if (de == NULL)
goto notfound;
de->de_flags &= ~DE_WHITEOUT;
- error = devfs_allocv(de, dvp->v_mount, vpp, td);
+ error = devfs_allocv(de, dvp->v_mount, vpp, &dm_unlock, td);
notfound:
- sx_xunlock(&dmp->dm_lock);
+ if (dm_unlock == 1)
+ sx_xunlock(&dmp->dm_lock);
return (error);
}
@@ -1102,6 +1118,7 @@
struct devfs_dirent *de;
struct devfs_mount *dmp;
struct thread *td;
+ int dm_unlock;
td = ap->a_cnp->cn_thread;
KASSERT(td == curthread, ("devfs_symlink: td != curthread"));
@@ -1120,12 +1137,14 @@
de->de_symlink = malloc(i, M_DEVFS, M_WAITOK);
bcopy(ap->a_target, de->de_symlink, i);
sx_xlock(&dmp->dm_lock);
+ dm_unlock = 1;
#ifdef MAC
mac_create_devfs_symlink(ap->a_cnp->cn_cred, dmp->dm_mount, dd, de);
#endif
TAILQ_INSERT_TAIL(&dd->de_dlist, de, de_list);
- devfs_allocv(de, ap->a_dvp->v_mount, ap->a_vpp, td);
- sx_xunlock(&dmp->dm_lock);
+ devfs_allocv(de, ap->a_dvp->v_mount, ap->a_vpp, &dm_unlock, td);
+ if (dm_unlock == 1)
+ sx_xunlock(&dmp->dm_lock);
return (0);
}
------=_Part_100777_6803680.1157365937363
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Konstantin Belousov created the following patch against -CURRENT noting that devfs from 6.X should be replaced by -CURRENT soon enough. This patch trades the deadlock for a race at unmount time.<br><br>Index: devfs.h<br>===================================================================
<br><div style="direction: ltr;">RCS file: /usr/local/arch/ncvs/src/sys/fs/devfs/devfs.h,v<br>retrieving revision 1.29<br>diff -u -r1.29 devfs.h<br>--- devfs.h 12 Apr 2006 12:17:29 -0000 1.29<br>+++ devfs.h 1 Sep 2006 11:00:34 -0000
<br>@@ -163,7 +163,7 @@<br> void devfs_rules_apply(struct devfs_mount *dm, struct devfs_dirent *de);<br> void devfs_rules_cleanup (struct devfs_mount *dm);<br> int devfs_rules_ioctl(struct devfs_mount *dm, u_long cmd, caddr_t data, struct thread *td);
<br>-int devfs_allocv (struct devfs_dirent *de, struct mount *mp, struct vnode **vpp, struct thread *td);<br>+int devfs_allocv (struct devfs_dirent *de, struct mount *mp, struct vnode **vpp, int *dm_lock, struct thread *td);
<br> void devfs_delete(struct devfs_mount *dm, struct devfs_dirent *de);<br> void devfs_populate (struct devfs_mount *dm);<br> void devfs_cleanup (struct devfs_mount *dm);<br>Index: devfs_vfsops.c<br>===================================================================
<br>RCS file: /usr/local/arch/ncvs/src/sys/fs/devfs/devfs_vfsops.c,v<br>retrieving revision 1.50<br>diff -u -r1.50 devfs_vfsops.c<br>--- devfs_vfsops.c 17 Jul 2006 09:07:01 -0000 1.50<br>+++ devfs_vfsops.c 1 Sep 2006 11:00:34 -0000
<br>@@ -135,9 +135,11 @@<br> int error;<br> struct vnode *vp;<br> struct devfs_mount *dmp;<br>+ int dm_lock;<br><br> dmp = VFSTODEVFS(mp);<br>- error = devfs_allocv(dmp->dm_rootdir, mp, &vp, td);
<br>+ dm_lock = 0;<br>+ error = devfs_allocv(dmp->dm_rootdir, mp, &vp, &dm_lock, td);<br> if (error)<br> return (error);<br> vp->v_vflag |= VV_ROOT;<br>Index: devfs_vnops.c
<br>===================================================================<br>RCS file: /usr/local/arch/ncvs/src/sys/fs/devfs/devfs_vnops.c,v<br>retrieving revision 1.133<br>diff -u -r1.133 devfs_vnops.c<br>--- devfs_vnops.c 17 Jul 2006 09:07:01 -0000
1.133<br>+++ devfs_vnops.c 1 Sep 2006 11:00:34 -0000<br>@@ -126,20 +126,26 @@<br> }<br><br> int<br>-devfs_allocv(struct devfs_dirent *de, struct mount *mp, struct vnode **vpp, struct thread *td)<br>+devfs_allocv(struct devfs_dirent *de, struct mount *mp, struct vnode **vpp,
<br>+ int *dm_unlock, struct thread *td)<br> {<br> int error;<br> struct vnode *vp;<br> struct cdev *dev;<br>+ struct devfs_mount *dmp;<br><br> KASSERT(td == curthread, ("devfs_allocv: td != curthread"));
<br>+ dmp = VFSTODEVFS(mp);<br> loop:<br>-<br> mtx_lock(&devfs_de_interlock);<br> vp = de->de_vnode;<br> if (vp != NULL) {<br> VI_LOCK(vp);<br> mtx_unlock(&devfs_de_interlock);
<br>+ if (*dm_unlock) {<br>+ sx_xunlock(&dmp->dm_lock);<br>+ *dm_unlock = 0;<br>+ }<br> if (vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, td))
<br> goto loop;<br> *vpp = vp;<br>@@ -182,6 +188,10 @@<br> vp->v_data = de;<br> de->de_vnode = vp;<br> mtx_unlock(&devfs_de_interlock);<br>+ if (*dm_unlock) {
<br>+ sx_xunlock(&dmp->dm_lock);<br>+ *dm_unlock = 0;<br>+ }<br> vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);<br> #ifdef MAC<br> mac_associate_vnode_devfs(mp, de, vp);<br>
@@ -456,7 +466,7 @@<br> }<br><br> static int<br>-devfs_lookupx(struct vop_lookup_args *ap)<br>+devfs_lookupx(struct vop_lookup_args *ap, int *dm_unlock)<br> {<br> struct componentname *cnp;<br> struct vnode *dvp, **vpp;
<br>@@ -507,7 +517,7 @@<br> de = TAILQ_FIRST(&dd->de_dlist); /* "." */<br> de = TAILQ_NEXT(de, de_list); /* ".." */<br> de = de->de_dir;
<br>- error = devfs_allocv(de, dvp->v_mount, vpp, td);<br>+ error = devfs_allocv(de, dvp->v_mount, vpp, dm_unlock, td);<br> vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td);<br> return (error);
<br> }<br>@@ -564,7 +574,7 @@<br> return (0);<br> }<br> }<br>- error = devfs_allocv(de, dvp->v_mount, vpp, td);<br>+ error = devfs_allocv(de, dvp->v_mount, vpp, dm_unlock, td);
<br> return (error);<br> }<br><br>@@ -573,11 +583,14 @@<br> {<br> int j;<br> struct devfs_mount *dmp;<br>+ int dm_unlock;<br><br> dmp = VFSTODEVFS(ap->a_dvp->v_mount);<br>+ dm_unlock = 1;
<br> sx_xlock(&dmp->dm_lock);<br>- j = devfs_lookupx(ap);<br>- sx_xunlock(&dmp->dm_lock);<br>+ j = devfs_lookupx(ap, &dm_unlock);<br>+ if (dm_unlock == 1)<br>+ sx_xunlock(&dmp->dm_lock);
<br> return (j);<br> }<br><br>@@ -589,6 +602,7 @@<br> struct thread *td;<br> struct devfs_dirent *dd, *de;<br> struct devfs_mount *dmp;<br>+ int dm_unlock;<br> int error;<br><br> /*
<br>@@ -600,6 +614,7 @@<br> dvp = ap->a_dvp;<br> dmp = VFSTODEVFS(dvp->v_mount);<br> sx_xlock(&dmp->dm_lock);<br>+ dm_unlock = 1;<br><br> cnp = ap->a_cnp;<br> vpp = ap->a_vpp;
<br>@@ -620,9 +635,10 @@<br> if (de == NULL)<br> goto notfound;<br> de->de_flags &= ~DE_WHITEOUT;<br>- error = devfs_allocv(de, dvp->v_mount, vpp, td);<br>+ error = devfs_allocv(de, dvp->v_mount, vpp, &dm_unlock, td);
<br> notfound:<br>- sx_xunlock(&dmp->dm_lock);<br>+ if (dm_unlock == 1)<br>+ sx_xunlock(&dmp->dm_lock);<br> return (error);<br> }<br><br>@@ -1102,6 +1118,7 @@<br> struct devfs_dirent *de;
<br> struct devfs_mount *dmp;<br> struct thread *td;<br>+ int dm_unlock;<br><br> td = ap->a_cnp->cn_thread;<br> KASSERT(td == curthread, ("devfs_symlink: td != curthread"));
<br>@@ -1120,12 +1137,14 @@<br> de->de_symlink = malloc(i, M_DEVFS, M_WAITOK);<br> bcopy(ap->a_target, de->de_symlink, i);<br> sx_xlock(&dmp->dm_lock);<br>+ dm_unlock = 1;<br> #ifdef MAC
<br> mac_create_devfs_symlink(ap->a_cnp->cn_cred, dmp->dm_mount, dd, de);<br> #endif<br> TAILQ_INSERT_TAIL(&dd->de_dlist, de, de_list);<br>- devfs_allocv(de, ap->a_dvp->v_mount, ap->a_vpp, td);
<br>- sx_xunlock(&dmp->dm_lock);<br>+ devfs_allocv(de, ap->a_dvp->v_mount, ap->a_vpp, &dm_unlock, td);<br>+ if (dm_unlock == 1)<br>+ sx_xunlock(&dmp->dm_lock);<br>
return (0);<br> }<br></div><br><br>
------=_Part_100777_6803680.1157365937363--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200609041040.k84AeVMh091025>