On Fri, Jun 23, 2006 at 05:14:00PM -0700, Bill Studenmund wrote:
> On Fri, Jun 23, 2006 at 09:02:33PM +0200, Juergen Hannken-Illjes wrote:
> > On Fri, Jun 23, 2006 at 10:26:00AM -0700, Bill Studenmund wrote:
> > > On Thu, Jun 22, 2006 at 05:55:52PM +0200, Juergen Hannken-Illjes wrote:
> > > > For specfs/fifofs we could
> > > > 
> > > >     need_hold = (vp->v_flags & VHOLD);
> > > >     if (need_hold)
> > > > 	vn_release(vp);
> > > >     ... device operation that may sleep long ...
> > > >     if (need_hold)
> > > > 	vn_hold(vp, V_WAIT);
> > > 
> > > I don't see why fifofs should get special treatment.
> > 
> > It has long sleeps on "fifor" and "fifow" and its so_receive() and so_send()
> > calls may also go to long sleep.
> 
> The thing is that if we are really trying to support transactions, the
> it's very questionable for the file system to release then reaquire the 
> transaction lock.
> 
> Especially if we go with the idea of having the file system grab and 
> release the mountpoint-transaction-lock (the current incarnation of the 
> "gate" you spoke of originally, the thing that makes us atomic w.r.t. a 
> snapshot), then it's not a problem. We just have the read and write code 
> in fifofs not take the transaction lock, and we're fine.
> 
> Not taking a lock is fine, releasing someone else's lock and retaking one 
> is a problem.
> 
> Such a change would limit our exposure to cases where someone is trying to 
> make a transaction that involves writing to or reading from a fifo. If you 
> try to do that, you get what you get.

Both specfs and fifofs need special care because they are no real file systems.
Their vnodes live in real file systems that may update meta data before or
after they call operations on specfs/fifofs.  These updates need the transaction
lock.  The real operations (as long as they dont go to disk devices) cannot
keep this lock because they may sleep forever waiting for data.

We could move the special treatment of fifofs up by something like

    if (vp->v_type != VFIFO)
	vn_hold(vp);
    VOP_XXX(vp, ...);
    if (vp->v_type != VFIFO)
	vn_release(vp);

We cannot do this for for VCHR/VBLK devices because they may be tapes that
also need this special treatment on VBLK devices.

To me it looks better to put it into fifofs the same way its put into specfs.

And I'm still looking for a better name instead of vn_hold/vn_release ...
-- 
Juergen Hannken-Illjes - hannken@eis.cs.tu-bs.de - TU Braunschweig (Germany)