Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 3 Dec 2008 14:41:37 +0200
From:      Kostik Belousov <kostikbel@gmail.com>
To:        John Baldwin <jhb@freebsd.org>
Cc:        scottl@freebsd.org, current@freebsd.org
Subject:   Re: [PATCH] Make udf(4) MPSAFE and use shared lookups
Message-ID:  <20081203124137.GE3045@deviant.kiev.zoral.com.ua>
In-Reply-To: <200812021828.36857.jhb@freebsd.org>
References:  <200811201627.58289.jhb@freebsd.org> <200811211452.02545.jhb@freebsd.org> <20081122115028.GB6408@deviant.kiev.zoral.com.ua> <200812021828.36857.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--q/d9vTEvvdeKbPNw
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Dec 02, 2008 at 06:28:36PM -0500, John Baldwin wrote:
> On Saturday 22 November 2008 06:50:28 am Kostik Belousov wrote:
> > udf_vget() does insmntque() before vnode is fully initialized, allowing
> > other threads to find the vnode on the mount list. This is typical for
> > !MPSAFE fs, and it seems corresponding call was not marked XXX for udf.
>=20
> It does the same as ufs.  ufs only partially initializes the i-node (as m=
uch=20
> as both cd9660 and udf do) and then exclusive locks the vnode before=20
> insmntque().  They then finish initializing the i-node (bread() the d-nod=
e,=20
> for example) and finally drop the vnode lock.

My bad, unjustified grumbling.

>=20
> > udf_lookup for ISDOTDOT case unlocks dvp before vget'ing "..", allowing
> > the same race on forced unmount as ufs (I will finally commit ufs patch
> > today). The race happens for !MPSAFE code too, but it is easier to
> > execute without Giant.
>=20
> Every fs is going to need this workaround it seems.  Would be nice if the=
re=20
> was an easier way to avoid cut and pasting this code N times.  Perhaps we=
=20
> could make lookup() check VI_DOOMED instead?  I had changed it do that at=
 one=20
> point, but then someone pointed me at the deadfs stuff and said that was=
=20
> sufficient.

The point of the patch is busying mp while parent vnode is locked, that
guarantees that mp is not unmounted during whole DOTDOT traversing in
vop_lookup(). The deadfs stuff works for lookup result vnode and is
sufficient.

The fragment that someone committed into UFS can be extracted into the
vfs support routine. I doubt that it can be embedded into lookup().
The problem is that some filesystems do additional operations inside
vop_lookup().

--q/d9vTEvvdeKbPNw
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (FreeBSD)

iEYEARECAAYFAkk2foEACgkQC3+MBN1Mb4hA4QCfbOvumkkzfUeFlQNtEddszDdq
rMwAnjCmPda6vAtxmlGFrclRn/KYE4DB
=ikoi
-----END PGP SIGNATURE-----

--q/d9vTEvvdeKbPNw--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20081203124137.GE3045>