Date: Tue, 12 Jul 2011 14:19:25 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Kohji Okuno <okuno.kohji@jp.panasonic.com> Cc: freebsd-current@freebsd.org Subject: Re: Bug about devfs? Message-ID: <20110712111925.GH43872@deviant.kiev.zoral.com.ua> In-Reply-To: <20110712.191028.650619413057975749.okuno.kohji@jp.panasonic.com> References: <20110712.191028.650619413057975749.okuno.kohji@jp.panasonic.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--TMJUm4hsTn/F+xXF
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Tue, Jul 12, 2011 at 07:10:28PM +0900, Kohji Okuno wrote:
> Hello,
>=20
> I think that devfs has a problem.
> I encountered the problem that open("/dev/AAA") returned ENOENT.
> Of course, /dev/AAA exists.
>=20
> ENOENT was created by the point(***) in devfs_allocv().
> I think that the race condition had occurred between process A and
> vnlru kernel thread.
>=20
> Please check the following.
>=20
> If vnlru set VI_DOOMED to vp->v_iflag but vnlru didn't still execute
> VOP_RECLAIM(), process A cat get valid vp from de->de_vnode.
> But, vget() will return ENOENT, because vp->v_iflag has VI_DOOMED.
>=20
> When I set the break point to (***), I checked that de->de_vnode and
> vp->v_data were NULL.
>=20
>=20
> process A: vnlru:
>=20
> devfs_allocv() {
> vgonel(vp) {
> ... ...
> vp->v_iflag |=3D VI_DOOMED;
> mtx_lock(&devfs_de_interlock); ...
> vp =3D de->de_vnode; =20
> if (vp !=3D NULL) { VI_UNLOCK(vp);
> _____________/ ...
> VI_LOCK(vp); ____________/ if (VOP_RECLAIM(vp, td))
> mtx_unlock(&devfs_de_interlock); ...
> ... \ devfs_reclaim(ap) {
> error =3D vget(vp,...); \ =20
> ... \______ mtx_lock(&devfs_de_interlock);=20
> if (devfs_allocv_drop_refs(...)) { de =3D vp->v_data;
> ... if (de !=3D NULL) {
> } de->de_vnode =3D NULL; =20
> else if (error) { vp->v_data =3D NULL;
> ... }
> rturn (error); (***) mtx_unlock(&devfs_de_interlock);
> } ...
> }
>=20
>=20
>=20
> I think that devfs_allocv() should be fixed as below.
> How do you think?
>=20
> devfs_allocv(struct devfs_dirent *de, struct mount *mp, struct vnode **vp=
p)
> {
> int error;
> struct vnode *vp;
> struct cdev *dev;
> struct devfs_mount *dmp;
> =20
> dmp =3D VFSTODEVFS(mp);
> +#if 1
> + retry:
> +#endif
> if (de->de_flags & DE_DOOMED) {
>=20
> ...
>=20
> mtx_lock(&devfs_de_interlock);
> vp =3D de->de_vnode;
> if (vp !=3D NULL) {
> VI_LOCK(vp);
> mtx_unlock(&devfs_de_interlock);
> sx_xunlock(&dmp->dm_lock);
> error =3D vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, curthread=
);
> sx_xlock(&dmp->dm_lock);
> if (devfs_allocv_drop_refs(0, dmp, de)) {
> if (error =3D=3D 0)
> vput(vp);
> return (ENOENT);
> }
> else if (error) {
> +#if 1
> + if (error =3D=3D ENOENT)
> + goto retry;
> +#endif
> sx_xunlock(&dmp->dm_lock);
> return (error);
> }
>=20
Thank you for the report.
The proposed change would revert r179247, which also caused some issues.
Are you able to reproduce the problem ?
Could you try the following patch ? I cannot reproduce your situation,
so the patch is untested by me.
diff --git a/sys/fs/devfs/devfs_vnops.c b/sys/fs/devfs/devfs_vnops.c
index bf6dab8..bbbfff4 100644
--- a/sys/fs/devfs/devfs_vnops.c
+++ b/sys/fs/devfs/devfs_vnops.c
@@ -397,6 +397,7 @@ devfs_allocv(struct devfs_dirent *de, struct mount *mp,=
int lockmode,
sx_xunlock(&dmp->dm_lock);
return (ENOENT);
}
+loop:
DEVFS_DE_HOLD(de);
DEVFS_DMP_HOLD(dmp);
mtx_lock(&devfs_de_interlock);
@@ -412,7 +413,16 @@ devfs_allocv(struct devfs_dirent *de, struct mount *mp=
, int lockmode,
vput(vp);
return (ENOENT);
}
- else if (error) {
+ else if (error !=3D 0) {
+ if (error =3D=3D ENOENT) {
+ mtx_lock(&devfs_de_interlock);
+ while (de->de_vnode !=3D NULL) {
+ msleep(&de->de_vnode,
+ &devfs_de_interlock, 0, "dvall", 0);
+ }
+ mtx_unlock(&devfs_de_interlock);
+ goto loop;
+ }
sx_xunlock(&dmp->dm_lock);
return (error);
}
@@ -1259,6 +1269,7 @@ devfs_reclaim(struct vop_reclaim_args *ap)
if (de !=3D NULL) {
de->de_vnode =3D NULL;
vp->v_data =3D NULL;
+ wakeup(&de->de_vnode);
}
mtx_unlock(&devfs_de_interlock);
=20
--TMJUm4hsTn/F+xXF
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)
iEYEARECAAYFAk4cLb0ACgkQC3+MBN1Mb4h/mACeKHM9d6XArwTUhxSrFv8cK0tG
lIAAnRXA7KmckLgCMGE8XG5jpQqwJHnM
=X++1
-----END PGP SIGNATURE-----
--TMJUm4hsTn/F+xXF--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110712111925.GH43872>