On Mon, Nov 20, 2023 at 09:47:03AM +1100, NeilBrown wrote:
> On Sat, 18 Nov 2023, Chuck Lever wrote:
> > On Fri, Nov 17, 2023 at 01:18:52PM +1100, NeilBrown wrote:
> > > For NFSv4.1 and later the client easily discovers if there is any
> > > admin-revoked state and will then find and explicitly free it.
> > >
> > > For NFSv4.0 there is no such mechanism. The client can only find that
> > > state is admin-revoked if it tries to use that state, and there is no
> > > way for it to explicitly free the state. So the server must hold on to
> > > the stateid (at least) for an indefinite amount of time. A
> > > RELEASE_LOCKOWNER request might justify forgetting some of these
> > > stateids, as would the whole clients lease lapsing, but these are not
> > > reliable.
> > >
> > > This patch takes two approaches.
> > >
> > > Whenever a client uses an revoked stateid, that stateid is then
> > > discarded and will not be recognised again. This might confuse a client
> > > which expect to get NFS4ERR_ADMIN_REVOKED consistently once it get it at
> > > all, but should mostly work. Hopefully one error will lead to other
> > > resources being closed (e.g. process exits), which will result in more
> > > stateid being freed when a CLOSE attempt gets NFS4ERR_ADMIN_REVOKED.
> > >
> > > Also, any admin-revoked stateids that have been that way for more than
> > > one lease time are periodically revoke.
> > >
> > > No actual freeing of state happens in this patch. That will come in
> > > future patches which handle the different sorts of revoked state.
> > >
> > > Signed-off-by: NeilBrown <neilb@xxxxxxx>
> > > ---
> > > fs/nfsd/netns.h | 4 ++
> > > fs/nfsd/nfs4state.c | 97 ++++++++++++++++++++++++++++++++++++++++++++-
> > > 2 files changed, 100 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> > > index ec49b200b797..02f8fa095b0f 100644
> > > --- a/fs/nfsd/netns.h
> > > +++ b/fs/nfsd/netns.h
> > > @@ -197,6 +197,10 @@ struct nfsd_net {
> > > atomic_t nfsd_courtesy_clients;
> > > struct shrinker nfsd_client_shrinker;
> > > struct work_struct nfsd_shrinker_work;
> > > +
> > > + /* last time an admin-revoke happened for NFSv4.0 */
> > > + time64_t nfs40_last_revoke;
> > > +
> > > };
> >
> > This hunk doesn't apply to nfsd-next due to v6.7-rc changes to NFSD
> > to implement a dynamic shrinker. So I stopped my review here for
> > now.
>
> I didn't a rebase onto nfsd-next and there were no conflicts!
>
> I guess the change from
> struct work_struct nfsd_shrinker_work;
> to
> struct work_struct *nfsd_shrinker_work;
>
> was technically a conflict but I'm surprised your tool complained..
That was the change I noticed when it failed to apply. "stg import"
is pretty picky, unfortunately...
--
Chuck Lever
