Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <25454461d696b25482a1b48aed10abda1d1c2060.camel@kernel.org>
Subject: Re: [PATCH v4 2/7] NFSD: Revert "NFSD: NFSv4 CLOSE should release
 an nfsd_file immediately"
From:   Jeff Layton <jlayton@kernel.org>
To:     Chuck Lever <chuck.lever@oracle.com>, linux-nfs@vger.kernel.org
Cc:     neilb@suse.de
Date:   Wed, 19 Oct 2022 06:48:39 -0400
In-Reply-To: <166612311196.1291.3976661000746956975.stgit@manet.1015granger.net>
References: <166612295223.1291.11761205673682408148.stgit@manet.1015granger.net>
         <166612311196.1291.3976661000746956975.stgit@manet.1015granger.net>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Tue, 2022-10-18 at 15:58 -0400, Chuck Lever wrote:
> This reverts commit 5e138c4a750dc140d881dab4a8804b094bbc08d2.
>=20
> That commit attempted to make files available to other users as soon
> as all NFSv4 clients were done with them, rather than waiting until
> the filecache LRU had garbage collected them.
>=20
> It gets the reference counting wrong, for one thing.
>=20
> But it also misses that DELEGRETURN should release a file in the
> same fashion. In fact, any nfsd_file_put() on an file held open
> by an NFSv4 client needs potentially to release the file
> immediately...
>=20
> Clear the way for implementing that idea.
>=20
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/filecache.c |   18 ------------------
>  fs/nfsd/filecache.h |    1 -
>  fs/nfsd/nfs4state.c |    4 ++--
>  3 files changed, 2 insertions(+), 21 deletions(-)
>=20
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index a2adfc247648..b7aa523c2010 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -444,24 +444,6 @@ nfsd_file_put(struct nfsd_file *nf)
>  		nfsd_file_put_noref(nf);
>  }
> =20
> -/**
> - * nfsd_file_close - Close an nfsd_file
> - * @nf: nfsd_file to close
> - *
> - * If this is the final reference for @nf, free it immediately.
> - * This reflects an on-the-wire CLOSE or DELEGRETURN into the
> - * VFS and exported filesystem.
> - */
> -void nfsd_file_close(struct nfsd_file *nf)
> -{
> -	nfsd_file_put(nf);
> -	if (refcount_dec_if_one(&nf->nf_ref)) {
> -		nfsd_file_unhash(nf);
> -		nfsd_file_lru_remove(nf);
> -		nfsd_file_free(nf);
> -	}
> -}
> -
>  struct nfsd_file *
>  nfsd_file_get(struct nfsd_file *nf)
>  {
> diff --git a/fs/nfsd/filecache.h b/fs/nfsd/filecache.h
> index 8e8c0c47d67d..f81c198f4ed6 100644
> --- a/fs/nfsd/filecache.h
> +++ b/fs/nfsd/filecache.h
> @@ -52,7 +52,6 @@ void nfsd_file_cache_shutdown(void);
>  int nfsd_file_cache_start_net(struct net *net);
>  void nfsd_file_cache_shutdown_net(struct net *net);
>  void nfsd_file_put(struct nfsd_file *nf);
> -void nfsd_file_close(struct nfsd_file *nf);
>  struct nfsd_file *nfsd_file_get(struct nfsd_file *nf);
>  void nfsd_file_close_inode_sync(struct inode *inode);
>  bool nfsd_file_is_cached(struct inode *inode);
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c5d199d7e6b4..2b850de288cf 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -820,9 +820,9 @@ static void __nfs4_file_put_access(struct nfs4_file *=
fp, int oflag)
>  			swap(f2, fp->fi_fds[O_RDWR]);
>  		spin_unlock(&fp->fi_lock);
>  		if (f1)
> -			nfsd_file_close(f1);
> +			nfsd_file_put(f1);
>  		if (f2)
> -			nfsd_file_close(f2);
> +			nfsd_file_put(f2);
>  	}
>  }
> =20
>=20
>=20

Reviewed-by: Jeff Layton <jlayton@kernel.org>