Date: Sat, 22 Nov 2008 13:50:28 +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: <20081122115028.GB6408@deviant.kiev.zoral.com.ua> In-Reply-To: <200811211452.02545.jhb@freebsd.org> References: <200811201627.58289.jhb@freebsd.org> <3a142e750811201532u6e697488w2747242e0a8614a9@mail.gmail.com> <200811211452.02545.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--TakKZr9L6Hm6aLOc Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 21, 2008 at 02:52:02PM -0500, John Baldwin wrote: > On Thursday 20 November 2008 06:32:25 pm Paul B. Mahol wrote: > > On 11/20/08, John Baldwin <jhb@freebsd.org> wrote: > > > So this patch is fairly minimal since udf(4) is currently read-only. = The > > > changes include: > > > > > > * Set VV_ROOT in udf_vget() if we ever return a vnode instead of doin= g it > > > only > > > in udf_root(). This matches the behavior of other operating system= s and > > > correctly tags the root vnode with VV_ROOT in the unlikely case tha= t we > > > create the vnode during a call to ufs_vget() that does not come from > > > ufs_root(). > > > * If the hash lookup in ufs_vget() fails, ensure an exclusive vnode l= ock=20 > is > > > used while creating the new vnode (same as UFS). > > > * Allow lock recursion (XXX: not really sure this is needed actually). > > > * Allow shared vnode locks on non-fifos. > > > * Honor the requested locking flags (shared vs exclusive) instead of= =20 > always > > > using exclusive vnode locks during a lookup operation. > > > * Handle "." lookups the same way other filesystems do by just bumpin= g the > > > reference on 'dvp' rather than calling udf_vget(). > > > > > > http://www.FreeBSD.org/~jhb/patches/udf_mpsafe.patch > > > > > > I have only verified that this compiles and would appreciate it if so= me > > > folks > > > could test this, preferable with INVARIANTS and DEBUG_VFS_LOCKS enabl= ed. > >=20 > > I lightly tested it with md(4) on 47M size iso created with k3b > > (it contains two files) > >=20 > > I only got this lor related to udf: > >=20 > > lock order reversal: > > 1st 0xc4449270 udf (udf) @ /usr/src/sys/kern/vfs_lookup.c:442 > > 2nd 0xd7d10850 bufwait (bufwait) @ /usr/src/sys/kern/vfs_bio.c:2443 > > 3rd 0xc4399488 udf (udf) @ > > /usr/src/sys/modules/udf/../../fs/udf/udf_vfsops.c:625 >=20 > I've updated the patch at the same URL above to fix this LOR. 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. 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. --TakKZr9L6Hm6aLOc Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkkn8gMACgkQC3+MBN1Mb4jVOwCcDQGGhm0gQ++oiEAy3KptHUFF sNAAoMDBs3rcsTLN+eS3GJ1jQtGhy3lN =xTI5 -----END PGP SIGNATURE----- --TakKZr9L6Hm6aLOc--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20081122115028.GB6408>