From owner-freebsd-current@FreeBSD.ORG Sat Nov 22 00:04:51 2008 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E4FE5106564A for ; Sat, 22 Nov 2008 00:04:50 +0000 (UTC) (envelope-from onemda@gmail.com) Received: from mail-gx0-f12.google.com (mail-gx0-f12.google.com [209.85.217.12]) by mx1.freebsd.org (Postfix) with ESMTP id 4E98B8FC16 for ; Sat, 22 Nov 2008 00:04:49 +0000 (UTC) (envelope-from onemda@gmail.com) Received: by gxk5 with SMTP id 5so90159gxk.19 for ; Fri, 21 Nov 2008 16:04:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:to :subject:cc:in-reply-to:mime-version:content-type :content-transfer-encoding:content-disposition:references; bh=oR03/9cpmQ3rs8adJeBc7dA4UYNMfVCbAhJ6CB7+skY=; b=YoLYJO2G6pGmTYt2JlAI/06CzJttMvT8mLauUWGb5q8i9seKjr1dIPmW1U1OVZkPMn T+CaLXGqMqp6hr9v4BaaHBTvjgUHRLUdm0dPAB3ILe6cjz3LhEWfA9GfYSWfhukzH5r+ FbQyO71ZeaV2JOSMmrh2OUIYnzvwzvFDbcIf4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:cc:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=moOrKEeRBqDjw3bJ22L4A/YApEqOs27HC7YLPe7//WNnQlxD5xazMRZwcUy1bwHrJL zgdWilMn2fDQ364KQR+mXj9TbnEAB/QMfKC6OeD6CD0fr/2qpbUAP0OpqMycIv3D8l2g hhSkdWMI5WLhTTw9ccN7G9mf9JJW5B0AJASfc= Received: by 10.231.16.129 with SMTP id o1mr27656iba.47.1227312287582; Fri, 21 Nov 2008 16:04:47 -0800 (PST) Received: by 10.231.11.7 with HTTP; Fri, 21 Nov 2008 16:04:47 -0800 (PST) Message-ID: <3a142e750811211604u75e44c0fifa66529f12b4335a@mail.gmail.com> Date: Sat, 22 Nov 2008 01:04:47 +0100 From: "Paul B. Mahol" To: "John Baldwin" In-Reply-To: <200811211353.42283.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200811201627.58289.jhb@freebsd.org> <3a142e750811201532u6e697488w2747242e0a8614a9@mail.gmail.com> <200811211353.42283.jhb@freebsd.org> Cc: scottl@freebsd.org, current@freebsd.org Subject: Re: [PATCH] Make udf(4) MPSAFE and use shared lookups X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 22 Nov 2008 00:04:51 -0000 On 11/21/08, John Baldwin wrote: > On Thursday 20 November 2008 06:32:25 pm Paul B. Mahol wrote: >> On 11/20/08, John Baldwin 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 doing >> > it >> > only >> > in udf_root(). This matches the behavior of other operating systems >> > and >> > correctly tags the root vnode with VV_ROOT in the unlikely case that >> > 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 lock >> > > 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 > always >> > using exclusive vnode locks during a lookup operation. >> > * Handle "." lookups the same way other filesystems do by just bumping >> > 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 some >> > folks >> > could test this, preferable with INVARIANTS and DEBUG_VFS_LOCKS enabled. >> >> I lightly tested it with md(4) on 47M size iso created with k3b >> (it contains two files) >> >> I only got this lor related to udf: >> >> 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 > > I think you would get this same LOR w/o the patch. I think cd9660 might > have > the same bug. The issue is holding a buffer from bread() (i.e. not calling > brelse() yet) while calling udf_vget() in udf_lookup(). cd9660 have this LOR: lock order reversal: 1st 0xc43a0594 isofs (isofs) @ /usr/src/sys/kern/vfs_lookup.c:442 2nd 0xd7d105b0 bufwait (bufwait) @ /usr/src/sys/kern/vfs_bio.c:2443 3rd 0xc43a0488 isofs (isofs) @ /usr/src/sys/modules/cd9660/../../fs/cd9660/cd9660_vfsops.c:694 KDB: stack backtrace: db_trace_self_wrapper(c069e216,c3b646d0,c04e793f,4,c069995b,...) at db_trace_self_wrapper+0x26 kdb_backtrace(4,c069995b,c3cb7538,c3cb9ca0,c3b6472c,...) at kdb_backtrace+0x29 _witness_debugger(c06a0ee3,c43a0488,c443da9a,c3cb9ca0,c443d9de,...) at _witness_debugger+0x1e witness_checkorder(c43a0488,9,c443d9de,2b6,0,...) at witness_checkorder+0x811 __lockmgr_args(c43a0488,80000,0,0,0,...) at __lockmgr_args+0x762 cd9660_vget_internal(c443f500,1e0ee,200000,c3b648b0,0,...) at cd9660_vget_internal+0x13e cd9660_lookup(c3b649fc,c43a053c,c3b64bb4,c43a053c,c3b64a1c,...) at cd9660_lookup+0x621 VOP_CACHEDLOOKUP_APV(c443e360,c3b649fc,c3b64bb4,c3b64ba0,c06fa1c0,...) at VOP_CACHEDLOOKUP_APV+0xa0 vfs_cache_lookup(c3b64a7c,c3b64a7c,0,200000,c43a053c,...) at vfs_cache_lookup+0xc3 VOP_LOOKUP_APV(c443e360,c3b64a7c,c06a6c14,1ba,c3b64ba0,...) at VOP_LOOKUP_APV+0xaa lookup(c3b64b88,c06a6c14,dc,bc,c4186a2c,...) at lookup+0x507 namei(c3b64b88,c06a693e,143,c108ba34,c3c80000,...) at namei+0x45b kern_statat(c4442000,200,ffffff9c,282162f8,0,...) at kern_statat+0x66 kern_lstat(c4442000,282162f8,0,c3b64c1c,0,...) at kern_lstat+0x36 lstat(c4442000,c3b64cf8,8,c06a1b96,c06cfb50,...) at lstat+0x2b syscall(c3b64d38) at syscall+0x261 Xint0x80_syscall() at Xint0x80_syscall+0x20 --- syscall (5, FreeBSD ELF32, open), eip = 0x281f13ab, esp = 0xbfbfe8ac, ebp = 0xbfbfe8c8 --- -- Paul