Re: [PATCH 0/5 v2] sunrpc: stop refcounting svc_serv

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2023-12-15 at 15:37 +0000, Chuck Lever III wrote:
> > On Dec 15, 2023, at 9:38 AM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> > 
> > > On Dec 15, 2023, at 9:26 AM, Jeff Layton <jlayton@kernel.org> wrote:
> > > 
> > > On Fri, 2023-12-15 at 14:19 +0000, Chuck Lever III wrote:
> > > > > On Dec 15, 2023, at 5:59 AM, Jeff Layton <jlayton@kernel.org> wrote:
> > > > > 
> > > > > On Fri, 2023-12-15 at 11:56 +1100, NeilBrown wrote:
> > > > > > I sent an earlier version of this series, got some feed back, revised
> > > > > > it, but never sent it again.  Sorry.
> > > > > > 
> > > > > > The main feedback was around the interaction between sunrpc and nfsd for
> > > > > > handling poolstats.  I have changed that so that nfsd tells sunrpc where
> > > > > > the svc_serv pointer lives, and where to find a mutex to protect it.
> > > > > > sunrpc then taks the mutex and accesses the pointer - if not NULL.  I
> > > > > > think this is nicer than the version that pass around funciton pointers.
> > > > > > 
> > > > > > This series is against nfsd-next
> > > > > > 
> > > > > > Thanks,
> > > > > > NeilBrown
> > > > > > 
> > > > > > 
> > > > > > [PATCH 1/5] nfsd: call nfsd_last_thread() before final nfsd_put()
> > > > > > [PATCH 2/5] svc: don't hold reference for poolstats, only mutex.
> > > > > > [PATCH 3/5] nfsd: hold nfsd_mutex across entire netlink operation
> > > > > > [PATCH 4/5] SUNRPC: discard sv_refcnt, and svc_get/svc_put
> > > > > > [PATCH 5/5] nfsd: rename nfsd_last_thread() to nfsd_destroy_serv()
> > > > > 
> > > > > I'm not sure patch #2 is better than the version with function pointers,
> > > > > but it seems reasonable.
> > > > > 
> > > > > Note that patch #1 probably needs to go to v6.6 stable, and I think we
> > > > > want #3 in v6.7 before it ships.
> > > > 
> > > > Remind me why #3 should go into v6.7-rc ? There's no Fixes tag on
> > > > that one.
> > > > 
> > > > 
> > > 
> > > It's the problem I noted to Lorenzo the other day:
> > > 
> > > 
> > > https://lore.kernel.org/linux-nfs/5d9bbb599569ce29f16e4e0eef6b291eda0f375b.camel@kernel.org/T/#u
> > > 
> > > Once you've dropped the nfsd_mutex, there is no guarantee that
> > > nn->nfsd_serv will still be a valid pointer. Holding the mutex across
> > > the operation (like Neil's patch does), should close the race.
> > 
> > OK. I'll add:
> > 
> >  Fixes: bd9d6a3efa97 ("NFSD: add rpc_status netlink support")
> > 
> > I will apply 1/3 and 3/3 to v6.7-rc, and the others will go to
> > v6.8 (nfsd-next) once it is rebased on v6.7-rc7.
> 
> Please check the two patches at the tip of the nfsd-fixes
> branch here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
> 
> I plan to apply the other three in this series to nfsd-next.
> 
> 

They look good to me.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>