diff mbox

[rdma-rc,v1] RDMA/restrack: Move restrack_clean to be symmetrical to restrack_init

Message ID 20180321070029.28671-1-leon@kernel.org (mailing list archive)
State Accepted
Delegated to: Jason Gunthorpe
Headers show

Commit Message

Leon Romanovsky March 21, 2018, 7 a.m. UTC
From: Leon Romanovsky <leonro@mellanox.com>

The fact that resource tracking 02d8883f520e ("RDMA/restrack: Add general
infrastructure to track RDMA resources") was added immediately after
commit 16c1975f1032 ("IB/mlx5: Create profile infrastructure to add and
remove stages") caused to miss the fact that PD and CQ are created after
ib_register_device, but released after ib_unregister_device() and not
before as it is expected from normal flow.

Fix introduced in commit 42cea83f9524 ("IB/mlx5: Fix cleanup order on unload")
revealed this fact, so this patch is needed to avoid from restrack warnings

It fixes resource tracking warnings during shutdown.

[   43.473906] CPU: 5 PID: 3016 Comm: modprobe Not tainted 4.16.0-rc5-for-linust-perf-2018-03-19_07-01-58-14 #1
[   43.473907] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu2 04/01/2014
[   43.473919] RIP: 0010:rdma_restrack_clean+0x25/0x30 [ib_core]
[   43.473921] RSP: 0018:ffffc9000267be48 EFLAGS: 00010282
[   43.473924] RAX: 0000000000000000 RBX: ffff88033c690070 RCX: 0000000180080006
[   43.473925] RDX: ffff88035ce922e0 RSI: ffffea000cf1a200 RDI: ffff88033c6907c8
[   43.473926] RBP: ffff88033c690070 R08: ffff88033c689000 R09: 0000000180080006
[   43.473927] R10: 000000003c68a001 R11: ffff88033c689000 R12: ffff88033c690000
[   43.473929] R13: ffff88033c69005c R14: 0000000000000000 R15: 0000000000000000
[   43.473932] FS:  00007f5928359740(0000) GS:ffff88036c540000(0000) knlGS:0000000000000000
[   43.473933] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   43.473935] CR2: 00007ffffc760cc8 CR3: 000000035620c000 CR4: 00000000000006e0
[   43.473940] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   43.473941] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   43.473942] Call Trace:
[   43.473969]  ib_unregister_device+0xf5/0x190 [ib_core]
[   43.474000]  __mlx5_ib_remove+0x2e/0x40 [mlx5_ib]
[   43.474098]  mlx5_remove_device+0xf5/0x120 [mlx5_core]
[   43.474132]  mlx5_unregister_interface+0x37/0x90 [mlx5_core]
[   43.474142]  mlx5_ib_cleanup+0xc/0x16a [mlx5_ib]
[   43.474152]  SyS_delete_module+0x159/0x260
[   43.474159]  do_syscall_64+0x61/0x110
[   43.474165]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[   43.474168] RIP: 0033:0x7f59278466b7
[   43.474170] RSP: 002b:00007ffffc763e38 EFLAGS: 00000202 ORIG_RAX: 00000000000000b0
[   43.474172] RAX: ffffffffffffffda RBX: 000000000130d590 RCX: 00007f59278466b7
[   43.474173] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 000000000130d5f8
[   43.474175] RBP: 0000000000000000 R08: 00007f5927b0b060 R09: 00007f59278b6a40
[   43.474176] R10: 00007ffffc763bc0 R11: 0000000000000202 R12: 0000000000000000
[   43.474177] R13: 0000000000000001 R14: 000000000130d5f8 R15: 0000000000000000
[   43.474179] Code: 84 00 00 00 00 00 0f 1f 44 00 00 48 83 c7 28 31 c0
eb 0c 48 83 c0 08 48 3d 00 08 00 00 74 0f 48 8d 14 07 48 8b 12 48 85 d2
74 e8 <0f> 0b c3 f3 c3 66 0f 1f 44 00 00 0f 1f 44 00 00 53 48 8b 47 28
[   43.474221] ---[ end trace e89771e2250ffc23 ]---

Fixes: 42cea83f9524 ("IB/mlx5: Fix cleanup order on unload")
Reviewed-by: Mark Bloch <markb@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
Changelog v0->v1:
 * Reimplemented Parav's suggestion and added his SOB.
---
 drivers/infiniband/core/device.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jason Gunthorpe March 21, 2018, 5:48 p.m. UTC | #1
