* [PATCH] NFS: init client before declaration @ 2012-05-22 12:40 Stanislav Kinsbursky 2012-05-22 13:47 ` Chuck Lever 2012-05-22 14:29 ` Myklebust, Trond 0 siblings, 2 replies; 13+ messages in thread From: Stanislav Kinsbursky @ 2012-05-22 12:40 UTC (permalink / raw) To: Trond.Myklebust; +Cc: linux-nfs, linux-kernel, devel Client have to be initialized prior to adding it to per-net clients list, because otherwise there are races, shown below: CPU#0 CPU#1 _____ _____ nfs_get_client nfs_alloc_client list_add(..., nfs_client_list) rpc_fill_super rpc_pipefs_event nfs_get_client_for_event __rpc_pipefs_event (clp->cl_rpcclient is uninitialized) BUG() init_client clp->cl_rpcclient = ... Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com> --- fs/nfs/client.c | 22 ++++++++++++---------- 1 files changed, 12 insertions(+), 10 deletions(-) diff --git a/fs/nfs/client.c b/fs/nfs/client.c index ae29d4f..9bf4702 100644 --- a/fs/nfs/client.c +++ b/fs/nfs/client.c @@ -525,7 +525,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init, cl_init->hostname ?: "", cl_init->rpc_ops->version); /* see if the client already exists */ - do { + while (1) { spin_lock(&nn->nfs_client_lock); clp = nfs_match_client(cl_init); @@ -537,10 +537,18 @@ nfs_get_client(const struct nfs_client_initdata *cl_init, spin_unlock(&nn->nfs_client_lock); new = nfs_alloc_client(cl_init); - } while (!IS_ERR(new)); + if (IS_ERR(new)) { + dprintk("--> nfs_get_client() = %ld [failed]\n", PTR_ERR(new)); + return new; + } - dprintk("--> nfs_get_client() = %ld [failed]\n", PTR_ERR(new)); - return new; + error = cl_init->rpc_ops->init_client(new, timeparms, ip_addr, + authflavour, noresvport); + if (error < 0) { + nfs_put_client(new); + return ERR_PTR(error); + } + } /* install a new client and return with it unready */ install_client: @@ -548,12 +556,6 @@ install_client: list_add(&clp->cl_share_link, &nn->nfs_client_list); spin_unlock(&nn->nfs_client_lock); - error = cl_init->rpc_ops->init_client(clp, timeparms, ip_addr, - authflavour, noresvport); - if (error < 0) { - nfs_put_client(clp); - return ERR_PTR(error); - } dprintk("--> nfs_get_client() = %p [new]\n", clp); return clp; ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-22 12:40 [PATCH] NFS: init client before declaration Stanislav Kinsbursky @ 2012-05-22 13:47 ` Chuck Lever 2012-05-22 14:29 ` Myklebust, Trond 1 sibling, 0 replies; 13+ messages in thread From: Chuck Lever @ 2012-05-22 13:47 UTC (permalink / raw) To: Trond Myklebust Cc: Stanislav Kinsbursky, Linux NFS Mailing List, LKML Kernel, devel On May 22, 2012, at 8:40 AM, Stanislav Kinsbursky wrote: > Client have to be initialized prior to adding it to per-net clients list, > because otherwise there are races, shown below: > > CPU#0 CPU#1 > _____ _____ > > nfs_get_client > nfs_alloc_client > list_add(..., nfs_client_list) > rpc_fill_super > rpc_pipefs_event > nfs_get_client_for_event > __rpc_pipefs_event > (clp->cl_rpcclient is uninitialized) > BUG() > init_client > clp->cl_rpcclient = ... > > > Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com> This patch collides pretty hard with the server trunking detection work. If you agree this needs to be fixed, the best thing we can do, I guess, is take this patch and drop patch 11, 12, and 13 from my recent patch set, and I'll try to rework for 3.6. > --- > fs/nfs/client.c | 22 ++++++++++++---------- > 1 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/fs/nfs/client.c b/fs/nfs/client.c > index ae29d4f..9bf4702 100644 > --- a/fs/nfs/client.c > +++ b/fs/nfs/client.c > @@ -525,7 +525,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init, > cl_init->hostname ?: "", cl_init->rpc_ops->version); > > /* see if the client already exists */ > - do { > + while (1) { > spin_lock(&nn->nfs_client_lock); > > clp = nfs_match_client(cl_init); > @@ -537,10 +537,18 @@ nfs_get_client(const struct nfs_client_initdata *cl_init, > spin_unlock(&nn->nfs_client_lock); > > new = nfs_alloc_client(cl_init); > - } while (!IS_ERR(new)); > + if (IS_ERR(new)) { > + dprintk("--> nfs_get_client() = %ld [failed]\n", PTR_ERR(new)); > + return new; > + } > > - dprintk("--> nfs_get_client() = %ld [failed]\n", PTR_ERR(new)); > - return new; > + error = cl_init->rpc_ops->init_client(new, timeparms, ip_addr, > + authflavour, noresvport); > + if (error < 0) { > + nfs_put_client(new); > + return ERR_PTR(error); > + } > + } > > /* install a new client and return with it unready */ > install_client: > @@ -548,12 +556,6 @@ install_client: > list_add(&clp->cl_share_link, &nn->nfs_client_list); > spin_unlock(&nn->nfs_client_lock); > > - error = cl_init->rpc_ops->init_client(clp, timeparms, ip_addr, > - authflavour, noresvport); > - if (error < 0) { > - nfs_put_client(clp); > - return ERR_PTR(error); > - } > dprintk("--> nfs_get_client() = %p [new]\n", clp); > return clp; > > > -- > 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 -- Chuck Lever chuck[dot]lever[at]oracle[dot]com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-22 12:40 [PATCH] NFS: init client before declaration Stanislav Kinsbursky 2012-05-22 13:47 ` Chuck Lever @ 2012-05-22 14:29 ` Myklebust, Trond 2012-05-22 15:00 ` Myklebust, Trond 2012-05-22 15:03 ` Stanislav Kinsbursky 1 sibling, 2 replies; 13+ messages in thread From: Myklebust, Trond @ 2012-05-22 14:29 UTC (permalink / raw) To: Stanislav Kinsbursky Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org T24gVHVlLCAyMDEyLTA1LTIyIGF0IDE2OjQwICswNDAwLCBTdGFuaXNsYXYgS2luc2J1cnNreSB3 cm90ZToNCj4gQ2xpZW50IGhhdmUgdG8gYmUgaW5pdGlhbGl6ZWQgcHJpb3IgdG8gYWRkaW5nIGl0 IHRvIHBlci1uZXQgY2xpZW50cyBsaXN0LA0KPiBiZWNhdXNlIG90aGVyd2lzZSB0aGVyZSBhcmUg cmFjZXMsIHNob3duIGJlbG93Og0KPiANCj4gQ1BVIzAJCQkJCUNQVSMxDQo+IF9fX19fCQkJCQlf X19fXw0KPiANCj4gbmZzX2dldF9jbGllbnQNCj4gbmZzX2FsbG9jX2NsaWVudA0KPiBsaXN0X2Fk ZCguLi4sIG5mc19jbGllbnRfbGlzdCkNCj4gCQkJCQlycGNfZmlsbF9zdXBlcg0KPiAJCQkJCXJw Y19waXBlZnNfZXZlbnQNCj4gCQkJCQluZnNfZ2V0X2NsaWVudF9mb3JfZXZlbnQNCj4gCQkJCQlf X3JwY19waXBlZnNfZXZlbnQNCj4gCQkJCQkoY2xwLT5jbF9ycGNjbGllbnQgaXMgdW5pbml0aWFs aXplZCkNCj4gCQkJCQlCVUcoKQ0KPiBpbml0X2NsaWVudA0KPiBjbHAtPmNsX3JwY2NsaWVudCA9 IC4uLg0KPiANCg0KV2h5IG5vdCBzaW1wbHkgY2hhbmdlIG5mc19nZXRfY2xpZW50X2Zvcl9ldmVu dCgpIHNvIHRoYXQgaXQgZG9lc24ndA0KdG91Y2ggbmZzX2NsaWVudHMgdGhhdCBoYXZlIGNscC0+ Y2xfY29uc19zdGF0ZSE9TkZTX0NTX1JFQURZPw0KDQpUaGF0IHNob3VsZCBlbnN1cmUgdGhhdCBp dCBkb2Vzbid0IHRvdWNoIG5mc19jbGllbnRzIHRoYXQgZmFpbGVkIHRvDQppbml0aWFsaXNlIGFu ZC9vciBhcmUgc3RpbGwgaW4gdGhlIHByb2Nlc3Mgb2YgYmVpbmcgaW5pdGlhbGlzZWQuDQoNCkNo ZWVycw0KICBUcm9uZA0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBt YWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRh cHAuY29tDQoNCg== ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-22 14:29 ` Myklebust, Trond @ 2012-05-22 15:00 ` Myklebust, Trond 2012-05-22 15:29 ` Stanislav Kinsbursky 2012-05-22 15:03 ` Stanislav Kinsbursky 1 sibling, 1 reply; 13+ messages in thread From: Myklebust, Trond @ 2012-05-22 15:00 UTC (permalink / raw) To: Stanislav Kinsbursky Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org T24gVHVlLCAyMDEyLTA1LTIyIGF0IDEwOjI5IC0wNDAwLCBUcm9uZCBNeWtsZWJ1c3Qgd3JvdGU6 DQo+IE9uIFR1ZSwgMjAxMi0wNS0yMiBhdCAxNjo0MCArMDQwMCwgU3RhbmlzbGF2IEtpbnNidXJz a3kgd3JvdGU6DQo+ID4gQ2xpZW50IGhhdmUgdG8gYmUgaW5pdGlhbGl6ZWQgcHJpb3IgdG8gYWRk aW5nIGl0IHRvIHBlci1uZXQgY2xpZW50cyBsaXN0LA0KPiA+IGJlY2F1c2Ugb3RoZXJ3aXNlIHRo ZXJlIGFyZSByYWNlcywgc2hvd24gYmVsb3c6DQo+ID4gDQo+ID4gQ1BVIzAJCQkJCUNQVSMxDQo+ ID4gX19fX18JCQkJCV9fX19fDQo+ID4gDQo+ID4gbmZzX2dldF9jbGllbnQNCj4gPiBuZnNfYWxs b2NfY2xpZW50DQo+ID4gbGlzdF9hZGQoLi4uLCBuZnNfY2xpZW50X2xpc3QpDQo+ID4gCQkJCQly cGNfZmlsbF9zdXBlcg0KPiA+IAkJCQkJcnBjX3BpcGVmc19ldmVudA0KPiA+IAkJCQkJbmZzX2dl dF9jbGllbnRfZm9yX2V2ZW50DQo+ID4gCQkJCQlfX3JwY19waXBlZnNfZXZlbnQNCj4gPiAJCQkJ CShjbHAtPmNsX3JwY2NsaWVudCBpcyB1bmluaXRpYWxpemVkKQ0KPiA+IAkJCQkJQlVHKCkNCj4g PiBpbml0X2NsaWVudA0KPiA+IGNscC0+Y2xfcnBjY2xpZW50ID0gLi4uDQo+ID4gDQo+IA0KPiBX aHkgbm90IHNpbXBseSBjaGFuZ2UgbmZzX2dldF9jbGllbnRfZm9yX2V2ZW50KCkgc28gdGhhdCBp dCBkb2Vzbid0DQo+IHRvdWNoIG5mc19jbGllbnRzIHRoYXQgaGF2ZSBjbHAtPmNsX2NvbnNfc3Rh dGUhPU5GU19DU19SRUFEWT8NCj4gDQo+IFRoYXQgc2hvdWxkIGVuc3VyZSB0aGF0IGl0IGRvZXNu J3QgdG91Y2ggbmZzX2NsaWVudHMgdGhhdCBmYWlsZWQgdG8NCj4gaW5pdGlhbGlzZSBhbmQvb3Ig YXJlIHN0aWxsIGluIHRoZSBwcm9jZXNzIG9mIGJlaW5nIGluaXRpYWxpc2VkLg0KDQouLi5hY3R1 YWxseSwgY29tZSB0byB0aGluayBvZiBpdC4gV2h5IG5vdCBqdXN0IGFkZCBhIGhlbHBlciBmdW5j dGlvbg0KImJvb2wgbmZzX2NsaWVudF9hY3RpdmUoY29uc3Qgc3RydWN0IG5mc19jbGllbnQgKmNs cCkiIHRvDQpmcy9uZnMvY2xpZW50LmMgdGhhdCBkb2VzIGEgY2FsbCB0bw0KCXdhaXRfZXZlbnRf a2lsbGFibGUobmZzX2NsaWVudF9hY3RpdmVfd3EsIGNscC0+Y2xfY29uc19zdGF0ZSA8IE5GU19D U19JTklUSU5HKTsNCmFuZCBjaGVja3MgdGhlIHJlc3VsdGluZyB2YWx1ZSBvZiBjbHAtPmNsX2Nv bnNfc3RhdGU/DQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50 YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5j b20NCg0K ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-22 15:00 ` Myklebust, Trond @ 2012-05-22 15:29 ` Stanislav Kinsbursky 2012-05-22 15:51 ` Myklebust, Trond 0 siblings, 1 reply; 13+ messages in thread From: Stanislav Kinsbursky @ 2012-05-22 15:29 UTC (permalink / raw) To: Myklebust, Trond Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org On 22.05.2012 19:00, Myklebust, Trond wrote: > On Tue, 2012-05-22 at 10:29 -0400, Trond Myklebust wrote: >> On Tue, 2012-05-22 at 16:40 +0400, Stanislav Kinsbursky wrote: >>> Client have to be initialized prior to adding it to per-net clients list, >>> because otherwise there are races, shown below: >>> >>> CPU#0 CPU#1 >>> _____ _____ >>> >>> nfs_get_client >>> nfs_alloc_client >>> list_add(..., nfs_client_list) >>> rpc_fill_super >>> rpc_pipefs_event >>> nfs_get_client_for_event >>> __rpc_pipefs_event >>> (clp->cl_rpcclient is uninitialized) >>> BUG() >>> init_client >>> clp->cl_rpcclient = ... >>> >> >> Why not simply change nfs_get_client_for_event() so that it doesn't >> touch nfs_clients that have clp->cl_cons_state!=NFS_CS_READY? >> >> That should ensure that it doesn't touch nfs_clients that failed to >> initialise and/or are still in the process of being initialised. > > ...actually, come to think of it. Why not just add a helper function > "bool nfs_client_active(const struct nfs_client *clp)" to > fs/nfs/client.c that does a call to > wait_event_killable(nfs_client_active_wq, clp->cl_cons_state< NFS_CS_INITING); > and checks the resulting value of clp->cl_cons_state? > Sorry, but I don't understand the idea... Where are you proposing to call this function? In __rpc_pipefs_event() prior to dentries creatios? -- Best regards, Stanislav Kinsbursky ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-22 15:29 ` Stanislav Kinsbursky @ 2012-05-22 15:51 ` Myklebust, Trond 2012-05-22 16:18 ` Stanislav Kinsbursky 0 siblings, 1 reply; 13+ messages in thread From: Myklebust, Trond @ 2012-05-22 15:51 UTC (permalink / raw) To: Stanislav Kinsbursky Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org T24gVHVlLCAyMDEyLTA1LTIyIGF0IDE5OjI5ICswNDAwLCBTdGFuaXNsYXYgS2luc2J1cnNreSB3 cm90ZToNCj4gT24gMjIuMDUuMjAxMiAxOTowMCwgTXlrbGVidXN0LCBUcm9uZCB3cm90ZToNCj4g PiBPbiBUdWUsIDIwMTItMDUtMjIgYXQgMTA6MjkgLTA0MDAsIFRyb25kIE15a2xlYnVzdCB3cm90 ZToNCj4gPj4gT24gVHVlLCAyMDEyLTA1LTIyIGF0IDE2OjQwICswNDAwLCBTdGFuaXNsYXYgS2lu c2J1cnNreSB3cm90ZToNCj4gPj4+IENsaWVudCBoYXZlIHRvIGJlIGluaXRpYWxpemVkIHByaW9y IHRvIGFkZGluZyBpdCB0byBwZXItbmV0IGNsaWVudHMgbGlzdCwNCj4gPj4+IGJlY2F1c2Ugb3Ro ZXJ3aXNlIHRoZXJlIGFyZSByYWNlcywgc2hvd24gYmVsb3c6DQo+ID4+Pg0KPiA+Pj4gQ1BVIzAJ CQkJCUNQVSMxDQo+ID4+PiBfX19fXwkJCQkJX19fX18NCj4gPj4+DQo+ID4+PiBuZnNfZ2V0X2Ns aWVudA0KPiA+Pj4gbmZzX2FsbG9jX2NsaWVudA0KPiA+Pj4gbGlzdF9hZGQoLi4uLCBuZnNfY2xp ZW50X2xpc3QpDQo+ID4+PiAJCQkJCXJwY19maWxsX3N1cGVyDQo+ID4+PiAJCQkJCXJwY19waXBl ZnNfZXZlbnQNCj4gPj4+IAkJCQkJbmZzX2dldF9jbGllbnRfZm9yX2V2ZW50DQo+ID4+PiAJCQkJ CV9fcnBjX3BpcGVmc19ldmVudA0KPiA+Pj4gCQkJCQkoY2xwLT5jbF9ycGNjbGllbnQgaXMgdW5p bml0aWFsaXplZCkNCj4gPj4+IAkJCQkJQlVHKCkNCj4gPj4+IGluaXRfY2xpZW50DQo+ID4+PiBj bHAtPmNsX3JwY2NsaWVudCA9IC4uLg0KPiA+Pj4NCj4gPj4NCj4gPj4gV2h5IG5vdCBzaW1wbHkg Y2hhbmdlIG5mc19nZXRfY2xpZW50X2Zvcl9ldmVudCgpIHNvIHRoYXQgaXQgZG9lc24ndA0KPiA+ PiB0b3VjaCBuZnNfY2xpZW50cyB0aGF0IGhhdmUgY2xwLT5jbF9jb25zX3N0YXRlIT1ORlNfQ1Nf UkVBRFk/DQo+ID4+DQo+ID4+IFRoYXQgc2hvdWxkIGVuc3VyZSB0aGF0IGl0IGRvZXNuJ3QgdG91 Y2ggbmZzX2NsaWVudHMgdGhhdCBmYWlsZWQgdG8NCj4gPj4gaW5pdGlhbGlzZSBhbmQvb3IgYXJl IHN0aWxsIGluIHRoZSBwcm9jZXNzIG9mIGJlaW5nIGluaXRpYWxpc2VkLg0KPiA+DQo+ID4gLi4u YWN0dWFsbHksIGNvbWUgdG8gdGhpbmsgb2YgaXQuIFdoeSBub3QganVzdCBhZGQgYSBoZWxwZXIg ZnVuY3Rpb24NCj4gPiAiYm9vbCBuZnNfY2xpZW50X2FjdGl2ZShjb25zdCBzdHJ1Y3QgbmZzX2Ns aWVudCAqY2xwKSIgdG8NCj4gPiBmcy9uZnMvY2xpZW50LmMgdGhhdCBkb2VzIGEgY2FsbCB0bw0K PiA+IAl3YWl0X2V2ZW50X2tpbGxhYmxlKG5mc19jbGllbnRfYWN0aXZlX3dxLCBjbHAtPmNsX2Nv bnNfc3RhdGU8ICBORlNfQ1NfSU5JVElORyk7DQo+ID4gYW5kIGNoZWNrcyB0aGUgcmVzdWx0aW5n IHZhbHVlIG9mIGNscC0+Y2xfY29uc19zdGF0ZT8NCj4gPg0KPiANCj4gU29ycnksIGJ1dCBJIGRv bid0IHVuZGVyc3RhbmQgdGhlIGlkZWEuLi4NCj4gV2hlcmUgYXJlIHlvdSBwcm9wb3NpbmcgdG8g Y2FsbCB0aGlzIGZ1bmN0aW9uPw0KPiBJbiBfX3JwY19waXBlZnNfZXZlbnQoKSBwcmlvciB0byBk ZW50cmllcyBjcmVhdGlvcz8NCg0KU2VlIGJlbG93Og0KDQo4PC0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0NCkZyb20gZjViOTBkZjYzODFhMjAzOTVkOWY4OGExOTllOWU1MmY0NDI2NzQ1NyBNb24g U2VwIDE3IDAwOjAwOjAwIDIwMDENCkZyb206IFRyb25kIE15a2xlYnVzdCA8VHJvbmQuTXlrbGVi dXN0QG5ldGFwcC5jb20+DQpEYXRlOiBUdWUsIDIyIE1heSAyMDEyIDExOjQ5OjU1IC0wNDAwDQpT dWJqZWN0OiBbUEFUQ0hdIE5GU3Y0OiBGaXggYSByYWNlIGluIHRoZSBuZXQgbmFtZXNwYWNlIG1v dW50IG5vdGlmaWNhdGlvbg0KDQpTaW5jZSB0aGUgc3RydWN0IG5mc19jbGllbnQgZ2V0cyBhZGRl ZCB0byB0aGUgZ2xvYmFsIG5mc19jbGllbnRfbGlzdA0KYmVmb3JlIGl0IGlzIGluaXRpYWxpc2Vk LCBpdCBpcyBwb3NzaWJsZSB0aGF0IHJwY19waXBlZnNfZXZlbnQgY2FuDQplbmQgdXAgdHJ5aW5n IHRvIGNyZWF0ZSBpZG1hcHBlciBlbnRyaWVzIGZvciBzdWNoIGEgdGhpbmcuDQoNClRoZSBzb2x1 dGlvbiBpcyB0byBoYXZlIHRoZSBtb3VudCBub3RpZmljYXRpb24gd2FpdCBmb3IgdGhlDQpuZnNf Y2xpZW50IGluaXRpYWxpc2F0aW9uIHRvIGNvbXBsZXRlLg0KDQpSZXBvcnRlZC1ieTogU3Rhbmlz bGF2IEtpbnNidXJza3kgPHNraW5zYnVyc2t5QHBhcmFsbGVscy5jb20+DQpTaWduZWQtb2ZmLWJ5 OiBUcm9uZCBNeWtsZWJ1c3QgPFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tPg0KLS0tDQogZnMv bmZzL2NsaWVudC5jICAgfCAgIDE0ICsrKysrKysrKysrKysrDQogZnMvbmZzL2lkbWFwLmMgICAg fCAgICAzICsrLQ0KIGZzL25mcy9pbnRlcm5hbC5oIHwgICAgMSArDQogMyBmaWxlcyBjaGFuZ2Vk LCAxNyBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9ucygtKQ0KDQpkaWZmIC0tZ2l0IGEvZnMvbmZz L2NsaWVudC5jIGIvZnMvbmZzL2NsaWVudC5jDQppbmRleCA2MGY3ZTRlLi4zZmE0NGVmIDEwMDY0 NA0KLS0tIGEvZnMvbmZzL2NsaWVudC5jDQorKysgYi9mcy9uZnMvY2xpZW50LmMNCkBAIC01OTIs NiArNTkyLDIwIEBAIHZvaWQgbmZzX21hcmtfY2xpZW50X3JlYWR5KHN0cnVjdCBuZnNfY2xpZW50 ICpjbHAsIGludCBzdGF0ZSkNCiAJd2FrZV91cF9hbGwoJm5mc19jbGllbnRfYWN0aXZlX3dxKTsN CiB9DQogDQorc3RhdGljIGJvb2wgbmZzX2NsaWVudF9yZWFkeShzdHJ1Y3QgbmZzX2NsaWVudCAq Y2xwKQ0KK3sNCisJcmV0dXJuIGNscC0+Y2xfY29uc19zdGF0ZSA8PSBORlNfQ1NfUkVBRFk7DQor fQ0KKw0KK2ludCBuZnNfd2FpdF9jbGllbnRfcmVhZHkoc3RydWN0IG5mc19jbGllbnQgKmNscCkN Cit7DQorCWlmICh3YWl0X2V2ZW50X2tpbGxhYmxlKG5mc19jbGllbnRfYWN0aXZlX3dxLCBuZnNf Y2xpZW50X3JlYWR5KGNscCkpIDwgMCkNCisJCXJldHVybiAtRVJFU1RBUlRTWVM7DQorCWlmIChj bHAtPmNsX2NvbnNfc3RhdGUgPCAwKQ0KKwkJcmV0dXJuIGNscC0+Y2xfY29uc19zdGF0ZTsNCisJ cmV0dXJuIDA7DQorfQ0KKw0KIC8qDQogICogV2l0aCBzZXNzaW9ucywgdGhlIGNsaWVudCBpcyBu b3QgbWFya2VkIHJlYWR5IHVudGlsIGFmdGVyIGENCiAgKiBzdWNjZXNzZnVsIEVYQ0hBTkdFX0lE IGFuZCBDUkVBVEVfU0VTU0lPTi4NCmRpZmYgLS1naXQgYS9mcy9uZnMvaWRtYXAuYyBiL2ZzL25m cy9pZG1hcC5jDQppbmRleCAzZThlZGJlLi42Nzk2MmM4IDEwMDY0NA0KLS0tIGEvZnMvbmZzL2lk bWFwLmMNCisrKyBiL2ZzL25mcy9pZG1hcC5jDQpAQCAtNTU4LDcgKzU1OCw4IEBAIHN0YXRpYyBp bnQgcnBjX3BpcGVmc19ldmVudChzdHJ1Y3Qgbm90aWZpZXJfYmxvY2sgKm5iLCB1bnNpZ25lZCBs b25nIGV2ZW50LA0KIAkJcmV0dXJuIDA7DQogDQogCXdoaWxlICgoY2xwID0gbmZzX2dldF9jbGll bnRfZm9yX2V2ZW50KHNiLT5zX2ZzX2luZm8sIGV2ZW50KSkpIHsNCi0JCWVycm9yID0gX19ycGNf cGlwZWZzX2V2ZW50KGNscCwgZXZlbnQsIHNiKTsNCisJCWlmIChuZnNfd2FpdF9jbGllbnRfcmVh ZHkoY2xwKSA9PSAwKQ0KKwkJCWVycm9yID0gX19ycGNfcGlwZWZzX2V2ZW50KGNscCwgZXZlbnQs IHNiKTsNCiAJCW5mc19wdXRfY2xpZW50KGNscCk7DQogCQlpZiAoZXJyb3IpDQogCQkJYnJlYWs7 DQpkaWZmIC0tZ2l0IGEvZnMvbmZzL2ludGVybmFsLmggYi9mcy9uZnMvaW50ZXJuYWwuaA0KaW5k ZXggYjc3N2JkYS4uM2JlMDBhMCAxMDA2NDQNCi0tLSBhL2ZzL25mcy9pbnRlcm5hbC5oDQorKysg Yi9mcy9uZnMvaW50ZXJuYWwuaA0KQEAgLTE2OCw2ICsxNjgsNyBAQCBleHRlcm4gc3RydWN0IG5m c19zZXJ2ZXIgKm5mc19jbG9uZV9zZXJ2ZXIoc3RydWN0IG5mc19zZXJ2ZXIgKiwNCiAJCQkJCSAg IHN0cnVjdCBuZnNfZmF0dHIgKiwNCiAJCQkJCSAgIHJwY19hdXRoZmxhdm9yX3QpOw0KIGV4dGVy biB2b2lkIG5mc19tYXJrX2NsaWVudF9yZWFkeShzdHJ1Y3QgbmZzX2NsaWVudCAqY2xwLCBpbnQg c3RhdGUpOw0KK2V4dGVybiBpbnQgbmZzX3dhaXRfY2xpZW50X3JlYWR5KHN0cnVjdCBuZnNfY2xp ZW50ICpjbHApOw0KIGV4dGVybiBpbnQgbmZzNF9jaGVja19jbGllbnRfcmVhZHkoc3RydWN0IG5m c19jbGllbnQgKmNscCk7DQogZXh0ZXJuIHN0cnVjdCBuZnNfY2xpZW50ICpuZnM0X3NldF9kc19j bGllbnQoc3RydWN0IG5mc19jbGllbnQqIG1kc19jbHAsDQogCQkJCQkgICAgIGNvbnN0IHN0cnVj dCBzb2NrYWRkciAqZHNfYWRkciwNCi0tIA0KMS43LjcuNg0KDQoNCi0tIA0KVHJvbmQgTXlrbGVi dXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1 c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-22 15:51 ` Myklebust, Trond @ 2012-05-22 16:18 ` Stanislav Kinsbursky 2012-05-22 16:43 ` Myklebust, Trond 0 siblings, 1 reply; 13+ messages in thread From: Stanislav Kinsbursky @ 2012-05-22 16:18 UTC (permalink / raw) To: Myklebust, Trond Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org On 22.05.2012 19:51, Myklebust, Trond wrote: > On Tue, 2012-05-22 at 19:29 +0400, Stanislav Kinsbursky wrote: >> On 22.05.2012 19:00, Myklebust, Trond wrote: >>> On Tue, 2012-05-22 at 10:29 -0400, Trond Myklebust wrote: >>>> On Tue, 2012-05-22 at 16:40 +0400, Stanislav Kinsbursky wrote: >>>>> Client have to be initialized prior to adding it to per-net clients list, >>>>> because otherwise there are races, shown below: >>>>> >>>>> CPU#0 CPU#1 >>>>> _____ _____ >>>>> >>>>> nfs_get_client >>>>> nfs_alloc_client >>>>> list_add(..., nfs_client_list) >>>>> rpc_fill_super >>>>> rpc_pipefs_event >>>>> nfs_get_client_for_event >>>>> __rpc_pipefs_event >>>>> (clp->cl_rpcclient is uninitialized) >>>>> BUG() >>>>> init_client >>>>> clp->cl_rpcclient = ... >>>>> >>>> >>>> Why not simply change nfs_get_client_for_event() so that it doesn't >>>> touch nfs_clients that have clp->cl_cons_state!=NFS_CS_READY? >>>> >>>> That should ensure that it doesn't touch nfs_clients that failed to >>>> initialise and/or are still in the process of being initialised. >>> >>> ...actually, come to think of it. Why not just add a helper function >>> "bool nfs_client_active(const struct nfs_client *clp)" to >>> fs/nfs/client.c that does a call to >>> wait_event_killable(nfs_client_active_wq, clp->cl_cons_state< NFS_CS_INITING); >>> and checks the resulting value of clp->cl_cons_state? >>> >> >> Sorry, but I don't understand the idea... >> Where are you proposing to call this function? >> In __rpc_pipefs_event() prior to dentries creatios? > > See below: > > 8<---------------------------------------------------------------------------------- > From f5b90df6381a20395d9f88a199e9e52f44267457 Mon Sep 17 00:00:00 2001 > From: Trond Myklebust<Trond.Myklebust@netapp.com> > Date: Tue, 22 May 2012 11:49:55 -0400 > Subject: [PATCH] NFSv4: Fix a race in the net namespace mount notification > > Since the struct nfs_client gets added to the global nfs_client_list > before it is initialised, it is possible that rpc_pipefs_event can > end up trying to create idmapper entries for such a thing. > > The solution is to have the mount notification wait for the > nfs_client initialisation to complete. > > Reported-by: Stanislav Kinsbursky<skinsbursky@parallels.com> > Signed-off-by: Trond Myklebust<Trond.Myklebust@netapp.com> > --- > fs/nfs/client.c | 14 ++++++++++++++ > fs/nfs/idmap.c | 3 ++- > fs/nfs/internal.h | 1 + > 3 files changed, 17 insertions(+), 1 deletions(-) > > diff --git a/fs/nfs/client.c b/fs/nfs/client.c > index 60f7e4e..3fa44ef 100644 > --- a/fs/nfs/client.c > +++ b/fs/nfs/client.c > @@ -592,6 +592,20 @@ void nfs_mark_client_ready(struct nfs_client *clp, int state) > wake_up_all(&nfs_client_active_wq); > } > > +static bool nfs_client_ready(struct nfs_client *clp) > +{ > + return clp->cl_cons_state<= NFS_CS_READY; > +} > + > +int nfs_wait_client_ready(struct nfs_client *clp) > +{ > + if (wait_event_killable(nfs_client_active_wq, nfs_client_ready(clp))< 0) > + return -ERESTARTSYS; Ok, I see... BTW, caller of this function is pipefs mount operation call... And when this mount call waits for NFS clients - it look a bit odd to me... > + if (clp->cl_cons_state< 0) > + return clp->cl_cons_state; > + return 0; > +} > + > /* > * With sessions, the client is not marked ready until after a > * successful EXCHANGE_ID and CREATE_SESSION. > diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c > index 3e8edbe..67962c8 100644 > --- a/fs/nfs/idmap.c > +++ b/fs/nfs/idmap.c > @@ -558,7 +558,8 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event, > return 0; > > while ((clp = nfs_get_client_for_event(sb->s_fs_info, event))) { > - error = __rpc_pipefs_event(clp, event, sb); > + if (nfs_wait_client_ready(clp) == 0) > + error = __rpc_pipefs_event(clp, event, sb); We have another problem here. nfs4_init_client() will try to create pipe dentries prior to set of NFS_CS_READY to the client. And dentries will be created since semaphore is dropped and per-net superblock variable is initialized already. But __rpc_pipefs_event() relays on the fact, that no dentries present. Looks like the problem was introduced by me in aad9487c... So maybe we should not call "continue" instead "__rpc_pipefs_event()", when client becomes ready? Looks like this will allow us to handle such races. > nfs_put_client(clp); > if (error) > break; > diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h > index b777bda..3be00a0 100644 > --- a/fs/nfs/internal.h > +++ b/fs/nfs/internal.h > @@ -168,6 +168,7 @@ extern struct nfs_server *nfs_clone_server(struct nfs_server *, > struct nfs_fattr *, > rpc_authflavor_t); > extern void nfs_mark_client_ready(struct nfs_client *clp, int state); > +extern int nfs_wait_client_ready(struct nfs_client *clp); > extern int nfs4_check_client_ready(struct nfs_client *clp); > extern struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp, > const struct sockaddr *ds_addr, -- Best regards, Stanislav Kinsbursky ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-22 16:18 ` Stanislav Kinsbursky @ 2012-05-22 16:43 ` Myklebust, Trond 2012-05-22 20:32 ` Myklebust, Trond 0 siblings, 1 reply; 13+ messages in thread From: Myklebust, Trond @ 2012-05-22 16:43 UTC (permalink / raw) To: Stanislav Kinsbursky Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org T24gVHVlLCAyMDEyLTA1LTIyIGF0IDIwOjE4ICswNDAwLCBTdGFuaXNsYXYgS2luc2J1cnNreSB3 cm90ZToNCj4gT24gMjIuMDUuMjAxMiAxOTo1MSwgTXlrbGVidXN0LCBUcm9uZCB3cm90ZToNCj4g PiBPbiBUdWUsIDIwMTItMDUtMjIgYXQgMTk6MjkgKzA0MDAsIFN0YW5pc2xhdiBLaW5zYnVyc2t5 IHdyb3RlOg0KPiA+PiBPbiAyMi4wNS4yMDEyIDE5OjAwLCBNeWtsZWJ1c3QsIFRyb25kIHdyb3Rl Og0KPiA+Pj4gT24gVHVlLCAyMDEyLTA1LTIyIGF0IDEwOjI5IC0wNDAwLCBUcm9uZCBNeWtsZWJ1 c3Qgd3JvdGU6DQo+ID4+Pj4gT24gVHVlLCAyMDEyLTA1LTIyIGF0IDE2OjQwICswNDAwLCBTdGFu aXNsYXYgS2luc2J1cnNreSB3cm90ZToNCj4gPj4+Pj4gQ2xpZW50IGhhdmUgdG8gYmUgaW5pdGlh bGl6ZWQgcHJpb3IgdG8gYWRkaW5nIGl0IHRvIHBlci1uZXQgY2xpZW50cyBsaXN0LA0KPiA+Pj4+ PiBiZWNhdXNlIG90aGVyd2lzZSB0aGVyZSBhcmUgcmFjZXMsIHNob3duIGJlbG93Og0KPiA+Pj4+ Pg0KPiA+Pj4+PiBDUFUjMAkJCQkJQ1BVIzENCj4gPj4+Pj4gX19fX18JCQkJCV9fX19fDQo+ID4+ Pj4+DQo+ID4+Pj4+IG5mc19nZXRfY2xpZW50DQo+ID4+Pj4+IG5mc19hbGxvY19jbGllbnQNCj4g Pj4+Pj4gbGlzdF9hZGQoLi4uLCBuZnNfY2xpZW50X2xpc3QpDQo+ID4+Pj4+IAkJCQkJcnBjX2Zp bGxfc3VwZXINCj4gPj4+Pj4gCQkJCQlycGNfcGlwZWZzX2V2ZW50DQo+ID4+Pj4+IAkJCQkJbmZz X2dldF9jbGllbnRfZm9yX2V2ZW50DQo+ID4+Pj4+IAkJCQkJX19ycGNfcGlwZWZzX2V2ZW50DQo+ ID4+Pj4+IAkJCQkJKGNscC0+Y2xfcnBjY2xpZW50IGlzIHVuaW5pdGlhbGl6ZWQpDQo+ID4+Pj4+ IAkJCQkJQlVHKCkNCj4gPj4+Pj4gaW5pdF9jbGllbnQNCj4gPj4+Pj4gY2xwLT5jbF9ycGNjbGll bnQgPSAuLi4NCj4gPj4+Pj4NCj4gPj4+Pg0KPiA+Pj4+IFdoeSBub3Qgc2ltcGx5IGNoYW5nZSBu ZnNfZ2V0X2NsaWVudF9mb3JfZXZlbnQoKSBzbyB0aGF0IGl0IGRvZXNuJ3QNCj4gPj4+PiB0b3Vj aCBuZnNfY2xpZW50cyB0aGF0IGhhdmUgY2xwLT5jbF9jb25zX3N0YXRlIT1ORlNfQ1NfUkVBRFk/ DQo+ID4+Pj4NCj4gPj4+PiBUaGF0IHNob3VsZCBlbnN1cmUgdGhhdCBpdCBkb2Vzbid0IHRvdWNo IG5mc19jbGllbnRzIHRoYXQgZmFpbGVkIHRvDQo+ID4+Pj4gaW5pdGlhbGlzZSBhbmQvb3IgYXJl IHN0aWxsIGluIHRoZSBwcm9jZXNzIG9mIGJlaW5nIGluaXRpYWxpc2VkLg0KPiA+Pj4NCj4gPj4+ IC4uLmFjdHVhbGx5LCBjb21lIHRvIHRoaW5rIG9mIGl0LiBXaHkgbm90IGp1c3QgYWRkIGEgaGVs cGVyIGZ1bmN0aW9uDQo+ID4+PiAiYm9vbCBuZnNfY2xpZW50X2FjdGl2ZShjb25zdCBzdHJ1Y3Qg bmZzX2NsaWVudCAqY2xwKSIgdG8NCj4gPj4+IGZzL25mcy9jbGllbnQuYyB0aGF0IGRvZXMgYSBj YWxsIHRvDQo+ID4+PiAJd2FpdF9ldmVudF9raWxsYWJsZShuZnNfY2xpZW50X2FjdGl2ZV93cSwg Y2xwLT5jbF9jb25zX3N0YXRlPCAgIE5GU19DU19JTklUSU5HKTsNCj4gPj4+IGFuZCBjaGVja3Mg dGhlIHJlc3VsdGluZyB2YWx1ZSBvZiBjbHAtPmNsX2NvbnNfc3RhdGU/DQo+ID4+Pg0KPiA+Pg0K PiA+PiBTb3JyeSwgYnV0IEkgZG9uJ3QgdW5kZXJzdGFuZCB0aGUgaWRlYS4uLg0KPiA+PiBXaGVy ZSBhcmUgeW91IHByb3Bvc2luZyB0byBjYWxsIHRoaXMgZnVuY3Rpb24/DQo+ID4+IEluIF9fcnBj X3BpcGVmc19ldmVudCgpIHByaW9yIHRvIGRlbnRyaWVzIGNyZWF0aW9zPw0KPiA+DQo+ID4gU2Vl IGJlbG93Og0KPiA+DQo+ID4gODwtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQo+ID4gIEZyb20g ZjViOTBkZjYzODFhMjAzOTVkOWY4OGExOTllOWU1MmY0NDI2NzQ1NyBNb24gU2VwIDE3IDAwOjAw OjAwIDIwMDENCj4gPiBGcm9tOiBUcm9uZCBNeWtsZWJ1c3Q8VHJvbmQuTXlrbGVidXN0QG5ldGFw cC5jb20+DQo+ID4gRGF0ZTogVHVlLCAyMiBNYXkgMjAxMiAxMTo0OTo1NSAtMDQwMA0KPiA+IFN1 YmplY3Q6IFtQQVRDSF0gTkZTdjQ6IEZpeCBhIHJhY2UgaW4gdGhlIG5ldCBuYW1lc3BhY2UgbW91 bnQgbm90aWZpY2F0aW9uDQo+ID4NCj4gPiBTaW5jZSB0aGUgc3RydWN0IG5mc19jbGllbnQgZ2V0 cyBhZGRlZCB0byB0aGUgZ2xvYmFsIG5mc19jbGllbnRfbGlzdA0KPiA+IGJlZm9yZSBpdCBpcyBp bml0aWFsaXNlZCwgaXQgaXMgcG9zc2libGUgdGhhdCBycGNfcGlwZWZzX2V2ZW50IGNhbg0KPiA+ IGVuZCB1cCB0cnlpbmcgdG8gY3JlYXRlIGlkbWFwcGVyIGVudHJpZXMgZm9yIHN1Y2ggYSB0aGlu Zy4NCj4gPg0KPiA+IFRoZSBzb2x1dGlvbiBpcyB0byBoYXZlIHRoZSBtb3VudCBub3RpZmljYXRp b24gd2FpdCBmb3IgdGhlDQo+ID4gbmZzX2NsaWVudCBpbml0aWFsaXNhdGlvbiB0byBjb21wbGV0 ZS4NCj4gPg0KPiA+IFJlcG9ydGVkLWJ5OiBTdGFuaXNsYXYgS2luc2J1cnNreTxza2luc2J1cnNr eUBwYXJhbGxlbHMuY29tPg0KPiA+IFNpZ25lZC1vZmYtYnk6IFRyb25kIE15a2xlYnVzdDxUcm9u ZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbT4NCj4gPiAtLS0NCj4gPiAgIGZzL25mcy9jbGllbnQuYyAg IHwgICAxNCArKysrKysrKysrKysrKw0KPiA+ICAgZnMvbmZzL2lkbWFwLmMgICAgfCAgICAzICsr LQ0KPiA+ICAgZnMvbmZzL2ludGVybmFsLmggfCAgICAxICsNCj4gPiAgIDMgZmlsZXMgY2hhbmdl ZCwgMTcgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbnMoLSkNCj4gPg0KPiA+IGRpZmYgLS1naXQg YS9mcy9uZnMvY2xpZW50LmMgYi9mcy9uZnMvY2xpZW50LmMNCj4gPiBpbmRleCA2MGY3ZTRlLi4z ZmE0NGVmIDEwMDY0NA0KPiA+IC0tLSBhL2ZzL25mcy9jbGllbnQuYw0KPiA+ICsrKyBiL2ZzL25m cy9jbGllbnQuYw0KPiA+IEBAIC01OTIsNiArNTkyLDIwIEBAIHZvaWQgbmZzX21hcmtfY2xpZW50 X3JlYWR5KHN0cnVjdCBuZnNfY2xpZW50ICpjbHAsIGludCBzdGF0ZSkNCj4gPiAgIAl3YWtlX3Vw X2FsbCgmbmZzX2NsaWVudF9hY3RpdmVfd3EpOw0KPiA+ICAgfQ0KPiA+DQo+ID4gK3N0YXRpYyBi b29sIG5mc19jbGllbnRfcmVhZHkoc3RydWN0IG5mc19jbGllbnQgKmNscCkNCj4gPiArew0KPiA+ ICsJcmV0dXJuIGNscC0+Y2xfY29uc19zdGF0ZTw9IE5GU19DU19SRUFEWTsNCj4gPiArfQ0KPiA+ ICsNCj4gPiAraW50IG5mc193YWl0X2NsaWVudF9yZWFkeShzdHJ1Y3QgbmZzX2NsaWVudCAqY2xw KQ0KPiA+ICt7DQo+ID4gKwlpZiAod2FpdF9ldmVudF9raWxsYWJsZShuZnNfY2xpZW50X2FjdGl2 ZV93cSwgbmZzX2NsaWVudF9yZWFkeShjbHApKTwgIDApDQo+ID4gKwkJcmV0dXJuIC1FUkVTVEFS VFNZUzsNCj4gDQo+IE9rLCBJIHNlZS4uLg0KPiBCVFcsIGNhbGxlciBvZiB0aGlzIGZ1bmN0aW9u IGlzIHBpcGVmcyBtb3VudCBvcGVyYXRpb24gY2FsbC4uLiBBbmQgd2hlbiB0aGlzIA0KPiBtb3Vu dCBjYWxsIHdhaXRzIGZvciBORlMgY2xpZW50cyAtIGl0IGxvb2sgYSBiaXQgb2RkIHRvIG1lLi4u DQo+IA0KPiANCj4gPiArCWlmIChjbHAtPmNsX2NvbnNfc3RhdGU8ICAwKQ0KPiA+ICsJCXJldHVy biBjbHAtPmNsX2NvbnNfc3RhdGU7DQo+ID4gKwlyZXR1cm4gMDsNCj4gPiArfQ0KPiA+ICsNCj4g PiAgIC8qDQo+ID4gICAgKiBXaXRoIHNlc3Npb25zLCB0aGUgY2xpZW50IGlzIG5vdCBtYXJrZWQg cmVhZHkgdW50aWwgYWZ0ZXIgYQ0KPiA+ICAgICogc3VjY2Vzc2Z1bCBFWENIQU5HRV9JRCBhbmQg Q1JFQVRFX1NFU1NJT04uDQo+ID4gZGlmZiAtLWdpdCBhL2ZzL25mcy9pZG1hcC5jIGIvZnMvbmZz L2lkbWFwLmMNCj4gPiBpbmRleCAzZThlZGJlLi42Nzk2MmM4IDEwMDY0NA0KPiA+IC0tLSBhL2Zz L25mcy9pZG1hcC5jDQo+ID4gKysrIGIvZnMvbmZzL2lkbWFwLmMNCj4gPiBAQCAtNTU4LDcgKzU1 OCw4IEBAIHN0YXRpYyBpbnQgcnBjX3BpcGVmc19ldmVudChzdHJ1Y3Qgbm90aWZpZXJfYmxvY2sg Km5iLCB1bnNpZ25lZCBsb25nIGV2ZW50LA0KPiA+ICAgCQlyZXR1cm4gMDsNCj4gPg0KPiA+ICAg CXdoaWxlICgoY2xwID0gbmZzX2dldF9jbGllbnRfZm9yX2V2ZW50KHNiLT5zX2ZzX2luZm8sIGV2 ZW50KSkpIHsNCj4gPiAtCQllcnJvciA9IF9fcnBjX3BpcGVmc19ldmVudChjbHAsIGV2ZW50LCBz Yik7DQo+ID4gKwkJaWYgKG5mc193YWl0X2NsaWVudF9yZWFkeShjbHApID09IDApDQo+ID4gKwkJ CWVycm9yID0gX19ycGNfcGlwZWZzX2V2ZW50KGNscCwgZXZlbnQsIHNiKTsNCj4gDQo+IA0KPiBX ZSBoYXZlIGFub3RoZXIgcHJvYmxlbSBoZXJlLg0KPiBuZnM0X2luaXRfY2xpZW50KCkgd2lsbCB0 cnkgdG8gY3JlYXRlIHBpcGUgZGVudHJpZXMgcHJpb3IgdG8gc2V0IG9mIE5GU19DU19SRUFEWSAN Cj4gdG8gdGhlIGNsaWVudC4gQW5kIGRlbnRyaWVzIHdpbGwgYmUgY3JlYXRlZCBzaW5jZSBzZW1h cGhvcmUgaXMgZHJvcHBlZCBhbmQgDQo+IHBlci1uZXQgc3VwZXJibG9jayB2YXJpYWJsZSBpcyBp bml0aWFsaXplZCBhbHJlYWR5Lg0KPiBCdXQgX19ycGNfcGlwZWZzX2V2ZW50KCkgcmVsYXlzIG9u IHRoZSBmYWN0LCB0aGF0IG5vIGRlbnRyaWVzIHByZXNlbnQuDQo+IExvb2tzIGxpa2UgdGhlIHBy b2JsZW0gd2FzIGludHJvZHVjZWQgYnkgbWUgaW4gYWFkOTQ4N2MuLi4NCj4gU28gbWF5YmUgd2Ug c2hvdWxkIG5vdCBjYWxsICJjb250aW51ZSIgaW5zdGVhZCAiX19ycGNfcGlwZWZzX2V2ZW50KCki LCB3aGVuIA0KPiBjbGllbnQgYmVjb21lcyByZWFkeT8NCj4gTG9va3MgbGlrZSB0aGlzIHdpbGwg YWxsb3cgdXMgdG8gaGFuZGxlIHN1Y2ggcmFjZXMuDQoNCkxldCBtZSByZXdvcmsgdGhpcyBwYXRj aCBhIGJpdC4uLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWlu dGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAu Y29tDQoNCg== ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-22 16:43 ` Myklebust, Trond @ 2012-05-22 20:32 ` Myklebust, Trond 2012-05-23 11:30 ` Stanislav Kinsbursky 0 siblings, 1 reply; 13+ messages in thread From: Myklebust, Trond @ 2012-05-22 20:32 UTC (permalink / raw) To: Stanislav Kinsbursky Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org T24gVHVlLCAyMDEyLTA1LTIyIGF0IDEyOjQzIC0wNDAwLCBUcm9uZCBNeWtsZWJ1c3Qgd3JvdGU6 DQo+IE9uIFR1ZSwgMjAxMi0wNS0yMiBhdCAyMDoxOCArMDQwMCwgU3RhbmlzbGF2IEtpbnNidXJz a3kgd3JvdGU6DQo+ID4gV2UgaGF2ZSBhbm90aGVyIHByb2JsZW0gaGVyZS4NCj4gPiBuZnM0X2lu aXRfY2xpZW50KCkgd2lsbCB0cnkgdG8gY3JlYXRlIHBpcGUgZGVudHJpZXMgcHJpb3IgdG8gc2V0 IG9mIE5GU19DU19SRUFEWSANCj4gPiB0byB0aGUgY2xpZW50LiBBbmQgZGVudHJpZXMgd2lsbCBi ZSBjcmVhdGVkIHNpbmNlIHNlbWFwaG9yZSBpcyBkcm9wcGVkIGFuZCANCj4gPiBwZXItbmV0IHN1 cGVyYmxvY2sgdmFyaWFibGUgaXMgaW5pdGlhbGl6ZWQgYWxyZWFkeS4NCj4gPiBCdXQgX19ycGNf cGlwZWZzX2V2ZW50KCkgcmVsYXlzIG9uIHRoZSBmYWN0LCB0aGF0IG5vIGRlbnRyaWVzIHByZXNl bnQuDQo+ID4gTG9va3MgbGlrZSB0aGUgcHJvYmxlbSB3YXMgaW50cm9kdWNlZCBieSBtZSBpbiBh YWQ5NDg3Yy4uLg0KPiA+IFNvIG1heWJlIHdlIHNob3VsZCBub3QgY2FsbCAiY29udGludWUiIGlu c3RlYWQgIl9fcnBjX3BpcGVmc19ldmVudCgpIiwgd2hlbiANCj4gPiBjbGllbnQgYmVjb21lcyBy ZWFkeT8NCj4gPiBMb29rcyBsaWtlIHRoaXMgd2lsbCBhbGxvdyB1cyB0byBoYW5kbGUgc3VjaCBy YWNlcy4NCj4gDQo+IExldCBtZSByZXdvcmsgdGhpcyBwYXRjaCBhIGJpdC4uLg0KDQpUaGUgZm9s bG93aW5nIGlzIHVnbHksIGJ1dCBpdCBzaG91bGQgYmUgZGVtb25zdHJhYmx5IGNvcnJlY3QsIGFu ZCB3aWxsDQplbnN1cmUgdGhhdCBfX3JwY19waXBlZnNfZXZlbnQoKSB3aWxsIG9ubHkgYmUgY2Fs bGVkIGZvciBmdWxseQ0KaW5pdGlhbGlzZWQgbmZzX2NsaWVudHMuLi4NCg0KODwtLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0NCkZyb20gOTBjM2I5ZmU5ZmFlYWUzMmM4ZjYyOWU4YjZjYmY1ZjUwYmI5YjI5NSBNb24gU2Vw IDE3IDAwOjAwOjAwIDIwMDENCkZyb206IFRyb25kIE15a2xlYnVzdCA8VHJvbmQuTXlrbGVidXN0 QG5ldGFwcC5jb20+DQpEYXRlOiBUdWUsIDIyIE1heSAyMDEyIDE2OjIyOjUwIC0wNDAwDQpTdWJq ZWN0OiBbUEFUQ0ggMS8yXSBORlN2NDogRml4IGEgcmFjZSBpbiB0aGUgbmV0IG5hbWVzcGFjZSBt b3VudA0KIG5vdGlmaWNhdGlvbg0KDQpTaW5jZSB0aGUgc3RydWN0IG5mc19jbGllbnQgZ2V0cyBh ZGRlZCB0byB0aGUgZ2xvYmFsIG5mc19jbGllbnRfbGlzdA0KYmVmb3JlIGl0IGlzIGluaXRpYWxp c2VkLCBpdCBpcyBwb3NzaWJsZSB0aGF0IHJwY19waXBlZnNfZXZlbnQgY2FuDQplbmQgdXAgdHJ5 aW5nIHRvIGNyZWF0ZSBpZG1hcHBlciBlbnRyaWVzIG9uIHN1Y2ggYSB0aGluZy4NCg0KVGhlIHNv bHV0aW9uIGlzIHRvIGhhdmUgdGhlIG1vdW50IG5vdGlmaWNhdGlvbiB3YWl0IGZvciB0aGUNCmlu aXRpYWxpc2F0aW9uIG9mIGVhY2ggbmZzX2NsaWVudCB0byBjb21wbGV0ZSwgYW5kIHRoZW4gdG8N CnNraXAgYW55IGVudHJpZXMgZm9yIHdoaWNoIHRoZSBpdCBmYWlsZWQuDQoNClJlcG9ydGVkLWJ5 OiBTdGFuaXNsYXYgS2luc2J1cnNreSA8c2tpbnNidXJza3lAcGFyYWxsZWxzLmNvbT4NClNpZ25l ZC1vZmYtYnk6IFRyb25kIE15a2xlYnVzdCA8VHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20+DQot LS0NCiBmcy9uZnMvY2xpZW50LmMgICB8ICAgMTAgKysrKysrKysrKw0KIGZzL25mcy9pZG1hcC5j ICAgIHwgICAxNSArKysrKysrKysrKysrKysNCiBmcy9uZnMvaW50ZXJuYWwuaCB8ICAgIDEgKw0K IDMgZmlsZXMgY2hhbmdlZCwgMjYgaW5zZXJ0aW9ucygrKSwgMCBkZWxldGlvbnMoLSkNCg0KZGlm ZiAtLWdpdCBhL2ZzL25mcy9jbGllbnQuYyBiL2ZzL25mcy9jbGllbnQuYw0KaW5kZXggNjBmN2U0 ZS4uZDNjODU1MyAxMDA2NDQNCi0tLSBhL2ZzL25mcy9jbGllbnQuYw0KKysrIGIvZnMvbmZzL2Ns aWVudC5jDQpAQCAtNTgzLDYgKzU4MywxNiBAQCBmb3VuZF9jbGllbnQ6DQogCXJldHVybiBjbHA7 DQogfQ0KIA0KK3N0YXRpYyBib29sIG5mc19jbGllbnRfcmVhZHkoY29uc3Qgc3RydWN0IG5mc19j bGllbnQgKmNscCkNCit7DQorCXJldHVybiBjbHAtPmNsX2NvbnNfc3RhdGUgPD0gTkZTX0NTX1JF QURZOw0KK30NCisNCitpbnQgbmZzX3dhaXRfY2xpZW50X3JlYWR5KGNvbnN0IHN0cnVjdCBuZnNf Y2xpZW50ICpjbHApDQorew0KKwlyZXR1cm4gd2FpdF9ldmVudF9raWxsYWJsZShuZnNfY2xpZW50 X2FjdGl2ZV93cSwgbmZzX2NsaWVudF9yZWFkeShjbHApKTsNCit9DQorDQogLyoNCiAgKiBNYXJr IGEgc2VydmVyIGFzIHJlYWR5IG9yIGZhaWxlZA0KICAqLw0KZGlmZiAtLWdpdCBhL2ZzL25mcy9p ZG1hcC5jIGIvZnMvbmZzL2lkbWFwLmMNCmluZGV4IDNlOGVkYmUuLmMwNzUzYzUgMTAwNjQ0DQot LS0gYS9mcy9uZnMvaWRtYXAuYw0KKysrIGIvZnMvbmZzL2lkbWFwLmMNCkBAIC01MzAsOSArNTMw LDI0IEBAIHN0YXRpYyBzdHJ1Y3QgbmZzX2NsaWVudCAqbmZzX2dldF9jbGllbnRfZm9yX2V2ZW50 KHN0cnVjdCBuZXQgKm5ldCwgaW50IGV2ZW50KQ0KIAlzdHJ1Y3QgbmZzX25ldCAqbm4gPSBuZXRf Z2VuZXJpYyhuZXQsIG5mc19uZXRfaWQpOw0KIAlzdHJ1Y3QgZGVudHJ5ICpjbF9kZW50cnk7DQog CXN0cnVjdCBuZnNfY2xpZW50ICpjbHA7DQorCWludCBlcnI7DQogDQorcmVzdGFydDoNCiAJc3Bp bl9sb2NrKCZubi0+bmZzX2NsaWVudF9sb2NrKTsNCiAJbGlzdF9mb3JfZWFjaF9lbnRyeShjbHAs ICZubi0+bmZzX2NsaWVudF9saXN0LCBjbF9zaGFyZV9saW5rKSB7DQorCQkvKiBXYWl0IGZvciBp bml0aWFsaXNhdGlvbiB0byBmaW5pc2ggKi8NCisJCWlmIChjbHAtPmNsX2NvbnNfc3RhdGUgPiBO RlNfQ1NfUkVBRFkpIHsNCisJCQlhdG9taWNfaW5jKCZjbHAtPmNsX2NvdW50KTsNCisJCQlzcGlu X3VubG9jaygmbm4tPm5mc19jbGllbnRfbG9jayk7DQorCQkJZXJyID0gbmZzX3dhaXRfY2xpZW50 X3JlYWR5KGNscCk7DQorCQkJbmZzX3B1dF9jbGllbnQoY2xwKTsNCisJCQlpZiAoZXJyKQ0KKwkJ CQlyZXR1cm4gTlVMTDsNCisJCQlnb3RvIHJlc3RhcnQ7DQorCQl9DQorCQkvKiBTa2lwIG5mc19j bGllbnRzIHRoYXQgZmFpbGVkIHRvIGluaXRpYWxpc2UgKi8NCisJCWlmIChjbHAtPmNsX2NvbnNf c3RhdGUgPCAwKQ0KKwkJCWNvbnRpbnVlOw0KIAkJaWYgKGNscC0+cnBjX29wcyAhPSAmbmZzX3Y0 X2NsaWVudG9wcykNCiAJCQljb250aW51ZTsNCiAJCWNsX2RlbnRyeSA9IGNscC0+Y2xfaWRtYXAt PmlkbWFwX3BpcGUtPmRlbnRyeTsNCmRpZmYgLS1naXQgYS9mcy9uZnMvaW50ZXJuYWwuaCBiL2Zz L25mcy9pbnRlcm5hbC5oDQppbmRleCBiNzc3YmRhLi4zZWU0MDQwIDEwMDY0NA0KLS0tIGEvZnMv bmZzL2ludGVybmFsLmgNCisrKyBiL2ZzL25mcy9pbnRlcm5hbC5oDQpAQCAtMTY4LDYgKzE2OCw3 IEBAIGV4dGVybiBzdHJ1Y3QgbmZzX3NlcnZlciAqbmZzX2Nsb25lX3NlcnZlcihzdHJ1Y3QgbmZz X3NlcnZlciAqLA0KIAkJCQkJICAgc3RydWN0IG5mc19mYXR0ciAqLA0KIAkJCQkJICAgcnBjX2F1 dGhmbGF2b3JfdCk7DQogZXh0ZXJuIHZvaWQgbmZzX21hcmtfY2xpZW50X3JlYWR5KHN0cnVjdCBu ZnNfY2xpZW50ICpjbHAsIGludCBzdGF0ZSk7DQorZXh0ZXJuIGludCBuZnNfd2FpdF9jbGllbnRf cmVhZHkoY29uc3Qgc3RydWN0IG5mc19jbGllbnQgKmNscCk7DQogZXh0ZXJuIGludCBuZnM0X2No ZWNrX2NsaWVudF9yZWFkeShzdHJ1Y3QgbmZzX2NsaWVudCAqY2xwKTsNCiBleHRlcm4gc3RydWN0 IG5mc19jbGllbnQgKm5mczRfc2V0X2RzX2NsaWVudChzdHJ1Y3QgbmZzX2NsaWVudCogbWRzX2Ns cCwNCiAJCQkJCSAgICAgY29uc3Qgc3RydWN0IHNvY2thZGRyICpkc19hZGRyLA0KLS0gDQoxLjcu Ny42DQoNCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5l cg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0K DQo= ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-22 20:32 ` Myklebust, Trond @ 2012-05-23 11:30 ` Stanislav Kinsbursky 2012-05-23 12:09 ` Kinsbursky Stanislav 2012-05-23 13:57 ` Myklebust, Trond 0 siblings, 2 replies; 13+ messages in thread From: Stanislav Kinsbursky @ 2012-05-23 11:30 UTC (permalink / raw) To: Myklebust, Trond Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org On 23.05.2012 00:32, Myklebust, Trond wrote: > On Tue, 2012-05-22 at 12:43 -0400, Trond Myklebust wrote: >> On Tue, 2012-05-22 at 20:18 +0400, Stanislav Kinsbursky wrote: >>> We have another problem here. >>> nfs4_init_client() will try to create pipe dentries prior to set of NFS_CS_READY >>> to the client. And dentries will be created since semaphore is dropped and >>> per-net superblock variable is initialized already. >>> But __rpc_pipefs_event() relays on the fact, that no dentries present. >>> Looks like the problem was introduced by me in aad9487c... >>> So maybe we should not call "continue" instead "__rpc_pipefs_event()", when >>> client becomes ready? >>> Looks like this will allow us to handle such races. >> >> Let me rework this patch a bit... > > The following is ugly, but it should be demonstrably correct, and will > ensure that __rpc_pipefs_event() will only be called for fully > initialised nfs_clients... > > 8<--------------------------------------------------------------------- > From 90c3b9fe9faeae32c8f629e8b6cbf5f50bb9b295 Mon Sep 17 00:00:00 2001 > From: Trond Myklebust<Trond.Myklebust@netapp.com> > Date: Tue, 22 May 2012 16:22:50 -0400 > Subject: [PATCH 1/2] NFSv4: Fix a race in the net namespace mount > notification > > Since the struct nfs_client gets added to the global nfs_client_list > before it is initialised, it is possible that rpc_pipefs_event can > end up trying to create idmapper entries on such a thing. > > The solution is to have the mount notification wait for the > initialisation of each nfs_client to complete, and then to > skip any entries for which the it failed. > > Reported-by: Stanislav Kinsbursky<skinsbursky@parallels.com> > Signed-off-by: Trond Myklebust<Trond.Myklebust@netapp.com> > --- > fs/nfs/client.c | 10 ++++++++++ > fs/nfs/idmap.c | 15 +++++++++++++++ > fs/nfs/internal.h | 1 + > 3 files changed, 26 insertions(+), 0 deletions(-) > > diff --git a/fs/nfs/client.c b/fs/nfs/client.c > index 60f7e4e..d3c8553 100644 > --- a/fs/nfs/client.c > +++ b/fs/nfs/client.c > @@ -583,6 +583,16 @@ found_client: > return clp; > } > > +static bool nfs_client_ready(const struct nfs_client *clp) > +{ > + return clp->cl_cons_state<= NFS_CS_READY; > +} > + > +int nfs_wait_client_ready(const struct nfs_client *clp) > +{ > + return wait_event_killable(nfs_client_active_wq, nfs_client_ready(clp)); > +} > + > /* > * Mark a server as ready or failed > */ > diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c > index 3e8edbe..c0753c5 100644 > --- a/fs/nfs/idmap.c > +++ b/fs/nfs/idmap.c > @@ -530,9 +530,24 @@ static struct nfs_client *nfs_get_client_for_event(struct net *net, int event) > struct nfs_net *nn = net_generic(net, nfs_net_id); > struct dentry *cl_dentry; > struct nfs_client *clp; > + int err; > > +restart: > spin_lock(&nn->nfs_client_lock); > list_for_each_entry(clp,&nn->nfs_client_list, cl_share_link) { > + /* Wait for initialisation to finish */ > + if (clp->cl_cons_state> NFS_CS_READY) { > + atomic_inc(&clp->cl_count); > + spin_unlock(&nn->nfs_client_lock); > + err = nfs_wait_client_ready(clp); What about NFSv4.1 ? It's clients NFS_CS_READY status depends on session establishing RPC calls... Which in turn can hung up pipefs mount call... Moreover, looks like pipefs dentries creation have to be synchronized by nfs_client_lock somehow... Otherwise because of races we can get a client without pipe dentry.... > + nfs_put_client(clp); > + if (err) > + return NULL; > + goto restart; > + } > + /* Skip nfs_clients that failed to initialise */ > + if (clp->cl_cons_state< 0) > + continue; > if (clp->rpc_ops !=&nfs_v4_clientops) > continue; > cl_dentry = clp->cl_idmap->idmap_pipe->dentry; > diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h > index b777bda..3ee4040 100644 > --- a/fs/nfs/internal.h > +++ b/fs/nfs/internal.h > @@ -168,6 +168,7 @@ extern struct nfs_server *nfs_clone_server(struct nfs_server *, > struct nfs_fattr *, > rpc_authflavor_t); > extern void nfs_mark_client_ready(struct nfs_client *clp, int state); > +extern int nfs_wait_client_ready(const struct nfs_client *clp); > extern int nfs4_check_client_ready(struct nfs_client *clp); > extern struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp, > const struct sockaddr *ds_addr, -- Best regards, Stanislav Kinsbursky ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-23 11:30 ` Stanislav Kinsbursky @ 2012-05-23 12:09 ` Kinsbursky Stanislav 2012-05-23 13:57 ` Myklebust, Trond 1 sibling, 0 replies; 13+ messages in thread From: Kinsbursky Stanislav @ 2012-05-23 12:09 UTC (permalink / raw) To: Myklebust, Trond Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org On 23.05.2012 15:30, Stanislav Kinsbursky wrote: > Moreover, looks like pipefs dentries creation have to be synchronized by > nfs_client_lock somehow... Otherwise because of races we can get a client > without pipe dentry.... Taking this back. -- Best regards, Stanislav Kinsbursky ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-23 11:30 ` Stanislav Kinsbursky 2012-05-23 12:09 ` Kinsbursky Stanislav @ 2012-05-23 13:57 ` Myklebust, Trond 1 sibling, 0 replies; 13+ messages in thread From: Myklebust, Trond @ 2012-05-23 13:57 UTC (permalink / raw) To: Stanislav Kinsbursky Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org T24gV2VkLCAyMDEyLTA1LTIzIGF0IDE1OjMwICswNDAwLCBTdGFuaXNsYXYgS2luc2J1cnNreSB3 cm90ZToNCj4gT24gMjMuMDUuMjAxMiAwMDozMiwgTXlrbGVidXN0LCBUcm9uZCB3cm90ZToNCj4g PiBPbiBUdWUsIDIwMTItMDUtMjIgYXQgMTI6NDMgLTA0MDAsIFRyb25kIE15a2xlYnVzdCB3cm90 ZToNCj4gPj4gT24gVHVlLCAyMDEyLTA1LTIyIGF0IDIwOjE4ICswNDAwLCBTdGFuaXNsYXYgS2lu c2J1cnNreSB3cm90ZToNCj4gPj4+IFdlIGhhdmUgYW5vdGhlciBwcm9ibGVtIGhlcmUuDQo+ID4+ PiBuZnM0X2luaXRfY2xpZW50KCkgd2lsbCB0cnkgdG8gY3JlYXRlIHBpcGUgZGVudHJpZXMgcHJp b3IgdG8gc2V0IG9mIE5GU19DU19SRUFEWQ0KPiA+Pj4gdG8gdGhlIGNsaWVudC4gQW5kIGRlbnRy aWVzIHdpbGwgYmUgY3JlYXRlZCBzaW5jZSBzZW1hcGhvcmUgaXMgZHJvcHBlZCBhbmQNCj4gPj4+ IHBlci1uZXQgc3VwZXJibG9jayB2YXJpYWJsZSBpcyBpbml0aWFsaXplZCBhbHJlYWR5Lg0KPiA+ Pj4gQnV0IF9fcnBjX3BpcGVmc19ldmVudCgpIHJlbGF5cyBvbiB0aGUgZmFjdCwgdGhhdCBubyBk ZW50cmllcyBwcmVzZW50Lg0KPiA+Pj4gTG9va3MgbGlrZSB0aGUgcHJvYmxlbSB3YXMgaW50cm9k dWNlZCBieSBtZSBpbiBhYWQ5NDg3Yy4uLg0KPiA+Pj4gU28gbWF5YmUgd2Ugc2hvdWxkIG5vdCBj YWxsICJjb250aW51ZSIgaW5zdGVhZCAiX19ycGNfcGlwZWZzX2V2ZW50KCkiLCB3aGVuDQo+ID4+ PiBjbGllbnQgYmVjb21lcyByZWFkeT8NCj4gPj4+IExvb2tzIGxpa2UgdGhpcyB3aWxsIGFsbG93 IHVzIHRvIGhhbmRsZSBzdWNoIHJhY2VzLg0KPiA+Pg0KPiA+PiBMZXQgbWUgcmV3b3JrIHRoaXMg cGF0Y2ggYSBiaXQuLi4NCj4gPg0KPiA+IFRoZSBmb2xsb3dpbmcgaXMgdWdseSwgYnV0IGl0IHNo b3VsZCBiZSBkZW1vbnN0cmFibHkgY29ycmVjdCwgYW5kIHdpbGwNCj4gPiBlbnN1cmUgdGhhdCBf X3JwY19waXBlZnNfZXZlbnQoKSB3aWxsIG9ubHkgYmUgY2FsbGVkIGZvciBmdWxseQ0KPiA+IGlu aXRpYWxpc2VkIG5mc19jbGllbnRzLi4uDQo+ID4NCj4gPiA4PC0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQ0KPiA+ICBG cm9tIDkwYzNiOWZlOWZhZWFlMzJjOGY2MjllOGI2Y2JmNWY1MGJiOWIyOTUgTW9uIFNlcCAxNyAw MDowMDowMCAyMDAxDQo+ID4gRnJvbTogVHJvbmQgTXlrbGVidXN0PFRyb25kLk15a2xlYnVzdEBu ZXRhcHAuY29tPg0KPiA+IERhdGU6IFR1ZSwgMjIgTWF5IDIwMTIgMTY6MjI6NTAgLTA0MDANCj4g PiBTdWJqZWN0OiBbUEFUQ0ggMS8yXSBORlN2NDogRml4IGEgcmFjZSBpbiB0aGUgbmV0IG5hbWVz cGFjZSBtb3VudA0KPiA+ICAgbm90aWZpY2F0aW9uDQo+ID4NCj4gPiBTaW5jZSB0aGUgc3RydWN0 IG5mc19jbGllbnQgZ2V0cyBhZGRlZCB0byB0aGUgZ2xvYmFsIG5mc19jbGllbnRfbGlzdA0KPiA+ IGJlZm9yZSBpdCBpcyBpbml0aWFsaXNlZCwgaXQgaXMgcG9zc2libGUgdGhhdCBycGNfcGlwZWZz X2V2ZW50IGNhbg0KPiA+IGVuZCB1cCB0cnlpbmcgdG8gY3JlYXRlIGlkbWFwcGVyIGVudHJpZXMg b24gc3VjaCBhIHRoaW5nLg0KPiA+DQo+ID4gVGhlIHNvbHV0aW9uIGlzIHRvIGhhdmUgdGhlIG1v dW50IG5vdGlmaWNhdGlvbiB3YWl0IGZvciB0aGUNCj4gPiBpbml0aWFsaXNhdGlvbiBvZiBlYWNo IG5mc19jbGllbnQgdG8gY29tcGxldGUsIGFuZCB0aGVuIHRvDQo+ID4gc2tpcCBhbnkgZW50cmll cyBmb3Igd2hpY2ggdGhlIGl0IGZhaWxlZC4NCj4gPg0KPiA+IFJlcG9ydGVkLWJ5OiBTdGFuaXNs YXYgS2luc2J1cnNreTxza2luc2J1cnNreUBwYXJhbGxlbHMuY29tPg0KPiA+IFNpZ25lZC1vZmYt Ynk6IFRyb25kIE15a2xlYnVzdDxUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbT4NCj4gPiAtLS0N Cj4gPiAgIGZzL25mcy9jbGllbnQuYyAgIHwgICAxMCArKysrKysrKysrDQo+ID4gICBmcy9uZnMv aWRtYXAuYyAgICB8ICAgMTUgKysrKysrKysrKysrKysrDQo+ID4gICBmcy9uZnMvaW50ZXJuYWwu aCB8ICAgIDEgKw0KPiA+ICAgMyBmaWxlcyBjaGFuZ2VkLCAyNiBpbnNlcnRpb25zKCspLCAwIGRl bGV0aW9ucygtKQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2ZzL25mcy9jbGllbnQuYyBiL2ZzL25m cy9jbGllbnQuYw0KPiA+IGluZGV4IDYwZjdlNGUuLmQzYzg1NTMgMTAwNjQ0DQo+ID4gLS0tIGEv ZnMvbmZzL2NsaWVudC5jDQo+ID4gKysrIGIvZnMvbmZzL2NsaWVudC5jDQo+ID4gQEAgLTU4Myw2 ICs1ODMsMTYgQEAgZm91bmRfY2xpZW50Og0KPiA+ICAgCXJldHVybiBjbHA7DQo+ID4gICB9DQo+ ID4NCj4gPiArc3RhdGljIGJvb2wgbmZzX2NsaWVudF9yZWFkeShjb25zdCBzdHJ1Y3QgbmZzX2Ns aWVudCAqY2xwKQ0KPiA+ICt7DQo+ID4gKwlyZXR1cm4gY2xwLT5jbF9jb25zX3N0YXRlPD0gTkZT X0NTX1JFQURZOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICtpbnQgbmZzX3dhaXRfY2xpZW50X3JlYWR5 KGNvbnN0IHN0cnVjdCBuZnNfY2xpZW50ICpjbHApDQo+ID4gK3sNCj4gPiArCXJldHVybiB3YWl0 X2V2ZW50X2tpbGxhYmxlKG5mc19jbGllbnRfYWN0aXZlX3dxLCBuZnNfY2xpZW50X3JlYWR5KGNs cCkpOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICAgLyoNCj4gPiAgICAqIE1hcmsgYSBzZXJ2ZXIgYXMg cmVhZHkgb3IgZmFpbGVkDQo+ID4gICAgKi8NCj4gPiBkaWZmIC0tZ2l0IGEvZnMvbmZzL2lkbWFw LmMgYi9mcy9uZnMvaWRtYXAuYw0KPiA+IGluZGV4IDNlOGVkYmUuLmMwNzUzYzUgMTAwNjQ0DQo+ ID4gLS0tIGEvZnMvbmZzL2lkbWFwLmMNCj4gPiArKysgYi9mcy9uZnMvaWRtYXAuYw0KPiA+IEBA IC01MzAsOSArNTMwLDI0IEBAIHN0YXRpYyBzdHJ1Y3QgbmZzX2NsaWVudCAqbmZzX2dldF9jbGll bnRfZm9yX2V2ZW50KHN0cnVjdCBuZXQgKm5ldCwgaW50IGV2ZW50KQ0KPiA+ICAgCXN0cnVjdCBu ZnNfbmV0ICpubiA9IG5ldF9nZW5lcmljKG5ldCwgbmZzX25ldF9pZCk7DQo+ID4gICAJc3RydWN0 IGRlbnRyeSAqY2xfZGVudHJ5Ow0KPiA+ICAgCXN0cnVjdCBuZnNfY2xpZW50ICpjbHA7DQo+ID4g KwlpbnQgZXJyOw0KPiA+DQo+ID4gK3Jlc3RhcnQ6DQo+ID4gICAJc3Bpbl9sb2NrKCZubi0+bmZz X2NsaWVudF9sb2NrKTsNCj4gPiAgIAlsaXN0X2Zvcl9lYWNoX2VudHJ5KGNscCwmbm4tPm5mc19j bGllbnRfbGlzdCwgY2xfc2hhcmVfbGluaykgew0KPiA+ICsJCS8qIFdhaXQgZm9yIGluaXRpYWxp c2F0aW9uIHRvIGZpbmlzaCAqLw0KPiA+ICsJCWlmIChjbHAtPmNsX2NvbnNfc3RhdGU+ICBORlNf Q1NfUkVBRFkpIHsNCj4gPiArCQkJYXRvbWljX2luYygmY2xwLT5jbF9jb3VudCk7DQo+ID4gKwkJ CXNwaW5fdW5sb2NrKCZubi0+bmZzX2NsaWVudF9sb2NrKTsNCj4gPiArCQkJZXJyID0gbmZzX3dh aXRfY2xpZW50X3JlYWR5KGNscCk7DQo+IA0KPiANCj4gV2hhdCBhYm91dCBORlN2NC4xID8NCj4g SXQncyBjbGllbnRzIE5GU19DU19SRUFEWSBzdGF0dXMgZGVwZW5kcyBvbiBzZXNzaW9uIGVzdGFi bGlzaGluZyBSUEMgY2FsbHMuLi4NCj4gV2hpY2ggaW4gdHVybiBjYW4gaHVuZyB1cCBwaXBlZnMg bW91bnQgY2FsbC4uLg0KDQpUaGUgYWx0ZXJuYXRpdmUgdGhlbiBpcyB0byB3YWl0IGZvciBjbF9j b25zX3N0YXRlICE9IE5GU19DU19JTklUSU5HLg0KVGhhdCBzaG91bGRuJ3QgcmVxdWlyZSBhbnkg dXBjYWxscywgYW5kIHNvIHNob3VsZG4ndCBiZSBhYmxlIHRvDQpkZWFkbG9jay4NCg0KLS0gDQpU cm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRy b25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0KDQo= ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] NFS: init client before declaration 2012-05-22 14:29 ` Myklebust, Trond 2012-05-22 15:00 ` Myklebust, Trond @ 2012-05-22 15:03 ` Stanislav Kinsbursky 1 sibling, 0 replies; 13+ messages in thread From: Stanislav Kinsbursky @ 2012-05-22 15:03 UTC (permalink / raw) To: Myklebust, Trond Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org On 22.05.2012 18:29, Myklebust, Trond wrote: > On Tue, 2012-05-22 at 16:40 +0400, Stanislav Kinsbursky wrote: >> Client have to be initialized prior to adding it to per-net clients list, >> because otherwise there are races, shown below: >> >> CPU#0 CPU#1 >> _____ _____ >> >> nfs_get_client >> nfs_alloc_client >> list_add(..., nfs_client_list) >> rpc_fill_super >> rpc_pipefs_event >> nfs_get_client_for_event >> __rpc_pipefs_event >> (clp->cl_rpcclient is uninitialized) >> BUG() >> init_client >> clp->cl_rpcclient = ... >> > > Why not simply change nfs_get_client_for_event() so that it doesn't > touch nfs_clients that have clp->cl_cons_state!=NFS_CS_READY? > > That should ensure that it doesn't touch nfs_clients that failed to > initialise and/or are still in the process of being initialised. > It looks like in this case we will have another races: CPU#0 CPU#1 _____ _____ nfs4_init_client nfs_idmap_new nfs_idmap_register rpc_get_sb_net (fail - no pipefs) rpc_fill_super rpc_pipefs_event nfs_get_client_for_event (skip client - NFS_CS_READY is not set) nfs_mark_client_ready(NFS_CS_READY) And we are having client without idmap pipe... > Cheers > Trond > -- Best regards, Stanislav Kinsbursky ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-05-23 13:57 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-22 12:40 [PATCH] NFS: init client before declaration Stanislav Kinsbursky 2012-05-22 13:47 ` Chuck Lever 2012-05-22 14:29 ` Myklebust, Trond 2012-05-22 15:00 ` Myklebust, Trond 2012-05-22 15:29 ` Stanislav Kinsbursky 2012-05-22 15:51 ` Myklebust, Trond 2012-05-22 16:18 ` Stanislav Kinsbursky 2012-05-22 16:43 ` Myklebust, Trond 2012-05-22 20:32 ` Myklebust, Trond 2012-05-23 11:30 ` Stanislav Kinsbursky 2012-05-23 12:09 ` Kinsbursky Stanislav 2012-05-23 13:57 ` Myklebust, Trond 2012-05-22 15:03 ` Stanislav Kinsbursky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).