Date: Sat, 17 May 2003 16:57:06 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Don Lewis <truckman@FreeBSD.org> Cc: current@FreeBSD.org Subject: Re: CFR: fifo_open()/fifo_close() patch Message-ID: <20030517160738.U15076@gamplex.bde.org> In-Reply-To: <200305161646.h4GGjuM7058624@gw.catspoiler.org> References: <200305161646.h4GGjuM7058624@gw.catspoiler.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 16 May 2003, Don Lewis wrote: > On 16 May, Bruce Evans wrote: > > Why not just lock the vnode in fifo_close()? RELENG[2-4] seems to have > > the same bug. I cannot be fixed there using the vnode interlock. > > That is probably the proper fix for RELENG4 where finer-grained locking > isn't needed because of Giant. I'd still probably move the resource > deallocation to fifo_inactive(). RELENG_4 actually can probably be fixed using the vnode interlock. The interlock exists there. It just may require more care because it is a spinlock. My question is mainly: why do you want or need the extra complexity for using the vnode interlock here? Hmm, I see that at least ufs_close() likes to use only the vnode interlock, but this seems to be wrong. From ufs_vnops.c:vop_close(): % VI_LOCK(vp); % if (vp->v_usecount > 1) % ufs_itimes(vp); % VI_UNLOCK(vp); % return (VOCALL(fifo_vnodeop_p, VOFFSET(vop_close), ap)); The interlock protects the v_usecount check here, but AFAIK it doesn't protect ufs_itimes(). ufs_itimes() hacks on the inode's flags and timestamps without acquiring any other locks. I believe it expects to be locked by the vnode lock. This behaviour goes back to Lite1 or before (not much except the names have changed since rev.1.1). Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030517160738.U15076>