Remove VFS_SYNC in dounmount() (was: Re: [PATCH] Fixing soft NFS umount -f, round 5)

[manu%netbsd.org@localhost (Emmanuel Dreyfus)]

Anyone has an idea why we need this VFS_SYNC() before VFS_UNMOUNT() in
dounmount()? As noted by Chuck below, this seems to just duplicate
functionality. And it is at the expense of disturbing force unmount
since the VFS_SYNC ignoes the MNT_FORCE flags and can make us wait for a
while.

Chuck Silvers <chuq%chuq.com@localhost> 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.
> 
> 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.


-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu%netbsd.org@localhost