Re: [PATCH] Fixing soft NFS umount -f, round 5

[David Holland <dholland-tech%netbsd.org@localhost>]

On Sun, Jul 12, 2015 at 06:54:21PM -0700, Chuck Silvers wrote:
 > > Now I think it would be nice to also cut coners in VFS_SYNC() when 
 > > the force flag is used, but that touches filesystem-indpendent code,
 > > in dounmount():
 > >         if ((mp->mnt_flag & MNT_RDONLY) == 0) {
 > >                 error = VFS_SYNC(mp, MNT_WAIT, l->l_cred);
 > >         }       
 > >         if (error == 0 || (flags & MNT_FORCE)) {
 > >                 error = VFS_UNMOUNT(mp, flags);
 > >         }
 > > 
 > > The first VFS_SYNC() is making us wait even if MNT_FORCE is
 > > set. This could be solved by adding a IMNT_UMOUNTFORCE to struct
 > > mount's mnt_iflag, just like I did for NFS in this patch. The
 > > flag would instruct underlying filesystem that force unmount is
 > > required and that fast return is expected.

Can this please be an argument to sync instead of a global state flag?
If nfs needs to make it an internal global state flag that's one
thing, but in general the proliferation of such flags is bad.

 > I haven't kept up with the VFS-level locking changes in the last
 > few years so I'll let someone else comment about all that.
 > 
 > in particular, I don't know what (if anything) prevents vnodes from
 > becoming dirty again in between the VFS_SYNC() and the
 > VFS_UNMOUNT().  every file system driver appears to flush
 > everything again (via vflush()) in its *_unmount() method, so I
 > don't know what benefit the VFS_SYNC() in dounmount() is providing.

I don't know offhand, but in another environment I've been working in
recently there's such a sync call and it works under the following
rules:
   - the vfs code locks the mount point of the fs before calling sync
     and unmount, so if the fs is not busy nothing can enter it;
   - the first thing the fs does in unmount is check if it's busy, and
     if so unmount fails;
   - if the fs is not busy there are no outstanding vnode references,
     so no more can be generated because of the vfs-level locking;
   - therefore unmount can proceed knowing both that the fs is already
     synced and that nothing can make it dirty again.

This works reasonably well; however, I don't know without wading in
how much relevance it might have to the way things do or might be made
to work in netbsd.

Offhand it seems to me that since the vnode cache is now fully
vfs-level (thanks to hannken@, who's been doing all my work for me for
like the last two years) that the vfs code now ought to be able to
handle the busy check itself too. But I have no immediate idea how
feasible that really is.

-- 
David A. Holland
dholland%netbsd.org@localhost