| Message ID | 20240206175449.1778317-1-kheib@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-rc] RDMA/qedr: Fix qedr_create_user_qp error flow | expand |
> -----Original Message----- > From: Kamal Heib <kheib@redhat.com> > Sent: Tuesday, February 6, 2024 11:25 PM > To: linux-rdma@vger.kernel.org > Cc: Jason Gunthorpe <jgg@ziepe.ca>; Leon Romanovsky <leon@kernel.org>; Michal Kalderon <mkalderon@marvell.com>; Ariel Elior > <aelior@marvell.com>; Kamal Heib <kheib@redhat.com> > Subject: [EXT] [PATCH for-rc] RDMA/qedr: Fix qedr_create_user_qp error flow > > External Email > > ---------------------------------------------------------------------- > Avoid the following warning by making sure to call qedr_cleanup_user() > in case qedr_init_user_queue() failed. > > -----------[ cut here ]----------- > WARNING: CPU: 0 PID: 143192 at drivers/infiniband/core/rdma_core.c:874 uverbs_destroy_ufile_hw+0xcf/0xf0 [ib_uverbs] > Modules linked in: tls target_core_user uio target_core_pscsi target_core_file target_core_iblock ib_srpt ib_srp scsi_transport_srp nfsd > nfs_acl rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache netfs 8021q garp mrp stp llc ext4 mbcache jbd2 opa_vnic > ib_umad ib_ipoib sunrpc rdma_ucm ib_isert iscsi_target_mod target_core_mod ib_iser libiscsi scsi_transport_iscsi rdma_cm iw_cm ib_cm > hfi1 intel_rapl_msr intel_rapl_common mgag200 qedr sb_edac drm_shmem_helper rdmavt x86_pkg_temp_thermal drm_kms_helper > intel_powerclamp ib_uverbs coretemp i2c_algo_bit kvm_intel dell_wmi_descriptor ipmi_ssif sparse_keymap kvm ib_core rfkill syscopyarea > sysfillrect video sysimgblt irqbypass ipmi_si ipmi_devintf fb_sys_fops rapl iTCO_wdt mxm_wmi iTCO_vendor_support intel_cstate pcspkr > dcdbas intel_uncore ipmi_msghandler lpc_ich acpi_power_meter mei_me mei fuse drm xfs libcrc32c qede sd_mod ahci libahci t10_pi sg > crct10dif_pclmul crc32_pclmul crc32c_intel qed libata tg3 > ghash_clmulni_intel megaraid_sas crc8 wmi [last unloaded: ib_srpt] > CPU: 0 PID: 143192 Comm: fi_rdm_tagged_p Kdump: loaded Not tainted 5.14.0-408.el9.x86_64 #1 > Hardware name: Dell Inc. PowerEdge R430/03XKDV, BIOS 2.14.0 01/25/2022 > RIP: 0010:uverbs_destroy_ufile_hw+0xcf/0xf0 [ib_uverbs] > Code: 5d 41 5c 41 5d 41 5e e9 0f 26 1b dd 48 89 df e8 67 6a ff ff 49 8b 86 10 01 00 00 48 85 c0 74 9c 4c 89 e7 e8 83 c0 cb dd eb 92 > <0f> 0b eb be 0f 0b be 04 00 00 00 48 89 df e8 8e f5 ff ff e9 6d ff > RSP: 0018:ffffb7c6cadfbc60 EFLAGS: 00010286 > RAX: ffff8f0889ee3f60 RBX: ffff8f088c1a5200 RCX: 00000000802a0016 > RDX: 00000000802a0017 RSI: 0000000000000001 RDI: ffff8f0880042600 > RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000 > R10: ffff8f11fffd5000 R11: 0000000000039000 R12: ffff8f0d5b36cd80 > R13: ffff8f088c1a5250 R14: ffff8f1206d91000 R15: 0000000000000000 > FS: 0000000000000000(0000) GS:ffff8f11d7c00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000147069200e20 CR3: 00000001c7210002 CR4: 00000000001706f0 > Call Trace: > <TASK> > ? show_trace_log_lvl+0x1c4/0x2df > ? show_trace_log_lvl+0x1c4/0x2df > ? ib_uverbs_close+0x1f/0xb0 [ib_uverbs] > ? uverbs_destroy_ufile_hw+0xcf/0xf0 [ib_uverbs] > ? __warn+0x81/0x110 > ? uverbs_destroy_ufile_hw+0xcf/0xf0 [ib_uverbs] > ? report_bug+0x10a/0x140 > ? handle_bug+0x3c/0x70 > ? exc_invalid_op+0x14/0x70 > ? asm_exc_invalid_op+0x16/0x20 > ? uverbs_destroy_ufile_hw+0xcf/0xf0 [ib_uverbs] > ib_uverbs_close+0x1f/0xb0 [ib_uverbs] > __fput+0x94/0x250 > task_work_run+0x5c/0x90 > do_exit+0x270/0x4a0 > do_group_exit+0x2d/0x90 > get_signal+0x87c/0x8c0 > arch_do_signal_or_restart+0x25/0x100 > ? ib_uverbs_ioctl+0xc2/0x110 [ib_uverbs] > exit_to_user_mode_loop+0x9c/0x130 > exit_to_user_mode_prepare+0xb6/0x100 > syscall_exit_to_user_mode+0x12/0x40 > do_syscall_64+0x69/0x90 > ? syscall_exit_work+0x103/0x130 > ? syscall_exit_to_user_mode+0x22/0x40 > ? do_syscall_64+0x69/0x90 > ? syscall_exit_work+0x103/0x130 > ? syscall_exit_to_user_mode+0x22/0x40 > ? do_syscall_64+0x69/0x90 > ? do_syscall_64+0x69/0x90 > ? common_interrupt+0x43/0xa0 > entry_SYSCALL_64_after_hwframe+0x72/0xdc > RIP: 0033:0x1470abe3ec6b > Code: Unable to access opcode bytes at RIP 0x1470abe3ec41. > RSP: 002b:00007fff13ce9108 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 > RAX: fffffffffffffffc RBX: 00007fff13ce9218 RCX: 00001470abe3ec6b > RDX: 00007fff13ce9200 RSI: 00000000c0181b01 RDI: 0000000000000004 > RBP: 00007fff13ce91e0 R08: 0000558d9655da10 R09: 0000558d9655dd00 > R10: 00007fff13ce95c0 R11: 0000000000000246 R12: 00007fff13ce9358 > R13: 0000000000000013 R14: 0000558d9655db50 R15: 00007fff13ce9470 > </TASK> > --[ end trace 888a9b92e04c5c97 ]-- > > Fixes: df15856132bc ("RDMA/qedr: restructure functions that create/destroy QPs") > Signed-off-by: Kamal Heib <kheib@redhat.com> > --- > drivers/infiniband/hw/qedr/verbs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c > index 7887a6786ed4..0943abd4de27 100644 > --- a/drivers/infiniband/hw/qedr/verbs.c > +++ b/drivers/infiniband/hw/qedr/verbs.c > @@ -1880,7 +1880,7 @@ static int qedr_create_user_qp(struct qedr_dev *dev, > rc = qedr_init_user_queue(udata, dev, &qp->urq, ureq.rq_addr, > ureq.rq_len, true, 0, alloc_and_init); > if (rc) > - return rc; > + goto err1; > } > > memset(&in_params, 0, sizeof(in_params)); > -- > 2.43.0 > Thanks for fixing it. Acked-by: Alok Prasad <palok@marvell.com>
On Tue, Feb 06, 2024 at 12:54:49PM -0500, Kamal Heib wrote: > Avoid the following warning by making sure to call qedr_cleanup_user() > in case qedr_init_user_queue() failed. <...> > diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c > index 7887a6786ed4..0943abd4de27 100644 > --- a/drivers/infiniband/hw/qedr/verbs.c > +++ b/drivers/infiniband/hw/qedr/verbs.c > @@ -1880,7 +1880,7 @@ static int qedr_create_user_qp(struct qedr_dev *dev, > rc = qedr_init_user_queue(udata, dev, &qp->urq, ureq.rq_addr, > ureq.rq_len, true, 0, alloc_and_init); > if (rc) > - return rc; > + goto err1; "goto err1" will cause to call to qedr_cleanup_user() which will call to qedr_free_pbl() with qp->urq.pbl_tbl) which can be NULL. Thanks > } > > memset(&in_params, 0, sizeof(in_params)); > -- > 2.43.0 > >
On Wed, Feb 07, 2024 at 09:31:14AM +0200, Leon Romanovsky wrote: > On Tue, Feb 06, 2024 at 12:54:49PM -0500, Kamal Heib wrote: > > Avoid the following warning by making sure to call qedr_cleanup_user() > > in case qedr_init_user_queue() failed. > > <...> > > > diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c > > index 7887a6786ed4..0943abd4de27 100644 > > --- a/drivers/infiniband/hw/qedr/verbs.c > > +++ b/drivers/infiniband/hw/qedr/verbs.c > > @@ -1880,7 +1880,7 @@ static int qedr_create_user_qp(struct qedr_dev *dev, > > rc = qedr_init_user_queue(udata, dev, &qp->urq, ureq.rq_addr, > > ureq.rq_len, true, 0, alloc_and_init); > > if (rc) > > - return rc; > > + goto err1; > > "goto err1" will cause to call to qedr_cleanup_user() which will call to qedr_free_pbl() > with qp->urq.pbl_tbl) which can be NULL. > > Thanks > I see, what about something like the following: diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c index 7887a6786ed4..f118ce0a9a61 100644 --- a/drivers/infiniband/hw/qedr/verbs.c +++ b/drivers/infiniband/hw/qedr/verbs.c @@ -1879,8 +1879,17 @@ static int qedr_create_user_qp(struct qedr_dev *dev, /* RQ - read access only (0) */ rc = qedr_init_user_queue(udata, dev, &qp->urq, ureq.rq_addr, ureq.rq_len, true, 0, alloc_and_init); - if (rc) + if (rc) { + ib_umem_release(qp->usq.umem); + qp->usq.umem = NULL; + if (rdma_protocol_roce(&dev->ibdev, 1)) { + qedr_free_pbl(dev, &qp->usq.pbl_info, + qp->usq.pbl_tbl); + } else { + kfree(qp->usq.pbl_tbl); + } return rc; + } } memset(&in_params, 0, sizeof(in_params)); Thanks! > > } > > > > memset(&in_params, 0, sizeof(in_params)); > > -- > > 2.43.0 > > > > >
On Wed, Feb 07, 2024 at 07:01:53PM -0500, Kamal Heib wrote: > On Wed, Feb 07, 2024 at 09:31:14AM +0200, Leon Romanovsky wrote: > > On Tue, Feb 06, 2024 at 12:54:49PM -0500, Kamal Heib wrote: > > > Avoid the following warning by making sure to call qedr_cleanup_user() > > > in case qedr_init_user_queue() failed. > > > > <...> > > > > > diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c > > > index 7887a6786ed4..0943abd4de27 100644 > > > --- a/drivers/infiniband/hw/qedr/verbs.c > > > +++ b/drivers/infiniband/hw/qedr/verbs.c > > > @@ -1880,7 +1880,7 @@ static int qedr_create_user_qp(struct qedr_dev *dev, > > > rc = qedr_init_user_queue(udata, dev, &qp->urq, ureq.rq_addr, > > > ureq.rq_len, true, 0, alloc_and_init); > > > if (rc) > > > - return rc; > > > + goto err1; > > > > "goto err1" will cause to call to qedr_cleanup_user() which will call to qedr_free_pbl() > > with qp->urq.pbl_tbl) which can be NULL. > > > > Thanks > > > > I see, what about something like the following: It will work. > > diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c > index 7887a6786ed4..f118ce0a9a61 100644 > --- a/drivers/infiniband/hw/qedr/verbs.c > +++ b/drivers/infiniband/hw/qedr/verbs.c > @@ -1879,8 +1879,17 @@ static int qedr_create_user_qp(struct qedr_dev *dev, > /* RQ - read access only (0) */ > rc = qedr_init_user_queue(udata, dev, &qp->urq, ureq.rq_addr, > ureq.rq_len, true, 0, alloc_and_init); > - if (rc) > + if (rc) { > + ib_umem_release(qp->usq.umem); > + qp->usq.umem = NULL; > + if (rdma_protocol_roce(&dev->ibdev, 1)) { > + qedr_free_pbl(dev, &qp->usq.pbl_info, > + qp->usq.pbl_tbl); > + } else { > + kfree(qp->usq.pbl_tbl); > + } > return rc; > + } > } > > memset(&in_params, 0, sizeof(in_params)); > > > Thanks! > > > > > } > > > > > > memset(&in_params, 0, sizeof(in_params)); > > > -- > > > 2.43.0 > > > > > > > > >
diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c index 7887a6786ed4..0943abd4de27 100644 --- a/drivers/infiniband/hw/qedr/verbs.c +++ b/drivers/infiniband/hw/qedr/verbs.c @@ -1880,7 +1880,7 @@ static int qedr_create_user_qp(struct qedr_dev *dev, rc = qedr_init_user_queue(udata, dev, &qp->urq, ureq.rq_addr, ureq.rq_len, true, 0, alloc_and_init); if (rc) - return rc; + goto err1; } memset(&in_params, 0, sizeof(in_params));
Avoid the following warning by making sure to call qedr_cleanup_user() in case qedr_init_user_queue() failed. -----------[ cut here ]----------- WARNING: CPU: 0 PID: 143192 at drivers/infiniband/core/rdma_core.c:874 uverbs_destroy_ufile_hw+0xcf/0xf0 [ib_uverbs] Modules linked in: tls target_core_user uio target_core_pscsi target_core_file target_core_iblock ib_srpt ib_srp scsi_transport_srp nfsd nfs_acl rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache netfs 8021q garp mrp stp llc ext4 mbcache jbd2 opa_vnic ib_umad ib_ipoib sunrpc rdma_ucm ib_isert iscsi_target_mod target_core_mod ib_iser libiscsi scsi_transport_iscsi rdma_cm iw_cm ib_cm hfi1 intel_rapl_msr intel_rapl_common mgag200 qedr sb_edac drm_shmem_helper rdmavt x86_pkg_temp_thermal drm_kms_helper intel_powerclamp ib_uverbs coretemp i2c_algo_bit kvm_intel dell_wmi_descriptor ipmi_ssif sparse_keymap kvm ib_core rfkill syscopyarea sysfillrect video sysimgblt irqbypass ipmi_si ipmi_devintf fb_sys_fops rapl iTCO_wdt mxm_wmi iTCO_vendor_support intel_cstate pcspkr dcdbas intel_uncore ipmi_msghandler lpc_ich acpi_power_meter mei_me mei fuse drm xfs libcrc32c qede sd_mod ahci libahci t10_pi sg crct10dif_pclmul crc32_pclmul crc32c_intel qed libata tg3 ghash_clmulni_intel megaraid_sas crc8 wmi [last unloaded: ib_srpt] CPU: 0 PID: 143192 Comm: fi_rdm_tagged_p Kdump: loaded Not tainted 5.14.0-408.el9.x86_64 #1 Hardware name: Dell Inc. PowerEdge R430/03XKDV, BIOS 2.14.0 01/25/2022 RIP: 0010:uverbs_destroy_ufile_hw+0xcf/0xf0 [ib_uverbs] Code: 5d 41 5c 41 5d 41 5e e9 0f 26 1b dd 48 89 df e8 67 6a ff ff 49 8b 86 10 01 00 00 48 85 c0 74 9c 4c 89 e7 e8 83 c0 cb dd eb 92 <0f> 0b eb be 0f 0b be 04 00 00 00 48 89 df e8 8e f5 ff ff e9 6d ff RSP: 0018:ffffb7c6cadfbc60 EFLAGS: 00010286 RAX: ffff8f0889ee3f60 RBX: ffff8f088c1a5200 RCX: 00000000802a0016 RDX: 00000000802a0017 RSI: 0000000000000001 RDI: ffff8f0880042600 RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000 R10: ffff8f11fffd5000 R11: 0000000000039000 R12: ffff8f0d5b36cd80 R13: ffff8f088c1a5250 R14: ffff8f1206d91000 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff8f11d7c00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000147069200e20 CR3: 00000001c7210002 CR4: 00000000001706f0 Call Trace: <TASK> ? show_trace_log_lvl+0x1c4/0x2df ? show_trace_log_lvl+0x1c4/0x2df ? ib_uverbs_close+0x1f/0xb0 [ib_uverbs] ? uverbs_destroy_ufile_hw+0xcf/0xf0 [ib_uverbs] ? __warn+0x81/0x110 ? uverbs_destroy_ufile_hw+0xcf/0xf0 [ib_uverbs] ? report_bug+0x10a/0x140 ? handle_bug+0x3c/0x70 ? exc_invalid_op+0x14/0x70 ? asm_exc_invalid_op+0x16/0x20 ? uverbs_destroy_ufile_hw+0xcf/0xf0 [ib_uverbs] ib_uverbs_close+0x1f/0xb0 [ib_uverbs] __fput+0x94/0x250 task_work_run+0x5c/0x90 do_exit+0x270/0x4a0 do_group_exit+0x2d/0x90 get_signal+0x87c/0x8c0 arch_do_signal_or_restart+0x25/0x100 ? ib_uverbs_ioctl+0xc2/0x110 [ib_uverbs] exit_to_user_mode_loop+0x9c/0x130 exit_to_user_mode_prepare+0xb6/0x100 syscall_exit_to_user_mode+0x12/0x40 do_syscall_64+0x69/0x90 ? syscall_exit_work+0x103/0x130 ? syscall_exit_to_user_mode+0x22/0x40 ? do_syscall_64+0x69/0x90 ? syscall_exit_work+0x103/0x130 ? syscall_exit_to_user_mode+0x22/0x40 ? do_syscall_64+0x69/0x90 ? do_syscall_64+0x69/0x90 ? common_interrupt+0x43/0xa0 entry_SYSCALL_64_after_hwframe+0x72/0xdc RIP: 0033:0x1470abe3ec6b Code: Unable to access opcode bytes at RIP 0x1470abe3ec41. RSP: 002b:00007fff13ce9108 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: fffffffffffffffc RBX: 00007fff13ce9218 RCX: 00001470abe3ec6b RDX: 00007fff13ce9200 RSI: 00000000c0181b01 RDI: 0000000000000004 RBP: 00007fff13ce91e0 R08: 0000558d9655da10 R09: 0000558d9655dd00 R10: 00007fff13ce95c0 R11: 0000000000000246 R12: 00007fff13ce9358 R13: 0000000000000013 R14: 0000558d9655db50 R15: 00007fff13ce9470 </TASK> --[ end trace 888a9b92e04c5c97 ]-- Fixes: df15856132bc ("RDMA/qedr: restructure functions that create/destroy QPs") Signed-off-by: Kamal Heib <kheib@redhat.com> --- drivers/infiniband/hw/qedr/verbs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)