On Wed, Mar 21, 2018 at 09:00:29AM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> The fact that resource tracking 02d8883f520e ("RDMA/restrack: Add general
> infrastructure to track RDMA resources") was added immediately after
> commit 16c1975f1032 ("IB/mlx5: Create profile infrastructure to add and
> remove stages") caused to miss the fact that PD and CQ are created after
> ib_register_device, but released after ib_unregister_device() and not
> before as it is expected from normal flow.
> 
> Fix introduced in commit 42cea83f9524 ("IB/mlx5: Fix cleanup order on unload")
> revealed this fact, so this patch is needed to avoid from restrack warnings
> 
> It fixes resource tracking warnings during shutdown.
> 
> [   43.473906] CPU: 5 PID: 3016 Comm: modprobe Not tainted 4.16.0-rc5-for-linust-perf-2018-03-19_07-01-58-14 #1
> [   43.473907] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu2 04/01/2014
> [   43.473919] RIP: 0010:rdma_restrack_clean+0x25/0x30 [ib_core]
> [   43.473921] RSP: 0018:ffffc9000267be48 EFLAGS: 00010282
> [   43.473924] RAX: 0000000000000000 RBX: ffff88033c690070 RCX: 0000000180080006
> [   43.473925] RDX: ffff88035ce922e0 RSI: ffffea000cf1a200 RDI: ffff88033c6907c8
> [   43.473926] RBP: ffff88033c690070 R08: ffff88033c689000 R09: 0000000180080006
> [   43.473927] R10: 000000003c68a001 R11: ffff88033c689000 R12: ffff88033c690000
> [   43.473929] R13: ffff88033c69005c R14: 0000000000000000 R15: 0000000000000000
> [   43.473932] FS:  00007f5928359740(0000) GS:ffff88036c540000(0000) knlGS:0000000000000000
> [   43.473933] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   43.473935] CR2: 00007ffffc760cc8 CR3: 000000035620c000 CR4: 00000000000006e0
> [   43.473940] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [   43.473941] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [   43.473942] Call Trace:
> [   43.473969]  ib_unregister_device+0xf5/0x190 [ib_core]
> [   43.474000]  __mlx5_ib_remove+0x2e/0x40 [mlx5_ib]
> [   43.474098]  mlx5_remove_device+0xf5/0x120 [mlx5_core]
> [   43.474132]  mlx5_unregister_interface+0x37/0x90 [mlx5_core]
> [   43.474142]  mlx5_ib_cleanup+0xc/0x16a [mlx5_ib]
> [   43.474152]  SyS_delete_module+0x159/0x260
> [   43.474159]  do_syscall_64+0x61/0x110
> [   43.474165]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> [   43.474168] RIP: 0033:0x7f59278466b7
> [   43.474170] RSP: 002b:00007ffffc763e38 EFLAGS: 00000202 ORIG_RAX: 00000000000000b0
> [   43.474172] RAX: ffffffffffffffda RBX: 000000000130d590 RCX: 00007f59278466b7
> [   43.474173] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 000000000130d5f8
> [   43.474175] RBP: 0000000000000000 R08: 00007f5927b0b060 R09: 00007f59278b6a40
> [   43.474176] R10: 00007ffffc763bc0 R11: 0000000000000202 R12: 0000000000000000
> [   43.474177] R13: 0000000000000001 R14: 000000000130d5f8 R15: 0000000000000000
> [   43.474179] Code: 84 00 00 00 00 00 0f 1f 44 00 00 48 83 c7 28 31 c0
> eb 0c 48 83 c0 08 48 3d 00 08 00 00 74 0f 48 8d 14 07 48 8b 12 48 85 d2
> 74 e8 <0f> 0b c3 f3 c3 66 0f 1f 44 00 00 0f 1f 44 00 00 53 48 8b 47 28
> [   43.474221] ---[ end trace e89771e2250ffc23 ]---
> 
> Fixes: 42cea83f9524 ("IB/mlx5: Fix cleanup order on unload")
> Reviewed-by: Mark Bloch <markb@mellanox.com>
> Signed-off-by: Parav Pandit <parav@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> Changelog v0->v1:
>  * Reimplemented Parav's suggestion and added his SOB.
> ---
>  drivers/infiniband/core/device.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Does this replace the whole series of two patches or is

https://patchwork.kernel.org/patch/10297185/

still needed too?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky March 21, 2018, 6:08 p.m. UTC | #2
On Wed, Mar 21, 2018 at 11:48:07AM -0600, Jason Gunthorpe wrote:
> On Wed, Mar 21, 2018 at 09:00:29AM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > The fact that resource tracking 02d8883f520e ("RDMA/restrack: Add general
> > infrastructure to track RDMA resources") was added immediately after
> > commit 16c1975f1032 ("IB/mlx5: Create profile infrastructure to add and
> > remove stages") caused to miss the fact that PD and CQ are created after
> > ib_register_device, but released after ib_unregister_device() and not
> > before as it is expected from normal flow.
> >
> > Fix introduced in commit 42cea83f9524 ("IB/mlx5: Fix cleanup order on unload")
> > revealed this fact, so this patch is needed to avoid from restrack warnings
> >
> > It fixes resource tracking warnings during shutdown.
> >
> > [   43.473906] CPU: 5 PID: 3016 Comm: modprobe Not tainted 4.16.0-rc5-for-linust-perf-2018-03-19_07-01-58-14 #1
> > [   43.473907] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu2 04/01/2014
> > [   43.473919] RIP: 0010:rdma_restrack_clean+0x25/0x30 [ib_core]
> > [   43.473921] RSP: 0018:ffffc9000267be48 EFLAGS: 00010282
> > [   43.473924] RAX: 0000000000000000 RBX: ffff88033c690070 RCX: 0000000180080006
> > [   43.473925] RDX: ffff88035ce922e0 RSI: ffffea000cf1a200 RDI: ffff88033c6907c8
> > [   43.473926] RBP: ffff88033c690070 R08: ffff88033c689000 R09: 0000000180080006
> > [   43.473927] R10: 000000003c68a001 R11: ffff88033c689000 R12: ffff88033c690000
> > [   43.473929] R13: ffff88033c69005c R14: 0000000000000000 R15: 0000000000000000
> > [   43.473932] FS:  00007f5928359740(0000) GS:ffff88036c540000(0000) knlGS:0000000000000000
> > [   43.473933] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   43.473935] CR2: 00007ffffc760cc8 CR3: 000000035620c000 CR4: 00000000000006e0
> > [   43.473940] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [   43.473941] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [   43.473942] Call Trace:
> > [   43.473969]  ib_unregister_device+0xf5/0x190 [ib_core]
> > [   43.474000]  __mlx5_ib_remove+0x2e/0x40 [mlx5_ib]
> > [   43.474098]  mlx5_remove_device+0xf5/0x120 [mlx5_core]
> > [   43.474132]  mlx5_unregister_interface+0x37/0x90 [mlx5_core]
> > [   43.474142]  mlx5_ib_cleanup+0xc/0x16a [mlx5_ib]
> > [   43.474152]  SyS_delete_module+0x159/0x260
> > [   43.474159]  do_syscall_64+0x61/0x110
> > [   43.474165]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > [   43.474168] RIP: 0033:0x7f59278466b7
> > [   43.474170] RSP: 002b:00007ffffc763e38 EFLAGS: 00000202 ORIG_RAX: 00000000000000b0
> > [   43.474172] RAX: ffffffffffffffda RBX: 000000000130d590 RCX: 00007f59278466b7
> > [   43.474173] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 000000000130d5f8
> > [   43.474175] RBP: 0000000000000000 R08: 00007f5927b0b060 R09: 00007f59278b6a40
> > [   43.474176] R10: 00007ffffc763bc0 R11: 0000000000000202 R12: 0000000000000000
> > [   43.474177] R13: 0000000000000001 R14: 000000000130d5f8 R15: 0000000000000000
> > [   43.474179] Code: 84 00 00 00 00 00 0f 1f 44 00 00 48 83 c7 28 31 c0
> > eb 0c 48 83 c0 08 48 3d 00 08 00 00 74 0f 48 8d 14 07 48 8b 12 48 85 d2
> > 74 e8 <0f> 0b c3 f3 c3 66 0f 1f 44 00 00 0f 1f 44 00 00 53 48 8b 47 28
> > [   43.474221] ---[ end trace e89771e2250ffc23 ]---
> >
> > Fixes: 42cea83f9524 ("IB/mlx5: Fix cleanup order on unload")
> > Reviewed-by: Mark Bloch <markb@mellanox.com>
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> > Changelog v0->v1:
> >  * Reimplemented Parav's suggestion and added his SOB.
> > ---
> >  drivers/infiniband/core/device.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
>
> Does this replace the whole series of two patches or is
>
> https://patchwork.kernel.org/patch/10297185/
>
> still needed too?

The second patch (UMR cleanup) is still needed.

Thanks

>
> Jason
Jason Gunthorpe March 21, 2018, 8:26 p.m. UTC | #3
On Wed, Mar 21, 2018 at 09:00:29AM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> The fact that resource tracking 02d8883f520e ("RDMA/restrack: Add general
> infrastructure to track RDMA resources") was added immediately after
> commit 16c1975f1032 ("IB/mlx5: Create profile infrastructure to add and
> remove stages") caused to miss the fact that PD and CQ are created after
> ib_register_device, but released after ib_unregister_device() and not
> before as it is expected from normal flow.
> 
> Fix introduced in commit 42cea83f9524 ("IB/mlx5: Fix cleanup order on unload")
> revealed this fact, so this patch is needed to avoid from restrack warnings
> 
> It fixes resource tracking warnings during shutdown.
> 
> [   43.473906] CPU: 5 PID: 3016 Comm: modprobe Not tainted 4.16.0-rc5-for-linust-perf-2018-03-19_07-01-58-14 #1
> [   43.473907] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu2 04/01/2014
> [   43.473919] RIP: 0010:rdma_restrack_clean+0x25/0x30 [ib_core]
> [   43.473921] RSP: 0018:ffffc9000267be48 EFLAGS: 00010282
> [   43.473924] RAX: 0000000000000000 RBX: ffff88033c690070 RCX: 0000000180080006
> [   43.473925] RDX: ffff88035ce922e0 RSI: ffffea000cf1a200 RDI: ffff88033c6907c8
> [   43.473926] RBP: ffff88033c690070 R08: ffff88033c689000 R09: 0000000180080006
> [   43.473927] R10: 000000003c68a001 R11: ffff88033c689000 R12: ffff88033c690000
> [   43.473929] R13: ffff88033c69005c R14: 0000000000000000 R15: 0000000000000000
> [   43.473932] FS:  00007f5928359740(0000) GS:ffff88036c540000(0000) knlGS:0000000000000000
> [   43.473933] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   43.473935] CR2: 00007ffffc760cc8 CR3: 000000035620c000 CR4: 00000000000006e0
> [   43.473940] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [   43.473941] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [   43.473942] Call Trace:
> [   43.473969]  ib_unregister_device+0xf5/0x190 [ib_core]
> [   43.474000]  __mlx5_ib_remove+0x2e/0x40 [mlx5_ib]
> [   43.474098]  mlx5_remove_device+0xf5/0x120 [mlx5_core]
> [   43.474132]  mlx5_unregister_interface+0x37/0x90 [mlx5_core]
> [   43.474142]  mlx5_ib_cleanup+0xc/0x16a [mlx5_ib]
> [   43.474152]  SyS_delete_module+0x159/0x260
> [   43.474159]  do_syscall_64+0x61/0x110
> [   43.474165]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> [   43.474168] RIP: 0033:0x7f59278466b7
> [   43.474170] RSP: 002b:00007ffffc763e38 EFLAGS: 00000202 ORIG_RAX: 00000000000000b0
> [   43.474172] RAX: ffffffffffffffda RBX: 000000000130d590 RCX: 00007f59278466b7
> [   43.474173] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 000000000130d5f8
> [   43.474175] RBP: 0000000000000000 R08: 00007f5927b0b060 R09: 00007f59278b6a40
> [   43.474176] R10: 00007ffffc763bc0 R11: 0000000000000202 R12: 0000000000000000
> [   43.474177] R13: 0000000000000001 R14: 000000000130d5f8 R15: 0000000000000000
> [   43.474179] Code: 84 00 00 00 00 00 0f 1f 44 00 00 48 83 c7 28 31 c0
> eb 0c 48 83 c0 08 48 3d 00 08 00 00 74 0f 48 8d 14 07 48 8b 12 48 85 d2
> 74 e8 <0f> 0b c3 f3 c3 66 0f 1f 44 00 00 0f 1f 44 00 00 53 48 8b 47 28
> [   43.474221] ---[ end trace e89771e2250ffc23 ]---
> 
> Fixes: 42cea83f9524 ("IB/mlx5: Fix cleanup order on unload")
> Reviewed-by: Mark Bloch <markb@mellanox.com>
> Signed-off-by: Parav Pandit <parav@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> Changelog v0->v1:
>  * Reimplemented Parav's suggestion and added his SOB.
> ---
>  drivers/infiniband/core/device.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Applied to for-next

Thanks,
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index bb065c9449be..b7459cf524e4 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -290,6 +290,7 @@  void ib_dealloc_device(struct ib_device *device)
 {
 	WARN_ON(device->reg_state != IB_DEV_UNREGISTERED &&
 		device->reg_state != IB_DEV_UNINITIALIZED);
+	rdma_restrack_clean(&device->res);
 	put_device(&device->dev);
 }
 EXPORT_SYMBOL(ib_dealloc_device);
@@ -600,8 +601,6 @@  void ib_unregister_device(struct ib_device *device)
 	}
 	up_read(&lists_rwsem);

-	rdma_restrack_clean(&device->res);
-
 	ib_device_unregister_rdmacg(device);
 	ib_device_unregister_sysfs(device);