diff for duplicates of <1500580582.5457.1.camel@primarydata.com> diff --git a/a/1.txt b/N1/1.txt index 3e67247..2c82348 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,30 +1,43 @@ -SGkgT2xnYSwNCg0KQXBvbG9naWVzIGZvciBtaXNzaW5nIHRoaXMgcGF0Y2guIEl0IHdhcyBoaWRp -bmcgaW4gbXkgJ2xpbnV4LWZzZGV2ZWwnDQptYWlsYm94LCBzbyBJIGRpZG4ndCByZWNvZ25pc2Ug -aXQgYXMgYSBORlMgcGF0Y2guDQoNCk9uIEZyaSwgMjAxNy0wNi0zMCBhdCAxNTo1MiAtMDQwMCwg -T2xnYSBLb3JuaWV2c2thaWEgd3JvdGU6DQo+IFRoZXJlIGlzIGEgcmVncmVzc2lvbiBieSBjb21t -aXQgOGQ0MGIwZjE0ODQ2ICgiTkZTIGZpbGVsYXlvdXQ6Y2FsbA0KPiBHRVRERVZJQ0VJTkZPIGFm -dGVyIHBuZnNfbGF5b3V0X3Byb2Nlc3MgY29tcGxldGVzIikuIEl0IGxlYXZlcyB0aGUNCj4gRFMg -bW91bnQgZGFuZ2xpbmcuDQo+IA0KPiBQcmV2aW91c2x5LCBmaWxlbGF5b3V0X2FsbG9jX3NlYygp -IHdvdWxkIGNhbGwNCj4gZmlsZWxheW91dF9jaGVja19sYXlvdXQoKQ0KPiB3aGljaCB3b3VsZCBj -YWxsIG5mczRfZmluZF9nZXRfZGV2aWNlaWQgd2hpY2ggdXBzIHRoZSBjb3VudCBvbiB0aGUNCj4g -ZGV2aWNlX2lkLiBJdCdzIG9ubHkgY2FsbGVkIG9uY2UgYW5kIGl0J3MgbWF0Y2hlZCBieSB0aGUN -Cj4gZmlsZWxheW91dF9mcmVlX2xzZWcoKSB0aGF0IGNhbGxzIG5mczRfZmxfcHV0X2RldmljZWlk -KCkuDQo+IA0KPiBBZnRlciB0aGF0IHBhdGNoLCBlYWNoIHJlYWQvd3JpdGUgZW5kcyB1cCBjYWxs -aW5nDQo+IG5mczRfZmluZF9nZXRfZGV2aWNlaWQNCj4gYW5kIHRoZXJlIGlzIG5vIGJhbGFuY2Ug -Zm9yIHRoYXQuIEluc3RlYWQsIGRvIG5mczRfZmxfcHV0X2RldmljZWlkKCkNCj4gaW4gdGhlIGZp -bGVsYXlvdXQncyAucGdfY2xlYW51cCBhbmQgcmVtb3ZlIGl0IGZyb20NCj4gZmlsZWxheW91dF9m -cmVlX2xzZWcuDQo+IA0KPiBCdXQgd2Ugc3RpbGwgbmVlZCBhIHJlZmVyZW5jZSB0byBob2xkIG92 -ZXIgdGhlIGxpZmV0aW1lIG9mIHRoZQ0KPiBzZWdtZW50Lg0KPiBGb3IgZXZlcnkgbmV3IGxzZWcg -dGhhdCdzIGNyZWF0ZWQgd2UgbmVlZCB0byB0YWtlIGEgcmVmZXJlbmNlIG9uDQo+IGRldmljZWlk -DQo+IHRoYXQgdXNlcyBpdC4gSXQgd2lsbCBiZSByZWxlYXNlZCBpbiB0aGUgImZyZWVfbHNlZyIg -cm91dGluZS4NCg0KVGhpcyBpcyB3aGF0IEknbSBub3QgdW5kZXJzdGFuZGluZy4gSWYgeW91IGhh -dmUgYSByZWZlcmVuY2UgaW4gdGhlDQpsYXlvdXQgc2VnbWVudCwgdGhlbiB3aHkgZG8geW91IG5l -ZWQgdG8gY2FsbCBuZnM0X2ZpbmRfZ2V0X2RldmljZWlkKCkNCmluIHRoZSByZWFkL3dyaXRlIGNv -ZGU/DQoNCklzbid0IGl0IHN1ZmZpY2llbnQgdG8gY2hhbmdlIHRoZSAicGdfaW5pdCIgY2FsbHMg -dG8gY2hlY2sgd2hldGhlciBvcg0Kbm90IHRoZSBzdHJ1Y3QgbmZzNF9maWxlbGF5b3V0X3NlZ21l -bnQgaGFzIHNldCBhIHZhbHVlIGZvciBkc2FkZHIgKHRoYXQNCm5lZWRzIHRvIGJlIGRvbmUgd2l0 -aCBjYXJlIHRvIGF2b2lkIHJhY2VzIC0gY21weGNoZygpIGlzIHlvdXIgZnJpZW5kKSwNCmFuZCB0 -aGVuIHJlbHkgb24gdGhhdCByZWZlcmVuY2UgYmVpbmcgc2V0IGZvciB0aGUgcmVtYWluZGVyIG9m -IHRoZQ0KbGF5b3V0IHNlZ21lbnQgbGlmZXRpbWU/DQoNCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QN -CkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lciwgUHJpbWFyeURhdGENCnRyb25kLm15a2xlYnVz -dEBwcmltYXJ5ZGF0YS5jb20NCg== +Hi Olga, + +Apologies for missing this patch. It was hiding in my 'linux-fsdevel' +mailbox, so I didn't recognise it as a NFS patch. + +On Fri, 2017-06-30 at 15:52 -0400, Olga Kornievskaia wrote: +> There is a regression by commit 8d40b0f14846 ("NFS filelayout:call +> GETDEVICEINFO after pnfs_layout_process completes"). It leaves the +> DS mount dangling. +> +> Previously, filelayout_alloc_sec() would call +> filelayout_check_layout() +> which would call nfs4_find_get_deviceid which ups the count on the +> device_id. It's only called once and it's matched by the +> filelayout_free_lseg() that calls nfs4_fl_put_deviceid(). +> +> After that patch, each read/write ends up calling +> nfs4_find_get_deviceid +> and there is no balance for that. Instead, do nfs4_fl_put_deviceid() +> in the filelayout's .pg_cleanup and remove it from +> filelayout_free_lseg. +> +> But we still need a reference to hold over the lifetime of the +> segment. +> For every new lseg that's created we need to take a reference on +> deviceid +> that uses it. It will be released in the "free_lseg" routine. + +This is what I'm not understanding. If you have a reference in the +layout segment, then why do you need to call nfs4_find_get_deviceid() +in the read/write code? + +Isn't it sufficient to change the "pg_init" calls to check whether or +not the struct nfs4_filelayout_segment has set a value for dsaddr (that +needs to be done with care to avoid races - cmpxchg() is your friend), +and then rely on that reference being set for the remainder of the +layout segment lifetime? + + +-- +Trond Myklebust +Linux NFS client maintainer, PrimaryData +trond.myklebust@primarydata.com diff --git a/a/content_digest b/N1/content_digest index 14048a9..2ee5bef 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -7,35 +7,48 @@ " linux-fsdevel@vger.kernel.org <linux-fsdevel@vger.kernel.org>\0" "\00:1\0" "b\0" - "SGkgT2xnYSwNCg0KQXBvbG9naWVzIGZvciBtaXNzaW5nIHRoaXMgcGF0Y2guIEl0IHdhcyBoaWRp\n" - "bmcgaW4gbXkgJ2xpbnV4LWZzZGV2ZWwnDQptYWlsYm94LCBzbyBJIGRpZG4ndCByZWNvZ25pc2Ug\n" - "aXQgYXMgYSBORlMgcGF0Y2guDQoNCk9uIEZyaSwgMjAxNy0wNi0zMCBhdCAxNTo1MiAtMDQwMCwg\n" - "T2xnYSBLb3JuaWV2c2thaWEgd3JvdGU6DQo+IFRoZXJlIGlzIGEgcmVncmVzc2lvbiBieSBjb21t\n" - "aXQgOGQ0MGIwZjE0ODQ2ICgiTkZTIGZpbGVsYXlvdXQ6Y2FsbA0KPiBHRVRERVZJQ0VJTkZPIGFm\n" - "dGVyIHBuZnNfbGF5b3V0X3Byb2Nlc3MgY29tcGxldGVzIikuIEl0IGxlYXZlcyB0aGUNCj4gRFMg\n" - "bW91bnQgZGFuZ2xpbmcuDQo+IA0KPiBQcmV2aW91c2x5LCBmaWxlbGF5b3V0X2FsbG9jX3NlYygp\n" - "IHdvdWxkIGNhbGwNCj4gZmlsZWxheW91dF9jaGVja19sYXlvdXQoKQ0KPiB3aGljaCB3b3VsZCBj\n" - "YWxsIG5mczRfZmluZF9nZXRfZGV2aWNlaWQgd2hpY2ggdXBzIHRoZSBjb3VudCBvbiB0aGUNCj4g\n" - "ZGV2aWNlX2lkLiBJdCdzIG9ubHkgY2FsbGVkIG9uY2UgYW5kIGl0J3MgbWF0Y2hlZCBieSB0aGUN\n" - "Cj4gZmlsZWxheW91dF9mcmVlX2xzZWcoKSB0aGF0IGNhbGxzIG5mczRfZmxfcHV0X2RldmljZWlk\n" - "KCkuDQo+IA0KPiBBZnRlciB0aGF0IHBhdGNoLCBlYWNoIHJlYWQvd3JpdGUgZW5kcyB1cCBjYWxs\n" - "aW5nDQo+IG5mczRfZmluZF9nZXRfZGV2aWNlaWQNCj4gYW5kIHRoZXJlIGlzIG5vIGJhbGFuY2Ug\n" - "Zm9yIHRoYXQuIEluc3RlYWQsIGRvIG5mczRfZmxfcHV0X2RldmljZWlkKCkNCj4gaW4gdGhlIGZp\n" - "bGVsYXlvdXQncyAucGdfY2xlYW51cCBhbmQgcmVtb3ZlIGl0IGZyb20NCj4gZmlsZWxheW91dF9m\n" - "cmVlX2xzZWcuDQo+IA0KPiBCdXQgd2Ugc3RpbGwgbmVlZCBhIHJlZmVyZW5jZSB0byBob2xkIG92\n" - "ZXIgdGhlIGxpZmV0aW1lIG9mIHRoZQ0KPiBzZWdtZW50Lg0KPiBGb3IgZXZlcnkgbmV3IGxzZWcg\n" - "dGhhdCdzIGNyZWF0ZWQgd2UgbmVlZCB0byB0YWtlIGEgcmVmZXJlbmNlIG9uDQo+IGRldmljZWlk\n" - "DQo+IHRoYXQgdXNlcyBpdC4gSXQgd2lsbCBiZSByZWxlYXNlZCBpbiB0aGUgImZyZWVfbHNlZyIg\n" - "cm91dGluZS4NCg0KVGhpcyBpcyB3aGF0IEknbSBub3QgdW5kZXJzdGFuZGluZy4gSWYgeW91IGhh\n" - "dmUgYSByZWZlcmVuY2UgaW4gdGhlDQpsYXlvdXQgc2VnbWVudCwgdGhlbiB3aHkgZG8geW91IG5l\n" - "ZWQgdG8gY2FsbCBuZnM0X2ZpbmRfZ2V0X2RldmljZWlkKCkNCmluIHRoZSByZWFkL3dyaXRlIGNv\n" - "ZGU/DQoNCklzbid0IGl0IHN1ZmZpY2llbnQgdG8gY2hhbmdlIHRoZSAicGdfaW5pdCIgY2FsbHMg\n" - "dG8gY2hlY2sgd2hldGhlciBvcg0Kbm90IHRoZSBzdHJ1Y3QgbmZzNF9maWxlbGF5b3V0X3NlZ21l\n" - "bnQgaGFzIHNldCBhIHZhbHVlIGZvciBkc2FkZHIgKHRoYXQNCm5lZWRzIHRvIGJlIGRvbmUgd2l0\n" - "aCBjYXJlIHRvIGF2b2lkIHJhY2VzIC0gY21weGNoZygpIGlzIHlvdXIgZnJpZW5kKSwNCmFuZCB0\n" - "aGVuIHJlbHkgb24gdGhhdCByZWZlcmVuY2UgYmVpbmcgc2V0IGZvciB0aGUgcmVtYWluZGVyIG9m\n" - "IHRoZQ0KbGF5b3V0IHNlZ21lbnQgbGlmZXRpbWU/DQoNCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QN\n" - "CkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lciwgUHJpbWFyeURhdGENCnRyb25kLm15a2xlYnVz\n" - dEBwcmltYXJ5ZGF0YS5jb20NCg== + "Hi Olga,\n" + "\n" + "Apologies for missing this patch. It was hiding in my 'linux-fsdevel'\n" + "mailbox, so I didn't recognise it as a NFS patch.\n" + "\n" + "On Fri, 2017-06-30 at 15:52 -0400, Olga Kornievskaia wrote:\n" + "> There is a regression by commit 8d40b0f14846 (\"NFS filelayout:call\n" + "> GETDEVICEINFO after pnfs_layout_process completes\"). It leaves the\n" + "> DS mount dangling.\n" + "> \n" + "> Previously, filelayout_alloc_sec() would call\n" + "> filelayout_check_layout()\n" + "> which would call nfs4_find_get_deviceid which ups the count on the\n" + "> device_id. It's only called once and it's matched by the\n" + "> filelayout_free_lseg() that calls nfs4_fl_put_deviceid().\n" + "> \n" + "> After that patch, each read/write ends up calling\n" + "> nfs4_find_get_deviceid\n" + "> and there is no balance for that. Instead, do nfs4_fl_put_deviceid()\n" + "> in the filelayout's .pg_cleanup and remove it from\n" + "> filelayout_free_lseg.\n" + "> \n" + "> But we still need a reference to hold over the lifetime of the\n" + "> segment.\n" + "> For every new lseg that's created we need to take a reference on\n" + "> deviceid\n" + "> that uses it. It will be released in the \"free_lseg\" routine.\n" + "\n" + "This is what I'm not understanding. If you have a reference in the\n" + "layout segment, then why do you need to call nfs4_find_get_deviceid()\n" + "in the read/write code?\n" + "\n" + "Isn't it sufficient to change the \"pg_init\" calls to check whether or\n" + "not the struct nfs4_filelayout_segment has set a value for dsaddr (that\n" + "needs to be done with care to avoid races - cmpxchg() is your friend),\n" + "and then rely on that reference being set for the remainder of the\n" + "layout segment lifetime?\n" + "\n" + "\n" + "-- \n" + "Trond Myklebust\n" + "Linux NFS client maintainer, PrimaryData\n" + trond.myklebust@primarydata.com -28070eeb64cfef7b4b9724df1c01b5a94758de695bd365c5ab7a56c02c0b4572 +352e46fd17fee7c110f2cbdb40eebbead3027e4f207ce3ab46af2a9613ec1457
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.