Re: [PATCH v2 03/18] nfsd: convert laundry_wq to something less nfsd4 specific

["J. Bruce Fields" <bfields@fieldses.org>]

On Mon, Aug 10, 2015 at 08:14:56AM -0400, Jeff Layton wrote:
> On Mon, 10 Aug 2015 05:10:35 -0700
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > On Mon, Aug 10, 2015 at 07:23:51AM -0400, Jeff Layton wrote:
> > > I did consider a shrinker and LRU list for this. The problem there is
> > > that shrinkers are triggered on memory pressure. Keeping these files
> > > open after they've been idle for a long period of time would prevent
> > > the kernel from handing out leases on them, so closing them after a
> > > reasonable idle period seemed like the right thing to do.
> > 
> > True.
> > 
> > > I suppose however we could use a shrinker/LRU _and_ add a mechanism
> > > that would cause the kernel to close idle nfsd_files for an inode when
> > > there is an attempt to do a F_SETLEASE. That would probably work,
> > > unless I'm missing other reasons that keeping unused files open might
> > > be problematic. Are there any?
> > 
> > That seems reasonable.  Keepign the file open also will prevent
> > unmounting the file system, although currently any NFS export already
> > causes that as well.
> 
> Yes, though that's the reason for the new ->flush hook in the sunrpc
> cache code. On any export table change, we'll clean out the nfsd_file
> cache to help ensure that you'll be able to unmount soon after
> unexporting a filesystem.

There are definitely people with scripts that try to unexport and then
immediately unmount, e.g. to migrate a filesystem elsewhere.  They
already run into problems thanks to export caches, locks, and v4 state.
A complete shutdown of nfsd is currently the only supported way to
unmount.  Still, I wouldn't be surprised if there are people who
(possibly just out of luck) have a working setup now that will start
failing after we take these additional references.

Extending the unlock_* interfaces or getting Kinglong's stuff working
would help.

--b.

> 
> In any case, I'll look at the shrinker/lru thing for the next respin
> and see whether adding a hook into the setlease code might be
> reasonable.
> 
> Thanks for the review so far,
> -- 
> Jeff Layton <jlayton@poochiereds.net>