| Message ID | 20180530180553.38769-5-trond.myklebust@hammerspace.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Trond, This feature is enabled for Flexfiles layout type. Is there a reason that it shouldn't be generic for all pnfs? On Wed, May 30, 2018 at 2:05 PM, Trond Myklebust <trondmy@gmail.com> wrote: > From: Fred Isaman <fred.isaman@gmail.com> > > Driver can set flag to allow LAYOUTGET to be sent with OPEN. > > Signed-off-by: Fred Isaman <fred.isaman@gmail.com> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > --- > fs/nfs/flexfilelayout/flexfilelayout.c | 1 + > fs/nfs/pnfs.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c > index c75ad982bcfc..3ae038d9c292 100644 > --- a/fs/nfs/flexfilelayout/flexfilelayout.c > +++ b/fs/nfs/flexfilelayout/flexfilelayout.c > @@ -2347,6 +2347,7 @@ static struct pnfs_layoutdriver_type flexfilelayout_type = { > .id = LAYOUT_FLEX_FILES, > .name = "LAYOUT_FLEX_FILES", > .owner = THIS_MODULE, > + .flags = PNFS_LAYOUTGET_ON_OPEN, > .set_layoutdriver = ff_layout_set_layoutdriver, > .alloc_layout_hdr = ff_layout_alloc_layout_hdr, > .free_layout_hdr = ff_layout_free_layout_hdr, > diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h > index daf6cbf5c15f..f71a55f11b97 100644 > --- a/fs/nfs/pnfs.h > +++ b/fs/nfs/pnfs.h > @@ -110,6 +110,7 @@ enum layoutdriver_policy_flags { > PNFS_LAYOUTRET_ON_SETATTR = 1 << 0, > PNFS_LAYOUTRET_ON_ERROR = 1 << 1, > PNFS_READ_WHOLE_PAGE = 1 << 2, > + PNFS_LAYOUTGET_ON_OPEN = 1 << 3, > }; > > struct nfs4_deviceid_node; > -- > 2.17.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
T24gV2VkLCAyMDE4LTA1LTMwIGF0IDE2OjEwIC0wNDAwLCBPbGdhIEtvcm5pZXZza2FpYSB3cm90 ZToNCj4gSGkgVHJvbmQsDQo+IA0KPiBUaGlzIGZlYXR1cmUgaXMgZW5hYmxlZCBmb3IgRmxleGZp bGVzIGxheW91dCB0eXBlLiBJcyB0aGVyZSBhIHJlYXNvbg0KPiB0aGF0IGl0IHNob3VsZG4ndCBi ZSBnZW5lcmljIGZvciBhbGwgcG5mcz8NCg0KWWVzIGFuZCBuby4gSXQgcmVsaWVzIG9uIHVzZSBv ZiB0aGUgImN1cnJlbnQgc3RhdGVpZCIgZmVhdHVyZSwgd2hpY2gNCmhhcyBub3QgcHJldmlvdXNs eSBzZWVuIGhlYXZ5IHVzZSwgYW5kIHRoZSBhYmlsaXR5IG9mIHRoZSBwTkZTIHNlcnZlcg0KdG8g aGFuZGxlIGxheW91dGdldCByYWNlcyBjb3JyZWN0bHkgKHdoZW4gdGhlIGNsaWVudCBzZW5kcyAy IGxheW91dGdldA0KcmVxdWVzdHMgaW4gcGFyYWxsZWwsIGJvdGggdXNpbmcgdGhlIG9wZW4gc3Rh dGVpZCkuIElmIHRoZSBzZXJ2ZXIgaXMNCmludmFsaWRhdGluZyBvbmUgb2YgdGhlIHJlc3VsdGlu ZyBsYXlvdXQgc3RhdGVpZHMgYXMgaXQgc2hvdWxkLCB0aGVuIGl0DQpoYWQgYmV0dGVyIGVpdGhl ciBoYXZlIGEgZ29vZCBmZW5jaW5nIG1lY2hhbmlzbSwgb3IgaXQgbXVzdCByZWNhbGwgdGhhdA0K bGF5b3V0IHN0YXRlaWQgYmVmb3JlIGhhbmRpbmcgb3V0IHRoZSBzZWNvbmQgbGF5b3V0Lg0KDQpG b3IgdGhhdCByZWFzb24sIEknZCBsaWtlIHRvIGVuc3VyZSB0aGF0IHdlIGF0IGxlYXN0IHRlc3Qg dGhlIGV4aXN0aW5nDQpwTkZTIGltcGxlbWVudGF0aW9ucyB0byBlbnN1cmUgd2UgZG9uJ3Qgc2Vl IHJlZ3Jlc3Npb25zIGJlZm9yZSB3ZQ0KZW5hYmxlIHRoZSBmZWF0dXJlLg0KDQoNCj4gT24gV2Vk LCBNYXkgMzAsIDIwMTggYXQgMjowNSBQTSwgVHJvbmQgTXlrbGVidXN0IDx0cm9uZG15QGdtYWls LmNvbT4NCj4gd3JvdGU6DQo+ID4gRnJvbTogRnJlZCBJc2FtYW4gPGZyZWQuaXNhbWFuQGdtYWls LmNvbT4NCj4gPiANCj4gPiBEcml2ZXIgY2FuIHNldCBmbGFnIHRvIGFsbG93IExBWU9VVEdFVCB0 byBiZSBzZW50IHdpdGggT1BFTi4NCj4gPiANCj4gPiBTaWduZWQtb2ZmLWJ5OiBGcmVkIElzYW1h biA8ZnJlZC5pc2FtYW5AZ21haWwuY29tPg0KPiA+IFNpZ25lZC1vZmYtYnk6IFRyb25kIE15a2xl YnVzdCA8dHJvbmQubXlrbGVidXN0QGhhbW1lcnNwYWNlLmNvbT4NCj4gPiAtLS0NCj4gPiAgZnMv bmZzL2ZsZXhmaWxlbGF5b3V0L2ZsZXhmaWxlbGF5b3V0LmMgfCAxICsNCj4gPiAgZnMvbmZzL3Bu ZnMuaCAgICAgICAgICAgICAgICAgICAgICAgICAgfCAxICsNCj4gPiAgMiBmaWxlcyBjaGFuZ2Vk LCAyIGluc2VydGlvbnMoKykNCj4gPiANCj4gPiBkaWZmIC0tZ2l0IGEvZnMvbmZzL2ZsZXhmaWxl bGF5b3V0L2ZsZXhmaWxlbGF5b3V0LmMNCj4gPiBiL2ZzL25mcy9mbGV4ZmlsZWxheW91dC9mbGV4 ZmlsZWxheW91dC5jDQo+ID4gaW5kZXggYzc1YWQ5ODJiY2ZjLi4zYWUwMzhkOWMyOTIgMTAwNjQ0 DQo+ID4gLS0tIGEvZnMvbmZzL2ZsZXhmaWxlbGF5b3V0L2ZsZXhmaWxlbGF5b3V0LmMNCj4gPiAr KysgYi9mcy9uZnMvZmxleGZpbGVsYXlvdXQvZmxleGZpbGVsYXlvdXQuYw0KPiA+IEBAIC0yMzQ3 LDYgKzIzNDcsNyBAQCBzdGF0aWMgc3RydWN0IHBuZnNfbGF5b3V0ZHJpdmVyX3R5cGUNCj4gPiBm bGV4ZmlsZWxheW91dF90eXBlID0gew0KPiA+ICAgICAgICAgLmlkICAgICAgICAgICAgICAgICAg ICAgPSBMQVlPVVRfRkxFWF9GSUxFUywNCj4gPiAgICAgICAgIC5uYW1lICAgICAgICAgICAgICAg ICAgID0gIkxBWU9VVF9GTEVYX0ZJTEVTIiwNCj4gPiAgICAgICAgIC5vd25lciAgICAgICAgICAg ICAgICAgID0gVEhJU19NT0RVTEUsDQo+ID4gKyAgICAgICAuZmxhZ3MgICAgICAgICAgICAgICAg ICA9IFBORlNfTEFZT1VUR0VUX09OX09QRU4sDQo+ID4gICAgICAgICAuc2V0X2xheW91dGRyaXZl ciAgICAgICA9IGZmX2xheW91dF9zZXRfbGF5b3V0ZHJpdmVyLA0KPiA+ICAgICAgICAgLmFsbG9j X2xheW91dF9oZHIgICAgICAgPSBmZl9sYXlvdXRfYWxsb2NfbGF5b3V0X2hkciwNCj4gPiAgICAg ICAgIC5mcmVlX2xheW91dF9oZHIgICAgICAgID0gZmZfbGF5b3V0X2ZyZWVfbGF5b3V0X2hkciwN Cj4gPiBkaWZmIC0tZ2l0IGEvZnMvbmZzL3BuZnMuaCBiL2ZzL25mcy9wbmZzLmgNCj4gPiBpbmRl eCBkYWY2Y2JmNWMxNWYuLmY3MWE1NWYxMWI5NyAxMDA2NDQNCj4gPiAtLS0gYS9mcy9uZnMvcG5m cy5oDQo+ID4gKysrIGIvZnMvbmZzL3BuZnMuaA0KPiA+IEBAIC0xMTAsNiArMTEwLDcgQEAgZW51 bSBsYXlvdXRkcml2ZXJfcG9saWN5X2ZsYWdzIHsNCj4gPiAgICAgICAgIFBORlNfTEFZT1VUUkVU X09OX1NFVEFUVFIgICAgICAgPSAxIDw8IDAsDQo+ID4gICAgICAgICBQTkZTX0xBWU9VVFJFVF9P Tl9FUlJPUiAgICAgICAgID0gMSA8PCAxLA0KPiA+ICAgICAgICAgUE5GU19SRUFEX1dIT0xFX1BB R0UgICAgICAgICAgICA9IDEgPDwgMiwNCj4gPiArICAgICAgIFBORlNfTEFZT1VUR0VUX09OX09Q RU4gICAgICAgICAgPSAxIDw8IDMsDQo+ID4gIH07DQo+ID4gDQo+ID4gIHN0cnVjdCBuZnM0X2Rl dmljZWlkX25vZGU7DQo+ID4gLS0NCj4gPiAyLjE3LjANCj4gPiANCj4gPiAtLQ0KPiA+IFRvIHVu c3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51 eC0NCj4gPiBuZnMiIGluDQo+ID4gdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2 Z2VyLmtlcm5lbC5vcmcNCj4gPiBNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBodHRwOi8vdmdlci5r ZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwNCj4gDQo+IC0tDQo+IFRvIHVuc3Vic2NyaWJl IGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51eC1uZnMiDQo+ IGluDQo+IHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3Jn DQo+IE1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFqb3Jk b21vLWluZm8uaHRtbA0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFp bnRhaW5lciwgSGFtbWVyc3BhY2UNCnRyb25kLm15a2xlYnVzdEBoYW1tZXJzcGFjZS5jb20NCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 31, 2018 at 8:41 AM Trond Myklebust <trondmy@hammerspace.com> wrote: > > On Wed, 2018-05-30 at 16:10 -0400, Olga Kornievskaia wrote: > > Hi Trond, > > > > This feature is enabled for Flexfiles layout type. Is there a reason > > that it shouldn't be generic for all pnfs? > > Yes and no. It relies on use of the "current stateid" feature, which > has not previously seen heavy use, and the ability of the pNFS server > to handle layoutget races correctly (when the client sends 2 layoutget > requests in parallel, both using the open stateid). If the server is > invalidating one of the resulting layout stateids as it should, then it > had better either have a good fencing mechanism, or it must recall that > layout stateid before handing out the second layout. > > For that reason, I'd like to ensure that we at least test the existing > pNFS implementations to ensure we don't see regressions before we > enable the feature. Hi Trond, I'm getting back to trying to add LAYOUTGET to the OPEN compound for the file layout type. I'm looking at your reply and trying to figure out what kind of testing I should try to do. You mention a race where a client sends 2 layoutget requests in parallel. You say "using the open stateid", but when the LAYOUTGET is added to the OPEN, it'll be using as you said current stateid. So is the problem sending 2 concurrent OPENs for the same file (same owner)? Then server should issue the reply to the 1st, then a layout recall and reply to the 2nd? But for the testing is your suggestion to instrument sending 2 concurrent opens with the new code and see that happens? Thank you. > > On Wed, May 30, 2018 at 2:05 PM, Trond Myklebust <trondmy@gmail.com> > > wrote: > > > From: Fred Isaman <fred.isaman@gmail.com> > > > > > > Driver can set flag to allow LAYOUTGET to be sent with OPEN. > > > > > > Signed-off-by: Fred Isaman <fred.isaman@gmail.com> > > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > > > --- > > > fs/nfs/flexfilelayout/flexfilelayout.c | 1 + > > > fs/nfs/pnfs.h | 1 + > > > 2 files changed, 2 insertions(+) > > > > > > diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c > > > b/fs/nfs/flexfilelayout/flexfilelayout.c > > > index c75ad982bcfc..3ae038d9c292 100644 > > > --- a/fs/nfs/flexfilelayout/flexfilelayout.c > > > +++ b/fs/nfs/flexfilelayout/flexfilelayout.c > > > @@ -2347,6 +2347,7 @@ static struct pnfs_layoutdriver_type > > > flexfilelayout_type = { > > > .id = LAYOUT_FLEX_FILES, > > > .name = "LAYOUT_FLEX_FILES", > > > .owner = THIS_MODULE, > > > + .flags = PNFS_LAYOUTGET_ON_OPEN, > > > .set_layoutdriver = ff_layout_set_layoutdriver, > > > .alloc_layout_hdr = ff_layout_alloc_layout_hdr, > > > .free_layout_hdr = ff_layout_free_layout_hdr, > > > diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h > > > index daf6cbf5c15f..f71a55f11b97 100644 > > > --- a/fs/nfs/pnfs.h > > > +++ b/fs/nfs/pnfs.h > > > @@ -110,6 +110,7 @@ enum layoutdriver_policy_flags { > > > PNFS_LAYOUTRET_ON_SETATTR = 1 << 0, > > > PNFS_LAYOUTRET_ON_ERROR = 1 << 1, > > > PNFS_READ_WHOLE_PAGE = 1 << 2, > > > + PNFS_LAYOUTGET_ON_OPEN = 1 << 3, > > > }; > > > > > > struct nfs4_deviceid_node; > > > -- > > > 2.17.0 > > > > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux- > > > nfs" in > > > the body of a message to majordomo@vger.kernel.org > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" > > in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com >
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c index c75ad982bcfc..3ae038d9c292 100644 --- a/fs/nfs/flexfilelayout/flexfilelayout.c +++ b/fs/nfs/flexfilelayout/flexfilelayout.c @@ -2347,6 +2347,7 @@ static struct pnfs_layoutdriver_type flexfilelayout_type = { .id = LAYOUT_FLEX_FILES, .name = "LAYOUT_FLEX_FILES", .owner = THIS_MODULE, + .flags = PNFS_LAYOUTGET_ON_OPEN, .set_layoutdriver = ff_layout_set_layoutdriver, .alloc_layout_hdr = ff_layout_alloc_layout_hdr, .free_layout_hdr = ff_layout_free_layout_hdr, diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h index daf6cbf5c15f..f71a55f11b97 100644 --- a/fs/nfs/pnfs.h +++ b/fs/nfs/pnfs.h @@ -110,6 +110,7 @@ enum layoutdriver_policy_flags { PNFS_LAYOUTRET_ON_SETATTR = 1 << 0, PNFS_LAYOUTRET_ON_ERROR = 1 << 1, PNFS_READ_WHOLE_PAGE = 1 << 2, + PNFS_LAYOUTGET_ON_OPEN = 1 << 3, }; struct nfs4_deviceid_node;