On Tue, 2018-03-20 at 13:35 +0000, David Howells wrote:
> J. Bruce Fields <bfields@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx
