On Sun, Dec 31, 2006 at 10:34:03AM +0000, Pavel Cahyna wrote:
> On Sun, Dec 31, 2006 at 11:23:26AM +0100, Juergen Hannken-Illjes wrote:
> > On Sun, Dec 31, 2006 at 11:12:52AM +0100, Pavel Cahyna wrote:
> > > On Fri, Dec 29, 2006 at 04:34:45PM +0100, Juergen Hannken-Illjes wrote:
> > > > +/*
> > > > + * Lock the node.
> > > > + */
> > > > +int
> > > > +ufs_lock(void *v)
> > > > +{
> > > > +	struct vop_lock_args /* {
> > > > +		struct vnode *a_vp;
> > > > +		int a_flags;
> > > > +	} */ *ap = v;
> > > > +	struct vnode *vp = ap->a_vp;
> > > > +	struct mount *mp = vp->v_mount;
> > > > +
> > > > +	/*
> > > > +	 * Fake lock during file system suspension.
> > > > +	 */
> > > > +	if ((vp->v_type == VREG || vp->v_type == VDIR) &&
> > > > +	    fstrans_is_owner(mp) &&
> > > > +	    fstrans_getstate(mp) == fstrans_suspending) {
> > > > +		if ((ap->a_flags & LK_INTERLOCK) != 0)
> > > > +			simple_unlock(&vp->v_interlock);
> > > > +		return 0;
> > > 
> > > How is safe to "fake" a lock? If the caller wants to lock something, it
> > > probably does not expect the lock to be "faked".
> > 
> > Caller here is the thread running the suspension (fstrans_is_owner(mp)) while
> > it is preparing the suspension (fstrans_getstate(mp) == fstrans_suspending).
> > This is only the call to ffs_sync() in ffs_suspendctl().
> 
> But how do you prevent ffs_sync from seeing inconsistent state?

By taking care :-) During suspension most operations are blocked.  We only have
to care about those taking lazy locks.

> > ffs_sync() will release all locks it got so we will not break other locks.
> > 
> > > Why is it needed at all?
> > 
> > ffs_sync() and all operations it calls depend on locks. It would become an
> > ugly maze if we converted all these places to ignore locks while suspending.
> 
> Why should it ignore lock while suspending instead of taking them
> normally?

As we cannot add fstrans_start() to xxx_lock() not ignoring locks will
quickly deadlock.

-- 
Juergen Hannken-Illjes - hannken@eis.cs.tu-bs.de - TU Braunschweig (Germany)