From: Trond Myklebust <trondmy@primarydata.com>
To: "bfields@redhat.com" <bfields@redhat.com>,
"dhowells@redhat.com" <dhowells@redhat.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH 10/10] nfsd: clients don't need to break their own delegations
Date: Tue, 20 Mar 2018 13:46:20 +0000 [thread overview]
Message-ID: <1521553578.10293.4.camel@primarydata.com> (raw)
In-Reply-To: <29167.1521552951@warthog.procyon.org.uk>
T24gVHVlLCAyMDE4LTAzLTIwIGF0IDEzOjM1ICswMDAwLCBEYXZpZCBIb3dlbGxzIHdyb3RlOg0K
PiBKLiBCcnVjZSBGaWVsZHMgPGJmaWVsZHNAcmVkaGF0LmNvbT4gd3JvdGU6DQo+IA0KPiA+IEBA
IC0xMzksNiArMTM5LDkgQEAgc3RydWN0IGNyZWQgew0KPiA+ICAJc3RydWN0IGtleQkqdGhyZWFk
X2tleXJpbmc7IC8qIGtleXJpbmcgcHJpdmF0ZSB0bw0KPiA+IHRoaXMgdGhyZWFkICovDQo+ID4g
IAlzdHJ1Y3Qga2V5CSpyZXF1ZXN0X2tleV9hdXRoOyAvKiBhc3N1bWVkDQo+ID4gcmVxdWVzdF9r
ZXkgYXV0aG9yaXR5ICovDQo+ID4gICNlbmRpZg0KPiA+ICsjaWZkZWYgQ09ORklHX0ZJTEVfTE9D
S0lORw0KPiA+ICsJdm9pZAkJKmxlYXNlX2JyZWFrZXI7IC8qIGlkZW50aWZ5IE5GUyBjbGllbnQN
Cj4gPiBicmVha2luZyBhIGRlbGVnYXRpb24gKi8NCj4gPiArI2VuZGlmDQo+ID4gICNpZmRlZiBD
T05GSUdfU0VDVVJJVFkNCj4gPiAgCXZvaWQJCSpzZWN1cml0eTsJLyogc3ViamVjdGl2ZSBMU00N
Cj4gPiBzZWN1cml0eSAqLw0KPiA+ICAjZW5kaWYNCj4gDQo+IFNvcnJ5LCBidXQgZXd3dy4NCj4g
DQo+IFR3byByZWFzb25zIGZvciB0aGF0IGNvbW1lbnQ6DQo+IA0KPiAgKDEpIFRoZSBjcmVkIHN0
cnVjdCBtYXkgZ2V0IHJldGFpbmVkIGxvbmcgcGFzdCB3aGVyZSB5b3UgZXhwZWN0IGlmDQo+IGl0
IGdldHMNCj4gICAgICBhdHRhY2hlZCB0byBhbm90aGVyIHByb2Nlc3Mgb3IgYSBmaWxlIGRlc2Ny
aXB0b3IuDQo+IA0KPiAgKDIpIFRoZSAtPmxlYXNlX2JyZWFrZXIgcG9pbnRlciBuZWVkcyBsaWZl
dGltZSBtYW5hZ2VtZW50IGluDQo+IGNyZWQuYy4gIEl0IHdpbGwNCj4gICAgICBwb3RlbnRpYWxs
eSBnZXQgY29waWVkIGFyb3VuZCBhbmQgbWF5IG5lZWQgY2xlYW5pbmcgdXAuDQo+IA0KPiBDYW4g
eW91IHN0aWNrIHlvdXIgYnJlYWtlciBpZGVudGl0eSBpbiBhIGtleSBzdHJ1Y3QgYXMgSmVmZg0K
PiBzdWdnZXN0ZWQ/DQo+IA0KDQpCcnVjZSwNCg0KRG8geW91IHJlYWxseSBuZWVkIHRvIGRvIG1v
cmUgdGhhbiBqdXN0IGlkZW50aWZ5IHRoYXQgdGhpcyBpcyBhIGtuZnNkDQp0aHJlYWQgdnMgbm90
IGEga25mc2QgdGhyZWFkPyBJJ20gYXNzdW1pbmcgdGhhdCBhIGtuZnNkIHRocmVhZCB3aWxsDQp1
c3VhbGx5IGJlIGluIGEgcG9zaXRpb24gdG8gcmVjYWxsIGRlbGVnYXRpb25zIGJlZm9yZSBpdCBl
dmVuIGluaXRpYXRlcw0KYW4gb3BlcmF0aW9uIG9uIHRoZSBpbm9kZSBpbiBxdWVzdGlvbiwgd29u
J3QgaXQ/DQoNCklPVzogd2hhdCBpZiB5b3Ugd2VyZSB0byBtb2RpZnkgdGhlIGxlYXNlIGNvZGUg
dG8gYWxsb3cga25mc2QgdGhyZWFkcw0KdG8gcmV0dXJuIGEgInBsZWFzZSBpZ25vcmUgbWUsIGFu
ZCBwcm9jZWVkIHdpdGggdGhlIG9wZXJhdGlvbiB0aGF0DQp0cmlnZ2VyZWQgdGhlIGxlYXNlIGJy
ZWFrIiByZXBseSwgYW5kIHRoZW4gaGFuZGxlIGNvbmZsaWN0cyBiZXR3ZWVuIE5GUw0KY2xpZW50
cyBvdXRzaWRlIHRoZSBsZWFzZSBjYWxsYmFjayBjb2RlIGFsdG9nZXRoZXI/DQoNCkNoZWVycw0K
ICBUcm9uZA0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5l
ciwgUHJpbWFyeURhdGENCnRyb25kLm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20NCg==
WARNING: multiple messages have this Message-ID (diff)
From: Trond Myklebust <trondmy@primarydata.com>
To: "bfields@redhat.com" <bfields@redhat.com>,
"dhowells@redhat.com" <dhowells@redhat.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH 10/10] nfsd: clients don't need to break their own delegations
Date: Tue, 20 Mar 2018 13:46:20 +0000 [thread overview]
Message-ID: <1521553578.10293.4.camel@primarydata.com> (raw)
In-Reply-To: <29167.1521552951@warthog.procyon.org.uk>
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
next prev parent reply other threads:[~2018-03-20 13:46 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-19 14:36 [PATCH 00/10] Eliminate delegation self-conflicts v2 J. Bruce Fields
2018-03-19 14:36 ` [PATCH 01/10] vfs: remove unnecessary fl_owner_t typedef J. Bruce Fields
2018-03-19 14:36 ` J. Bruce Fields
2018-03-19 14:36 ` [PATCH 02/10] nfsd: simplify put of fi_deleg_file J. Bruce Fields
2018-03-19 14:36 ` [PATCH 03/10] nfsd: simplify nfs4_put_deleg_lease calls J. Bruce Fields
2018-03-19 14:36 ` [PATCH 04/10] nfsd4: set fl_owner to delegation, not file pointer J. Bruce Fields
2018-03-19 14:36 ` [PATCH 05/10] nfsd4: dp->dl_stid.sc_file doesn't need locking J. Bruce Fields
2018-03-19 14:36 ` [PATCH 06/10] nfsd: make nfs4_get_existing_delegation less confusing J. Bruce Fields
2018-03-19 14:36 ` [PATCH 07/10] nfsd: factor out common delegation-destruction code J. Bruce Fields
2018-03-19 14:36 ` [PATCH 08/10] nfsd: move sc_file assignment into alloc_init_deleg J. Bruce Fields
2018-03-19 14:36 ` [PATCH 09/10] nfsd: create a separate lease for each delegation J. Bruce Fields
2018-03-19 14:36 ` [PATCH 10/10] nfsd: clients don't need to break their own delegations J. Bruce Fields
2018-03-20 13:35 ` David Howells
2018-03-20 13:46 ` Trond Myklebust [this message]
2018-03-20 13:46 ` Trond Myklebust
2018-03-20 14:49 ` J. Bruce Fields
2018-03-20 15:13 ` Trond Myklebust
2018-03-20 15:13 ` Trond Myklebust
2018-03-20 16:02 ` bfields
2018-09-06 19:40 ` bfields
2018-03-20 14:52 ` J. Bruce Fields
2018-03-20 13:10 ` [PATCH 00/10] Eliminate delegation self-conflicts v2 Jeff Layton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1521553578.10293.4.camel@primarydata.com \
--to=trondmy@primarydata.com \
--cc=bfields@redhat.com \
--cc=dhowells@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.