* [PATCH] SUNRPC: Remove redundant call to cancel_work_sync() in xprt_destroy() @ 2017-10-11 18:01 Trond Myklebust 2017-10-12 13:47 ` Lorenzo Pieralisi 0 siblings, 1 reply; 4+ messages in thread From: Trond Myklebust @ 2017-10-11 18:01 UTC (permalink / raw) To: Anna Schumaker; +Cc: Lorenzo Pieralisi, linux-nfs We know that the socket autoclose cannot be queued after we've set the XPRT_LOCKED bit, so the call to cancel_work_sync() is redundant. In addition, it is causing lockdep to complain about a false ABA lock dependency. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- net/sunrpc/xprt.c | 1 - 1 file changed, 1 deletion(-) diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c index e741ec2b4d8e..5f12fe145f02 100644 --- a/net/sunrpc/xprt.c +++ b/net/sunrpc/xprt.c @@ -1464,7 +1464,6 @@ static void xprt_destroy(struct rpc_xprt *xprt) rpc_destroy_wait_queue(&xprt->pending); rpc_destroy_wait_queue(&xprt->sending); rpc_destroy_wait_queue(&xprt->backlog); - cancel_work_sync(&xprt->task_cleanup); kfree(xprt->servername); /* * Tear down transport state and free the rpc_xprt -- 2.13.6 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] SUNRPC: Remove redundant call to cancel_work_sync() in xprt_destroy() 2017-10-11 18:01 [PATCH] SUNRPC: Remove redundant call to cancel_work_sync() in xprt_destroy() Trond Myklebust @ 2017-10-12 13:47 ` Lorenzo Pieralisi 2017-10-19 18:49 ` Trond Myklebust 0 siblings, 1 reply; 4+ messages in thread From: Lorenzo Pieralisi @ 2017-10-12 13:47 UTC (permalink / raw) To: Trond Myklebust; +Cc: Anna Schumaker, linux-nfs Hi Trond, On Wed, Oct 11, 2017 at 02:01:34PM -0400, Trond Myklebust wrote: > We know that the socket autoclose cannot be queued after we've set > the XPRT_LOCKED bit, so the call to cancel_work_sync() is redundant. > In addition, it is causing lockdep to complain about a false ABA > lock dependency. > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > net/sunrpc/xprt.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c > index e741ec2b4d8e..5f12fe145f02 100644 > --- a/net/sunrpc/xprt.c > +++ b/net/sunrpc/xprt.c > @@ -1464,7 +1464,6 @@ static void xprt_destroy(struct rpc_xprt *xprt) > rpc_destroy_wait_queue(&xprt->pending); > rpc_destroy_wait_queue(&xprt->sending); > rpc_destroy_wait_queue(&xprt->backlog); > - cancel_work_sync(&xprt->task_cleanup); This does not make the lockdep warning go away, actually the lockdep is triggered by the xs_destroy() cancel_work_sync() call but I do not know this code path so I can't really comment on it, let me know if there is any specific test I can carry out. Thanks for looking into this, Lorenzo Log: [ 6.223423] ====================================================== [ 6.229611] WARNING: possible circular locking dependency detected [ 6.235801] 4.14.0-rc4-00001-g8ee3d7b #64 Not tainted [ 6.240856] ------------------------------------------------------ [ 6.247044] kworker/1:0H/17 is trying to acquire lock: [ 6.252188] ((&task->u.tk_work)){+.+.}, at: [<ffff0000080e64cc>] process_one_work+0x1cc/0x3f0 [ 6.260836] but task is already holding lock: [ 6.266676] ("xprtiod"){+.+.}, at: [<ffff0000080e64cc>] process_one_work+0x1cc/0x3f0 [ 6.274531] which lock already depends on the new lock. [ 6.282723] the existing dependency chain (in reverse order) is: [ 6.290217] -> #1 ("xprtiod"){+.+.}: [ 6.295292] lock_acquire+0x6c/0xb8 [ 6.299307] flush_work+0x188/0x270 [ 6.303321] __cancel_work_timer+0x120/0x198 [ 6.308119] cancel_work_sync+0x10/0x18 [ 6.312484] xs_destroy+0x34/0x58 [ 6.316324] xprt_destroy+0x7c/0x88 [ 6.320338] xprt_put+0x34/0x40 [ 6.324004] rpc_task_release_client+0x6c/0x80 [ 6.328976] rpc_release_resources_task+0x2c/0x38 [ 6.334210] __rpc_execute+0x9c/0x210 [ 6.338399] rpc_async_schedule+0x10/0x18 [ 6.342935] process_one_work+0x240/0x3f0 [ 6.347471] worker_thread+0x48/0x420 [ 6.351661] kthread+0x12c/0x158 [ 6.355416] ret_from_fork+0x10/0x18 [ 6.359514] -> #0 ((&task->u.tk_work)){+.+.}: [ 6.365372] __lock_acquire+0x12ec/0x14a8 [ 6.369908] lock_acquire+0x6c/0xb8 [ 6.373922] process_one_work+0x22c/0x3f0 [ 6.378459] worker_thread+0x48/0x420 [ 6.382648] kthread+0x12c/0x158 [ 6.386401] ret_from_fork+0x10/0x18 [ 6.390499] other info that might help us debug this: [ 6.398518] Possible unsafe locking scenario: [ 6.404445] CPU0 CPU1 [ 6.408978] ---- ---- [ 6.413511] lock("xprtiod"); [ 6.416571] lock((&task->u.tk_work)); [ 6.422938] lock("xprtiod"); [ 6.428521] lock((&task->u.tk_work)); [ 6.432364] *** DEADLOCK *** [ 6.438295] 1 lock held by kworker/1:0H/17: [ 6.442480] #0: ("xprtiod"){+.+.}, at: [<ffff0000080e64cc>] process_one_work+0x1cc/0x3f0 [ 6.450773] stack backtrace: [ 6.455138] CPU: 1 PID: 17 Comm: kworker/1:0H Not tainted 4.14.0-rc4-00001-g8ee3d7b #64 [ 6.463153] Hardware name: ARM Juno development board (r2) (DT) [ 6.469084] Workqueue: xprtiod rpc_async_schedule [ 6.473795] Call trace: [ 6.476246] [<ffff000008089430>] dump_backtrace+0x0/0x3c8 [ 6.481654] [<ffff00000808980c>] show_stack+0x14/0x20 [ 6.486715] [<ffff000008a01a30>] dump_stack+0xb8/0xf0 [ 6.491776] [<ffff0000081194ac>] print_circular_bug+0x224/0x3a0 [ 6.497706] [<ffff00000811a304>] check_prev_add+0x304/0x860 [ 6.503288] [<ffff00000811c8c4>] __lock_acquire+0x12ec/0x14a8 [ 6.509043] [<ffff00000811d144>] lock_acquire+0x6c/0xb8 [ 6.514276] [<ffff0000080e652c>] process_one_work+0x22c/0x3f0 [ 6.520031] [<ffff0000080e6738>] worker_thread+0x48/0x420 [ 6.525439] [<ffff0000080ed5bc>] kthread+0x12c/0x158 [ 6.530411] [<ffff000008084d48>] ret_from_fork+0x10/0x18 [ 7.446907] systemd[1]: systemd 232 running in system mode. (+PAM +AUDIT +SELINUX +IMA +APPARMOR +SMACK +SYSVINIT +UTMP +LIBCRYPTSETUP +GCRYPT +GNUTLS +ACL +XZ +LZ4 +SECCOMP +BLKID +ELFUTILS +KMOD +IDN) [ 7.465397] systemd[1]: Detected architecture arm64. [ 7.487124] systemd[1]: Set hostname to <localhost.localdomain>. [ 8.016682] systemd[1]: Listening on Journal Socket. [ 8.034382] systemd[1]: Listening on udev Control Socket. [ 8.053967] systemd[1]: Listening on Syslog Socket. [ 8.069953] systemd[1]: Listening on udev Kernel Socket. [ 8.089774] systemd[1]: Reached target Remote File Systems. [ 8.110534] systemd[1]: Created slice User and Session Slice. [ 8.130143] systemd[1]: Listening on Journal Audit Socket. [ 8.781699] systemd-journald[1479]: Received request to flush runtime journal from PID 1 [ 9.685766] sky2 0000:09:00.0 enp9s0: renamed from eth4 [ 9.778209] igb 0000:07:00.2 enp7s0f2: renamed from eth2 [ 9.802564] igb 0000:07:00.1 enp7s0f1: renamed from eth1 [ 9.822138] igb 0000:07:00.3 enp7s0f3: renamed from eth3 [ 9.838393] igb 0000:07:00.0 enp7s0f0: renamed from eth0 [ 54.271348] random: crng init done know this code path at all so I can't really comment on it, let me know if I c ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] SUNRPC: Remove redundant call to cancel_work_sync() in xprt_destroy() 2017-10-12 13:47 ` Lorenzo Pieralisi @ 2017-10-19 18:49 ` Trond Myklebust 2017-10-20 15:06 ` Lorenzo Pieralisi 0 siblings, 1 reply; 4+ messages in thread From: Trond Myklebust @ 2017-10-19 18:49 UTC (permalink / raw) To: lorenzo.pieralisi@arm.com Cc: anna.schumaker@netapp.com, linux-nfs@vger.kernel.org T24gVGh1LCAyMDE3LTEwLTEyIGF0IDE0OjQ3ICswMTAwLCBMb3JlbnpvIFBpZXJhbGlzaSB3cm90 ZToNCj4gSGkgVHJvbmQsDQo+IA0KPiBPbiBXZWQsIE9jdCAxMSwgMjAxNyBhdCAwMjowMTozNFBN IC0wNDAwLCBUcm9uZCBNeWtsZWJ1c3Qgd3JvdGU6DQo+ID4gV2Uga25vdyB0aGF0IHRoZSBzb2Nr ZXQgYXV0b2Nsb3NlIGNhbm5vdCBiZSBxdWV1ZWQgYWZ0ZXIgd2UndmUgc2V0DQo+ID4gdGhlIFhQ UlRfTE9DS0VEIGJpdCwgc28gdGhlIGNhbGwgdG8gY2FuY2VsX3dvcmtfc3luYygpIGlzDQo+ID4g cmVkdW5kYW50Lg0KPiA+IEluIGFkZGl0aW9uLCBpdCBpcyBjYXVzaW5nIGxvY2tkZXAgdG8gY29t cGxhaW4gYWJvdXQgYSBmYWxzZSBBQkENCj4gPiBsb2NrIGRlcGVuZGVuY3kuDQo+ID4gDQo+ID4g U2lnbmVkLW9mZi1ieTogVHJvbmQgTXlrbGVidXN0IDx0cm9uZC5teWtsZWJ1c3RAcHJpbWFyeWRh dGEuY29tPg0KPiA+IC0tLQ0KPiA+ICBuZXQvc3VucnBjL3hwcnQuYyB8IDEgLQ0KPiA+ICAxIGZp bGUgY2hhbmdlZCwgMSBkZWxldGlvbigtKQ0KPiA+IA0KPiA+IGRpZmYgLS1naXQgYS9uZXQvc3Vu cnBjL3hwcnQuYyBiL25ldC9zdW5ycGMveHBydC5jDQo+ID4gaW5kZXggZTc0MWVjMmI0ZDhlLi41 ZjEyZmUxNDVmMDIgMTAwNjQ0DQo+ID4gLS0tIGEvbmV0L3N1bnJwYy94cHJ0LmMNCj4gPiArKysg Yi9uZXQvc3VucnBjL3hwcnQuYw0KPiA+IEBAIC0xNDY0LDcgKzE0NjQsNiBAQCBzdGF0aWMgdm9p ZCB4cHJ0X2Rlc3Ryb3koc3RydWN0IHJwY194cHJ0DQo+ID4gKnhwcnQpDQo+ID4gIAlycGNfZGVz dHJveV93YWl0X3F1ZXVlKCZ4cHJ0LT5wZW5kaW5nKTsNCj4gPiAgCXJwY19kZXN0cm95X3dhaXRf cXVldWUoJnhwcnQtPnNlbmRpbmcpOw0KPiA+ICAJcnBjX2Rlc3Ryb3lfd2FpdF9xdWV1ZSgmeHBy dC0+YmFja2xvZyk7DQo+ID4gLQljYW5jZWxfd29ya19zeW5jKCZ4cHJ0LT50YXNrX2NsZWFudXAp Ow0KPiANCj4gVGhpcyBkb2VzIG5vdCBtYWtlIHRoZSBsb2NrZGVwIHdhcm5pbmcgZ28gYXdheSwg YWN0dWFsbHkgdGhlIGxvY2tkZXANCj4gaXMgdHJpZ2dlcmVkIGJ5IHRoZSB4c19kZXN0cm95KCkg Y2FuY2VsX3dvcmtfc3luYygpIGNhbGwgYnV0IEkgZG8gbm90DQo+IGtub3cgdGhpcyBjb2RlIHBh dGggc28gSSBjYW4ndCByZWFsbHkgY29tbWVudCBvbiBpdCwgbGV0IG1lIGtub3cgaWYNCj4gdGhl cmUgaXMgYW55IHNwZWNpZmljIHRlc3QgSSBjYW4gY2Fycnkgb3V0Lg0KPiANCj4gVGhhbmtzIGZv ciBsb29raW5nIGludG8gdGhpcywNCj4gTG9yZW56bw0KDQpTb3JyeSBmb3IgdGhlIGRlbGF5LiBE b2VzIHRoaXMgb25lIGhlbHA/DQoNCjg8LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LQ0KRnJvbSA1MjhmZDM1NDdiYWQwYmRkMzFjOGY5ODdlNWJkMDBjODNkZjhhZjM5IE1vbiBTZXAg MTcgMDA6MDA6MDAgMjAwMQ0KRnJvbTogVHJvbmQgTXlrbGVidXN0IDx0cm9uZC5teWtsZWJ1c3RA cHJpbWFyeWRhdGEuY29tPg0KRGF0ZTogVGh1LCAxOSBPY3QgMjAxNyAxMjoxMzoxMCAtMDQwMA0K U3ViamVjdDogW1BBVENIXSBTVU5SUEM6IERlc3Ryb3kgdHJhbnNwb3J0IGZyb20gdGhlIHN5c3Rl bSB3b3JrcXVldWUNCg0KVGhlIHRyYW5zcG9ydCBtYXkgbmVlZCB0byBmbHVzaCB0cmFuc3BvcnQg Y29ubmVjdCBhbmQgcmVjZWl2ZSB0YXNrcw0KdGhhdCBhcmUgcnVubmluZyBvbiBycGNpb2QuIElu IG9yZGVyIHRvIGRvIHNvIHNhZmVseSwgd2UgbmVlZCB0bw0KZW5zdXJlIHRoYXQgdGhlIGNhbGxl ciBvZiBjYW5jZWxfd29ya19zeW5jKCkgZXRjIGlzIG5vdCBpdHNlbGYNCnJ1bm5pbmcgb24gcnBj aW9kLg0KRG8gc28gYnkgcnVubmluZyB0aGUgZGVzdHJveSB0YXNrIGZyb20gdGhlIHN5c3RlbSB3 b3JrcXVldWUuDQoNClNpZ25lZC1vZmYtYnk6IFRyb25kIE15a2xlYnVzdCA8dHJvbmQubXlrbGVi dXN0QHByaW1hcnlkYXRhLmNvbT4NCi0tLQ0KIG5ldC9zdW5ycGMveHBydC5jIHwgMzQgKysrKysr KysrKysrKysrKysrKysrKysrLS0tLS0tLS0tLQ0KIDEgZmlsZSBjaGFuZ2VkLCAyNCBpbnNlcnRp b25zKCspLCAxMCBkZWxldGlvbnMoLSkNCg0KZGlmZiAtLWdpdCBhL25ldC9zdW5ycGMveHBydC5j IGIvbmV0L3N1bnJwYy94cHJ0LmMNCmluZGV4IDFhMzlhZDE0YzQyZi4uODk4NDg1ZTNlY2U0IDEw MDY0NA0KLS0tIGEvbmV0L3N1bnJwYy94cHJ0LmMNCisrKyBiL25ldC9zdW5ycGMveHBydC5jDQpA QCAtMTQ0NSw2ICsxNDQ1LDIzIEBAIHN0cnVjdCBycGNfeHBydCAqeHBydF9jcmVhdGVfdHJhbnNw b3J0KHN0cnVjdCB4cHJ0X2NyZWF0ZSAqYXJncykNCiAJcmV0dXJuIHhwcnQ7DQogfQ0KIA0KK3N0 YXRpYyB2b2lkIHhwcnRfZGVzdHJveV9jYihzdHJ1Y3Qgd29ya19zdHJ1Y3QgKndvcmspDQorew0K KwlzdHJ1Y3QgcnBjX3hwcnQgKnhwcnQgPQ0KKwkJY29udGFpbmVyX29mKHdvcmssIHN0cnVjdCBy cGNfeHBydCwgdGFza19jbGVhbnVwKTsNCisNCisJcnBjX3hwcnRfZGVidWdmc191bnJlZ2lzdGVy KHhwcnQpOw0KKwlycGNfZGVzdHJveV93YWl0X3F1ZXVlKCZ4cHJ0LT5iaW5kaW5nKTsNCisJcnBj X2Rlc3Ryb3lfd2FpdF9xdWV1ZSgmeHBydC0+cGVuZGluZyk7DQorCXJwY19kZXN0cm95X3dhaXRf cXVldWUoJnhwcnQtPnNlbmRpbmcpOw0KKwlycGNfZGVzdHJveV93YWl0X3F1ZXVlKCZ4cHJ0LT5i YWNrbG9nKTsNCisJa2ZyZWUoeHBydC0+c2VydmVybmFtZSk7DQorCS8qDQorCSAqIFRlYXIgZG93 biB0cmFuc3BvcnQgc3RhdGUgYW5kIGZyZWUgdGhlIHJwY194cHJ0DQorCSAqLw0KKwl4cHJ0LT5v cHMtPmRlc3Ryb3koeHBydCk7DQorfQ0KKw0KIC8qKg0KICAqIHhwcnRfZGVzdHJveSAtIGRlc3Ry b3kgYW4gUlBDIHRyYW5zcG9ydCwga2lsbGluZyBvZmYgYWxsIHJlcXVlc3RzLg0KICAqIEB4cHJ0 OiB0cmFuc3BvcnQgdG8gZGVzdHJveQ0KQEAgLTE0NTQsMjIgKzE0NzEsMTkgQEAgc3RhdGljIHZv aWQgeHBydF9kZXN0cm95KHN0cnVjdCBycGNfeHBydCAqeHBydCkNCiB7DQogCWRwcmludGsoIlJQ QzogICAgICAgZGVzdHJveWluZyB0cmFuc3BvcnQgJXBcbiIsIHhwcnQpOw0KIA0KLQkvKiBFeGNs dWRlIHRyYW5zcG9ydCBjb25uZWN0L2Rpc2Nvbm5lY3QgaGFuZGxlcnMgKi8NCisJLyoNCisJICog RXhjbHVkZSB0cmFuc3BvcnQgY29ubmVjdC9kaXNjb25uZWN0IGhhbmRsZXJzIGFuZCBhdXRvY2xv c2UNCisJICovDQogCXdhaXRfb25fYml0X2xvY2soJnhwcnQtPnN0YXRlLCBYUFJUX0xPQ0tFRCwg VEFTS19VTklOVEVSUlVQVElCTEUpOw0KIA0KIAlkZWxfdGltZXJfc3luYygmeHBydC0+dGltZXIp Ow0KIA0KLQlycGNfeHBydF9kZWJ1Z2ZzX3VucmVnaXN0ZXIoeHBydCk7DQotCXJwY19kZXN0cm95 X3dhaXRfcXVldWUoJnhwcnQtPmJpbmRpbmcpOw0KLQlycGNfZGVzdHJveV93YWl0X3F1ZXVlKCZ4 cHJ0LT5wZW5kaW5nKTsNCi0JcnBjX2Rlc3Ryb3lfd2FpdF9xdWV1ZSgmeHBydC0+c2VuZGluZyk7 DQotCXJwY19kZXN0cm95X3dhaXRfcXVldWUoJnhwcnQtPmJhY2tsb2cpOw0KLQljYW5jZWxfd29y a19zeW5jKCZ4cHJ0LT50YXNrX2NsZWFudXApOw0KLQlrZnJlZSh4cHJ0LT5zZXJ2ZXJuYW1lKTsN CiAJLyoNCi0JICogVGVhciBkb3duIHRyYW5zcG9ydCBzdGF0ZSBhbmQgZnJlZSB0aGUgcnBjX3hw cnQNCisJICogRGVzdHJveSBzb2NrZXRzIGV0YyBmcm9tIHRoZSBzeXN0ZW0gd29ya3F1ZXVlIHNv IHRoZXkgY2FuDQorCSAqIHNhZmVseSBmbHVzaCByZWNlaXZlIHdvcmsgcnVubmluZyBvbiBycGNp b2QuDQogCSAqLw0KLQl4cHJ0LT5vcHMtPmRlc3Ryb3koeHBydCk7DQorCUlOSVRfV09SSygmeHBy dC0+dGFza19jbGVhbnVwLCB4cHJ0X2Rlc3Ryb3lfY2IpOw0KKwlzY2hlZHVsZV93b3JrKCZ4cHJ0 LT50YXNrX2NsZWFudXApOw0KIH0NCiANCiBzdGF0aWMgdm9pZCB4cHJ0X2Rlc3Ryb3lfa3JlZihz dHJ1Y3Qga3JlZiAqa3JlZikNCi0tIA0KMi4xMy42DQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpM aW51eCBORlMgY2xpZW50IG1haW50YWluZXIsIFByaW1hcnlEYXRhDQp0cm9uZC5teWtsZWJ1c3RA cHJpbWFyeWRhdGEuY29tDQo= ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] SUNRPC: Remove redundant call to cancel_work_sync() in xprt_destroy() 2017-10-19 18:49 ` Trond Myklebust @ 2017-10-20 15:06 ` Lorenzo Pieralisi 0 siblings, 0 replies; 4+ messages in thread From: Lorenzo Pieralisi @ 2017-10-20 15:06 UTC (permalink / raw) To: Trond Myklebust; +Cc: anna.schumaker@netapp.com, linux-nfs@vger.kernel.org On Thu, Oct 19, 2017 at 06:49:18PM +0000, Trond Myklebust wrote: > On Thu, 2017-10-12 at 14:47 +0100, Lorenzo Pieralisi wrote: > > Hi Trond, > > > > On Wed, Oct 11, 2017 at 02:01:34PM -0400, Trond Myklebust wrote: > > > We know that the socket autoclose cannot be queued after we've set > > > the XPRT_LOCKED bit, so the call to cancel_work_sync() is > > > redundant. > > > In addition, it is causing lockdep to complain about a false ABA > > > lock dependency. > > > > > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > > > --- > > > net/sunrpc/xprt.c | 1 - > > > 1 file changed, 1 deletion(-) > > > > > > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c > > > index e741ec2b4d8e..5f12fe145f02 100644 > > > --- a/net/sunrpc/xprt.c > > > +++ b/net/sunrpc/xprt.c > > > @@ -1464,7 +1464,6 @@ static void xprt_destroy(struct rpc_xprt > > > *xprt) > > > rpc_destroy_wait_queue(&xprt->pending); > > > rpc_destroy_wait_queue(&xprt->sending); > > > rpc_destroy_wait_queue(&xprt->backlog); > > > - cancel_work_sync(&xprt->task_cleanup); > > > > This does not make the lockdep warning go away, actually the lockdep > > is triggered by the xs_destroy() cancel_work_sync() call but I do not > > know this code path so I can't really comment on it, let me know if > > there is any specific test I can carry out. > > > > Thanks for looking into this, > > Lorenzo > > Sorry for the delay. Does this one help? Thank you. Yes it does (on top of -rc5) - I get no lockdep warning anymore. Lorenzo > 8<---------------------------------- > From 528fd3547bad0bdd31c8f987e5bd00c83df8af39 Mon Sep 17 00:00:00 2001 > From: Trond Myklebust <trond.myklebust@primarydata.com> > Date: Thu, 19 Oct 2017 12:13:10 -0400 > Subject: [PATCH] SUNRPC: Destroy transport from the system workqueue > > The transport may need to flush transport connect and receive tasks > that are running on rpciod. In order to do so safely, we need to > ensure that the caller of cancel_work_sync() etc is not itself > running on rpciod. > Do so by running the destroy task from the system workqueue. > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > net/sunrpc/xprt.c | 34 ++++++++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 10 deletions(-) > > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c > index 1a39ad14c42f..898485e3ece4 100644 > --- a/net/sunrpc/xprt.c > +++ b/net/sunrpc/xprt.c > @@ -1445,6 +1445,23 @@ struct rpc_xprt *xprt_create_transport(struct xprt_create *args) > return xprt; > } > > +static void xprt_destroy_cb(struct work_struct *work) > +{ > + struct rpc_xprt *xprt = > + container_of(work, struct rpc_xprt, task_cleanup); > + > + rpc_xprt_debugfs_unregister(xprt); > + rpc_destroy_wait_queue(&xprt->binding); > + rpc_destroy_wait_queue(&xprt->pending); > + rpc_destroy_wait_queue(&xprt->sending); > + rpc_destroy_wait_queue(&xprt->backlog); > + kfree(xprt->servername); > + /* > + * Tear down transport state and free the rpc_xprt > + */ > + xprt->ops->destroy(xprt); > +} > + > /** > * xprt_destroy - destroy an RPC transport, killing off all requests. > * @xprt: transport to destroy > @@ -1454,22 +1471,19 @@ static void xprt_destroy(struct rpc_xprt *xprt) > { > dprintk("RPC: destroying transport %p\n", xprt); > > - /* Exclude transport connect/disconnect handlers */ > + /* > + * Exclude transport connect/disconnect handlers and autoclose > + */ > wait_on_bit_lock(&xprt->state, XPRT_LOCKED, TASK_UNINTERRUPTIBLE); > > del_timer_sync(&xprt->timer); > > - rpc_xprt_debugfs_unregister(xprt); > - rpc_destroy_wait_queue(&xprt->binding); > - rpc_destroy_wait_queue(&xprt->pending); > - rpc_destroy_wait_queue(&xprt->sending); > - rpc_destroy_wait_queue(&xprt->backlog); > - cancel_work_sync(&xprt->task_cleanup); > - kfree(xprt->servername); > /* > - * Tear down transport state and free the rpc_xprt > + * Destroy sockets etc from the system workqueue so they can > + * safely flush receive work running on rpciod. > */ > - xprt->ops->destroy(xprt); > + INIT_WORK(&xprt->task_cleanup, xprt_destroy_cb); > + schedule_work(&xprt->task_cleanup); > } > > static void xprt_destroy_kref(struct kref *kref) > -- > 2.13.6 > > -- > Trond Myklebust > Linux NFS client maintainer, PrimaryData > trond.myklebust@primarydata.com ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-20 15:06 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-11 18:01 [PATCH] SUNRPC: Remove redundant call to cancel_work_sync() in xprt_destroy() Trond Myklebust 2017-10-12 13:47 ` Lorenzo Pieralisi 2017-10-19 18:49 ` Trond Myklebust 2017-10-20 15:06 ` Lorenzo Pieralisi
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).