Re: [PATCH 10/10] nfsd: clients don't need to break their own delegations

[Trond Myklebust <trondmy@primarydata.com>]

On Tue, 2018-03-20 at 13:35 +0000, David Howells wrote:
> J. Bruce Fields <bfields@redhat.com> wrote:
> 
> > @@ -139,6 +139,9 @@ struct cred {
> >  	struct key	*thread_keyring; /* keyring private to
> > this thread */
> >  	struct key	*request_key_auth; /* assumed
> > request_key authority */
> >  #endif
> > +#ifdef CONFIG_FILE_LOCKING
> > +	void		*lease_breaker; /* identify NFS client
> > breaking a delegation */
> > +#endif
> >  #ifdef CONFIG_SECURITY
> >  	void		*security;	/* subjective LSM
> > security */
> >  #endif
> 
> Sorry, but ewww.
> 
> Two reasons for that comment:
> 
>  (1) The cred struct may get retained long past where you expect if
> it gets
>      attached to another process or a file descriptor.
> 
>  (2) The ->lease_breaker pointer needs lifetime management in
> cred.c.  It will
>      potentially get copied around and may need cleaning up.
> 
> Can you stick your breaker identity in a key struct as Jeff
> suggested?
> 

Bruce,

Do you really need to do more than just identify that this is a knfsd
thread vs not a knfsd thread? I'm assuming that a knfsd thread will
usually be in a position to recall delegations before it even initiates
an operation on the inode in question, won't it?

IOW: what if you were to modify the lease code to allow knfsd threads
to return a "please ignore me, and proceed with the operation that
triggered the lease break" reply, and then handle conflicts between NFS
clients outside the lease callback code altogether?

Cheers
  Trond
-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com