From owner-freebsd-hackers Mon Nov 19 3:29:22 2001 Delivered-To: freebsd-hackers@freebsd.org Received: from relay1.macomnet.ru (relay1.macomnet.ru [195.128.64.10]) by hub.freebsd.org (Postfix) with ESMTP id 6F09437B405; Mon, 19 Nov 2001 03:29:17 -0800 (PST) Received: from news1.macomnet.ru (maxim@news1.macomnet.ru [195.128.64.14]) by relay1.macomnet.ru (8.11.3/8.11.3) with ESMTP id fAJBTA7351844; Mon, 19 Nov 2001 14:29:10 +0300 (MSK) Date: Mon, 19 Nov 2001 14:29:10 +0300 (MSK) From: Maxim Konovalov To: Terry Lambert Cc: Alfred Perlstein , John Baldwin , Subject: Re: has 'options LOCKF_DEBUG' ever worked? (w/ patch) (fwd) In-Reply-To: <3BF5AC86.8A402C96@mindspring.com> Message-ID: <20011119133336.B37219-100000@news1.macomnet.ru> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Hello, Thank you Terry for your answer, I got your meaning. IMHO we can safely commit my patch meanwhile: http://news1.macomnet.ru/~maxim/p/kern_lockf.c.diff Committers? :-) On Fri, 16 Nov 2001, Terry Lambert wrote: > Maxim Konovalov wrote: > > Alfred, John, thanks you very much for your answers. I expected > > something similar. Btw are there any smart ways to find out does > > underlying FS support inode concept or not? Yes, I know about > > vnode.v_tag, but comparing it with VT_UFS/VT_NFS/VT_MFS etc does not > > look OK for me. > > In theory, vnodes are supposed to be opaque types, with no backing > object exposure. > > The real problem here is that the lock list should not be hung > off the inode at all, it should be hung off the vnode, and the > underlying FS objects not referenced (as you noted, there's a > "two for one" object reference locking problem in the current > code). You still need to go to the backing FS, though, since > you need to proxy NFS client locks to remote objects, etc.. IF > you look at ufs_advlock(), you can see the problem: > > ufs_advlock(ap) > struct vop_advlock_args /* { > struct vnode *a_vp; > caddr_t a_id; > int a_op; > struct flock *a_fl; > int a_flags; > } */ *ap; > { > register struct inode *ip = VTOI(ap->a_vp); > > return (lf_advlock(ap, &(ip->i_lockf), ip->i_size)); > } > > ... the vp lock has to imply an ip lock, at least for the i_lockf > list head, which is procedurally exposed. > > A better fix would be to move the lock list into the vnode, and > then call the VOP_ADVLOCK() and assert the lock only if the VFS > VOP_ADVLOCK() returned true. > > This really can't be done safely, since you need to addert locally > before remotely, but then you need to delay the local coelesce, or > you can screw up if the local succeeds and the remote fails, or > vice versa. > > So the true fix has to include delayed coelescing of the local lock > assertion. > > That's true anyway, since the NFSv4 specification demands that the > locks not be coelesced at all, for an implementation to be compliant. > > -- Terry > > -- Maxim Konovalov, MAcomnet, Internet-Intranet Dept., system engineer phone: +7 (095) 796-9079, mailto: maxim@macomnet.ru To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message