2022-12-22 15:06:08

by Jeff Layton

Subject: [PATCH] nfsd: shut down the NFSv4 state objects before the filecache

Currently, we shut down the filecache before trying to clean up the
stateids that depend on it. This leads to the kernel trying to free an
nfsd_file twice, and a refcount overput on the nf_mark.

Change the shutdown procedure to tear down all of the stateids prior
to shutting down the filecache.

Reported-and-Tested-by: Wang Yugui <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfssvc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 56fba1cba3af..325d3d3f1211 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -453,8 +453,8 @@ static void nfsd_shutdown_net(struct net *net)
{
struct nfsd_net *nn = net_generic(net, nfsd_net_id);

- nfsd_file_cache_shutdown_net(net);
nfs4_state_shutdown_net(net);
+ nfsd_file_cache_shutdown_net(net);
if (nn->lockd_up) {
lockd_down(net);
nn->lockd_up = false;
--
2.38.1

2022-12-22 15:06:51

by Chuck Lever III

Subject: Re: [PATCH] nfsd: shut down the NFSv4 state objects before the filecache

> On Dec 22, 2022, at 9:51 AM, Jeff Layton <[email protected]> wrote:
>
> Currently, we shut down the filecache before trying to clean up the
> stateids that depend on it. This leads to the kernel trying to free an
> nfsd_file twice, and a refcount overput on the nf_mark.
>
> Change the shutdown procedure to tear down all of the stateids prior
> to shutting down the filecache.
>
> Reported-and-Tested-by: Wang Yugui <[email protected]>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/nfsd/nfssvc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 56fba1cba3af..325d3d3f1211 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -453,8 +453,8 @@ static void nfsd_shutdown_net(struct net *net)
> {
> struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>
> - nfsd_file_cache_shutdown_net(net);
> nfs4_state_shutdown_net(net);
> + nfsd_file_cache_shutdown_net(net);
> if (nn->lockd_up) {
> lockd_down(net);
> nn->lockd_up = false;
> --
> 2.38.1
>

Hi Jeff, seems sensible. May I add:

Fixes: 5e113224c17e ("nfsd: nfsd_file cache entries should be per net namespace")

--
Chuck Lever

2022-12-22 15:12:47

by Jeff Layton

Subject: Re: [PATCH] nfsd: shut down the NFSv4 state objects before the filecache

On Thu, 2022-12-22 at 14:55 +0000, Chuck Lever III wrote:
>
> > On Dec 22, 2022, at 9:51 AM, Jeff Layton <[email protected]> wrote:
> >
> > Currently, we shut down the filecache before trying to clean up the
> > stateids that depend on it. This leads to the kernel trying to free an
> > nfsd_file twice, and a refcount overput on the nf_mark.
> >
> > Change the shutdown procedure to tear down all of the stateids prior
> > to shutting down the filecache.
> >
> > Reported-and-Tested-by: Wang Yugui <[email protected]>
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> > fs/nfsd/nfssvc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index 56fba1cba3af..325d3d3f1211 100644
> > --- a/fs/nfsd/nfssvc.c
> > +++ b/fs/nfsd/nfssvc.c
> > @@ -453,8 +453,8 @@ static void nfsd_shutdown_net(struct net *net)
> > {
> > struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> >
> > - nfsd_file_cache_shutdown_net(net);
> > nfs4_state_shutdown_net(net);
> > + nfsd_file_cache_shutdown_net(net);
> > if (nn->lockd_up) {
> > lockd_down(net);
> > nn->lockd_up = false;
> > --
> > 2.38.1
> >
>
> Hi Jeff, seems sensible. May I add:
>
> Fixes: 5e113224c17e ("nfsd: nfsd_file cache entries should be per net namespace")
>

Yes, thanks.
--
Jeff Layton <[email protected]>