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>
index | next in thread | previous in thread | raw e-mail
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). Brucehome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030517160738.U15076>