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

NeilBrown
2024-07-01T23:56:01Z

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.

>
> 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?

NeilBrown