Re: [PATCH 6/6] nfsd: add nfsd_file_acquire_local().

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2024-07-02 at 09:55 +1000, NeilBrown wrote:
> On Mon, 01 Jul 2024, Jeff Layton wrote:
> > 
> > Neil, in an earlier email you mentioned that the client could hold onto
> > the nfsd_file reference over several operations and then put it. That
> > would be fine too, but I'm unclear on how the client will manage this.
> > Does the client have a way to keep track of the nfsd_file over several
> > operations to the same inode?
> 
> Looking at 
>    [PATCH v10 13/19] nfs: add "localio" support
> 
> you can see that 
>    struct file *local_filp;
> is added to "struct nfs_open_context".  An nfs_open_context is stored
> in file->private_data and is attached to the inode via nfsi->open_files.
> It holds the nfs state for any file open on the NFS filesystem..
> 
> ->local_filp is set the first time nfs_local_file_open_cache() is called
> and remains set until the final __put_nfs_open_context() call destroys
> the context.  So it lasts as long as the NFS file is open.  Note that
> only one successful ->nfsd_open_local_fh() call is made for each opened
> NFS file.  All IO then uses the "struct file*" with no further reference
> to nfsd.
> 
> If we stored an nfsd_file in the nfs_open_context, either as well as
> the 'struct file*' or instead of, then we could call nfsd_file_put()
> when the nfs file is closed.  That seems to be the correct lifetime and
> matches (almost) exactly what happens with NFSv4 where OPEN and CLOSE
> are send over the wire.
> 

Ok.

> > 
> > Even then, I still think we're probably better off just garbage
> > collecting thse, since it seems likely that they will end up being
> > reused in many cases.
> 
> Why does this logic apply to localio, but not to normal NFSv4 access?
> 

The main idea with the filecache was to keep open files around for a
little while to reduce the open/close overhead between v3 RPCs. The
argument for not caching files with v4 is that we have an open stateid
that pins the open file in place, so we don't need to do that there.

Hanging an nfsd_file off of the nfs client's open_context sounds like a
reasonable alternative.
-- 
Jeff Layton <jlayton@kernel.org>