* RE: nvmet_rdma crash - DISCONNECT event with NULL queue
From: Steve Wise @ 2016-11-02 19:18 UTC (permalink / raw)
To: 'Sagi Grimberg', 'Christoph Hellwig'
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <004701d2351a$d9e4ad70$8dae0850$@opengridcomputing.com>
> I'll also try and reproduce this on mlx4 to rule out
> iwarp and cxgb4 anomolies.
Running the same test over mlx4/roce, I hit a warning in list_debug, and then a
stuck CPU...
I see this a few times:
[ 916.207157] ------------[ cut here ]------------
[ 916.212455] WARNING: CPU: 1 PID: 5553 at lib/list_debug.c:33
__list_add+0xbe/0xd0
[ 916.220670] list_add corruption. prev->next should be next
(ffffffffa0847070), but was (null). (prev=ffff880833baaf20).
[ 916.233852] Modules linked in: iw_cxgb4 cxgb4 nvmet_rdma nvmet null_blk brd
ip6table_filter ip6_tables ebtable_nat ebtables ipt_MASQUERADE
nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4
nf_dfrag_ipv4 xt_state nf_conntrack ipt_REJECT nf_reject_ipv4 xt_CHECKSUM
iptable_mangle iptable_filter ip_tables bridge 8021q mrp garp stp llc
ipmi_devintf cachefiles fscache rdma_ucm rdma_cm iw_cm ib_ipoib ib_cm ib_uverb
ib_umad ocrdma be2net iw_nes libcrc32c iw_cxgb3 cxgb3 mdio ib_qib rdmavt mlx5_ib
mlx5_core mlx4_ib mlx4_en mlx4_core ib_mthca ib_core binfmt_misc dm_mirror
dm_region_hash dm_log vhost_net macvtap macvlan vhost tun kvmirqbypass uinput
iTCO_wdt iTCO_vendor_support mxm_wmi pcspkr dm_mod i2c_i801 i2c_smbus sg lpc_ich
mfd_core mei_me mei nvme nvme_core igb dca ptp pps_core ipmi_si ipmi_msghandler
wmi ext4(E) mbcache(E) jbd2(E) sd_mod(E)ahci(E) libahci(E) libata(E) mgag200(E)
ttm(E) drm_kms_helper(E) drm(E) fb_sys_fops(E) sysimgblt(E) sysfillrect(E)
syscopyarea(E) i2c_algo_bit(E) i2c_core(E) [last unloaded: cxgb4]
[ 916.337427] CPU: 1 PID: 5553 Comm: kworker/1:15 Tainted: G E
4.8.0+ #131
[ 916.346192] Hardware name: Supermicro X9DR3-F/X9DR3-F, BIOS 3.2a 07/09/2015
[ 916.354126] Workqueue: ib_cm cm_work_handler [ib_cm]
[ 916.360096] 0000000000000000 ffff880817483968 ffffffff8135a817
ffffffff8137813e
[ 916.368594] ffff8808174839c8 ffff8808174839c8 0000000000000000
ffff8808174839b8
[ 916.377112] ffffffff81086dad 000000f002080020 0000002134f11400
ffff880834f11470
[ 916.385642] Call Trace:
[ 916.389181] [<ffffffff8135a817>] dump_stack+0x67/0x90
[ 916.395430] [<ffffffff8137813e>] ? __list_add+0xbe/0xd0
[ 916.401863] [<ffffffff81086dad>] __warn+0xfd/0x120
[ 916.407862] [<ffffffff81086e89>] warn_slowpath_fmt+0x49/0x50
[ 916.414741] [<ffffffff8137813e>] __list_add+0xbe/0xd0
[ 916.421034] [<ffffffff816e0be6>] ? mutex_lock+0x16/0x40
[ 916.427522] [<ffffffffa0844d40>] nvmet_rdma_queue_connect+0x110/0x1a0
[nvmet_rdma]
[ 916.436374] [<ffffffffa0845430>] nvmet_rdma_cm_handler+0x100/0x1b0
[nvmet_rdma]
[ 916.444998] [<ffffffffa072e1d0>] cma_req_handler+0x200/0x300 [rdma_cm]
[ 916.452847] [<ffffffffa06f3937>] cm_process_work+0x27/0x100 [ib_cm]
[ 916.460452] [<ffffffffa06f61ea>] cm_req_handler+0x35a/0x540 [ib_cm]
[ 916.468070] [<ffffffffa06f641b>] cm_work_handler+0x4b/0xd0 [ib_cm]
[ 916.475614] [<ffffffff810a1483>] process_one_work+0x183/0x4d0
[ 916.482751] [<ffffffff816deda0>] ? __schedule+0x1f0/0x5b0
[ 916.489539] [<ffffffff816df260>] ? schedule+0x40/0xb0
[ 916.495985] [<ffffffff810a211d>] worker_thread+0x16d/0x530
[ 916.502892] [<ffffffff816deda0>] ? __schedule+0x1f0/0x5b0
[ 916.509730] [<ffffffff810cb9b6>] ? __wake_up_common+0x56/0x90
[ 916.516926] [<ffffffff810a1fb0>] ? maybe_create_worker+0x120/0x120
[ 916.524568] [<ffffffff816df260>] ? schedule+0x40/0xb0
[ 916.531084] [<ffffffff810a1fb0>] ? maybe_create_worker+0x120/0x120
[ 916.538758] [<ffffffff810a6c5c>] kthread+0xcc/0xf0
[ 916.545053] [<ffffffff810b1aae>] ? schedule_tail+0x1e/0xc0
[ 916.552082] [<ffffffff816e2eff>] ret_from_fork+0x1f/0x40
[ 916.558935] [<ffffffff810a6b90>] ? kthread_freezable_should_stop+0x70/0x70
[ 916.567430] ---[ end trace a294c05aa08938f6 ]---
...
And then a cpu gets stuck:
[ 988.672768] NMI watchdog: BUG: soft lockup - CPU#1 stuck for 22s!
[kworker/1:12:5549]
[ 988.681814] Modules linked in: iw_cxgb4 cxgb4 nvmet_rdma nvmet null_blk brd
ip6table_filter ip6_tables ebtable_nat ebtables ipt_MASQUERADE
nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4
nf_dfrag_ipv4 xt_state nf_conntrack ipt_REJECT nf_reject_ipv4 xt_CHECKSUM
iptable_mangle iptable_filter ip_tables bridge 8021q mrp garp stp llc
ipmi_devintf cachefiles fscache rdma_ucm rdma_cm iw_cm ib_ipoib ib_cm ib_uverb
ib_umad ocrdma be2net iw_nes libcrc32c iw_cxgb3 cxgb3 mdio ib_qib rdmavt mlx5_ib
mlx5_core mlx4_ib mlx4_en mlx4_core ib_mthca ib_core binfmt_misc dm_mirror
dm_region_hash dm_log vhost_net macvtap macvlan vhost tun kvmirqbypass uinput
iTCO_wdt iTCO_vendor_support mxm_wmi pcspkr dm_mod i2c_i801 i2c_smbus sg lpc_ich
mfd_core mei_me mei nvme nvme_core igb dca ptp pps_core ipmi_si ipmi_msghandler
wmi ext4(E) mbcache(E) jbd2(E) sd_mod(E)ahci(E) libahci(E) libata(E) mgag200(E)
ttm(E) drm_kms_helper(E) drm(E) fb_sys_fops(E) sysimgblt(E) sysfillrect(E)
syscopyarea(E) i2c_algo_bit(E) i2c_core(E) [last unloaded: cxgb4]
[ 988.786988] CPU: 1 PID: 5549 Comm: kworker/1:12 Tainted: G W EL
4.8.0+ #131
[ 988.796023] Hardware name: Supermicro X9DR3-F/X9DR3-F, BIOS 3.2a 07/09/2015
[ 988.804188] Workqueue: events nvmet_keep_alive_timer [nvmet]
[ 988.811068] task: ffff880819328000 task.stack: ffff880819324000
[ 988.818195] RIP: 0010:[<ffffffffa084361c>] [<ffffffffa084361c>]
nvmet_rdma_delete_ctrl+0x3c/0xb0 [nvmet_rdma]
[ 988.829434] RSP: 0018:ffff880819327c58 EFLAGS: 00000287
[ 988.835946] RAX: ffff880834f11b20 RBX: ffff880834f11b20 RCX: 0000000000000000
[ 988.844285] RDX: 0000000000000001 RSI: ffff88085fa58ae0 RDI: ffffffffa0847040
[ 988.852626] RBP: ffff880819327c88 R08: ffff88085fa58ae0 R09: ffff880819327918
[ 988.860968] R10: 0000000000000920 R11: 0000000000000001 R12: ffff880834f11a00
[ 988.869310] R13: ffff88081a6a4800 R14: 0000000000000000 R15: ffff88085fa5d505
[ 988.877655] FS: 0000000000000000(0000) GS:ffff88085fa40000(0000)
knlGS:0000000000000000
[ 988.886955] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 988.893906] CR2: 00007f28fcc6e74b CR3: 0000000001c06000 CR4: 00000000000406e0
[ 988.902246] Stack:
[ 988.905457] ffff880817fc6720 0000000000000002 000000000000000f
ffff88081a6a4800
[ 988.914142] ffff88085fa58ac0 ffff88085fa5d500 ffff880819327ca8
ffffffffa0830237
[ 988.922825] ffff88085fa58ac0 ffff8808584ce900 ffff880819327d88
ffffffff810a1483
[ 988.931507] Call Trace:
[ 988.935152] [<ffffffffa0830237>] nvmet_keep_alive_timer+0x37/0x40 [nvmet]
[ 988.943232] [<ffffffff810a1483>] process_one_work+0x183/0x4d0
[ 988.950273] [<ffffffff816deda0>] ? __schedule+0x1f0/0x5b0
[ 988.956963] [<ffffffff816df260>] ? schedule+0x40/0xb0
[ 988.963299] [<ffffffff8102eb34>] ? __switch_to+0x1e4/0x790
[ 988.970070] [<ffffffff810a211d>] worker_thread+0x16d/0x530
[ 988.976848] [<ffffffff816deda0>] ? __schedule+0x1f0/0x5b0
[ 988.983541] [<ffffffff810cb9b6>] ? __wake_up_common+0x56/0x90
[ 988.990578] [<ffffffff810a1fb0>] ? maybe_create_worker+0x120/0x120
[ 988.998055] [<ffffffff816df260>] ? schedule+0x40/0xb0
[ 989.004394] [<ffffffff810a1fb0>] ? maybe_create_worker+0x120/0x120
[ 989.011861] [<ffffffff810a6c5c>] kthread+0xcc/0xf0
[ 989.017944] [<ffffffff810b1aae>] ? schedule_tail+0x1e/0xc0
[ 989.024728] [<ffffffff816e2eff>] ret_from_fork+0x1f/0x40
[ 989.031325] [<ffffffff810a6b90>] ? kthread_freezable_should_stop+0x70/0x70
[ 989.039488] Code: 90 49 89 fd 48 c7 c7 40 70 84 a0 e8 cf d5 e9 e0 48 8b 05 68
3a 00 00 48 3d 70 70 84 a0 4c 8d a0 e0 fe ff ff 48 89 c3 75 1c eb 55 <49> 8b 84
24 20 01 00 00 48 3d 70 70 84 a0 4c 8d a0 e0 fe ff ff
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 0/14] Fix race conditions related to stopping block layer queues
From: Jens Axboe @ 2016-11-02 18:52 UTC (permalink / raw)
To: Bart Van Assche
Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
Mike Snitzer, Doug Ledford, Keith Busch, Ming Lei,
Konrad Rzeszutek Wilk, Roger Pau Monné, Laurence Oberman,
linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <7460e8b2-2cfd-c0d5-7ae7-7f662d89dad3-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
On 10/28/2016 06:18 PM, Bart Van Assche wrote:
> Hello Jens,
>
> Multiple block drivers need the functionality to stop a request queue
> and to wait until all ongoing request_fn() / queue_rq() calls have
> finished without waiting until all outstanding requests have finished.
> Hence this patch series that introduces the blk_mq_quiesce_queue()
> function. The dm-mq, SRP and NVMe patches in this patch series are three
> examples of where these functions are useful. These patches have been
> tested on top of kernel v4.9-rc2. The following tests have been run to
> verify this patch series:
> - Mike's mptest suite that stress-tests dm-multipath.
> - My own srp-test suite that stress-tests SRP on top of dm-multipath.
> - fio on top of the NVMeOF host driver that was connected to the NVMeOF
> target driver on the same host.
> - Laurence verified the previous version (v3) of this patch series by
> running it through the Red Hat SRP and NVMe test suites.
>
> The changes compared to the third version of this patch series are:
> - Added a blk_mq_stop_hw_queues() call in blk_mq_quiesce_queue() as
> requested by Ming Lei.
> - Modified scsi_unblock_target() such that it waits until
> .queuecommand() finished. Unexported scsi_wait_for_queuecommand().
> - Reordered the two NVMe patches.
> - Added a patch that avoids that blk_mq_requeue_work() restarts stopped
> queues.
> - Added a patch that removes blk_mq_cancel_requeue_work().
Bart, I have applied the series, except patches 11+12. I don't mind
taking them through the block tree, but they can go through the SCSI
tree as well. Let me know what folks think.
--
Jens Axboe
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: SQ overflow seen running isert traffic
From: Steve Wise @ 2016-11-02 17:03 UTC (permalink / raw)
To: 'Nicholas A. Bellinger'
Cc: 'Sagi Grimberg', 'Potnuri Bharat Teja',
target-devel, linux-rdma
In-Reply-To: <1477885208.27946.8.camel@haakon3.risingtidesystems.com>
> > So to sum up - 2 issues:
> >
> > 1) we believe the iSER + RW API correctly sizes the SQ, yet we're seeing SQ
> > overflows. So the SQ sizing needs more investigation.
> >
> > 2) if the SQ is full, then the iSER/target code is supposed to resubmit. And
> > apparently that isn't working.
> >
>
> For #2, target-core expects -ENOMEM or -EAGAIN return from fabric driver
> callbacks to signal internal queue-full retry logic. Otherwise, the
> extra se_cmd->cmd_kref response SCF_ACK_KREF is leaked until session
> shutdown and/or reinstatement occurs.
>
> AFAICT, Potunri's earlier hung task with v4.8.y + ABORT_TASK is likely
> the earlier v4.1+ regression:
>
> https://github.com/torvalds/linux/commit/527268df31e57cf2b6d417198717c6d6afd
> b1e3e
>
> That said, there is room for improvement in target-core queue-full error
> signaling, and iscsi-target/iser-target callback error propagation.
>
> Sending out a series shortly to address these particular items.
> Please have a look.
>
Hey Nicholas, Bharat is out until next week. He'll try this all out next week and report back.
Thanks!
Steve.
^ permalink raw reply
* Re: [PATCH v4 10/10] IB/mlx5: Simplify completion into a wait_event
From: Leon Romanovsky @ 2016-11-02 15:59 UTC (permalink / raw)
To: Binoy Jayan
Cc: Sagi Grimberg, Doug Ledford, Sean Hefty, Hal Rosenstock,
Arnd Bergmann, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <8ceb7dbf-d242-1427-199a-b6ec82876f23-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2724 bytes --]
On Sun, Oct 30, 2016 at 11:17:57PM +0200, Sagi Grimberg wrote:
>
>
> On 27/10/16 09:59, Binoy Jayan wrote:
> >Convert the completion 'mlx5_ib_umr_context:done' to a wait_event as it
> >just waits for the return value to be filled.
On top of Sagi's response, I'm failing to understand why it is needed.
Can you elaborate more about it?
> >
> >Signed-off-by: Binoy Jayan <binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >---
> > drivers/infiniband/hw/mlx5/mlx5_ib.h | 2 +-
> > drivers/infiniband/hw/mlx5/mr.c | 9 +++++----
> > include/rdma/ib_verbs.h | 1 +
> > 3 files changed, 7 insertions(+), 5 deletions(-)
> >
> >diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> >index de31b5f..cf496b5 100644
> >--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
> >+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> >@@ -524,7 +524,7 @@ struct mlx5_ib_mw {
> > struct mlx5_ib_umr_context {
> > struct ib_cqe cqe;
> > enum ib_wc_status status;
> >- struct completion done;
> >+ wait_queue_head_t wq;
> > };
> >
> > struct umr_common {
> >diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
> >index dfaf6f6..49ff2af 100644
> >--- a/drivers/infiniband/hw/mlx5/mr.c
> >+++ b/drivers/infiniband/hw/mlx5/mr.c
> >@@ -846,14 +846,14 @@ static void mlx5_ib_umr_done(struct ib_cq *cq, struct ib_wc *wc)
> > container_of(wc->wr_cqe, struct mlx5_ib_umr_context, cqe);
> >
> > context->status = wc->status;
> >- complete(&context->done);
> >+ wake_up(&context->wq);
> > }
> >
> > static inline void mlx5_ib_init_umr_context(struct mlx5_ib_umr_context *context)
> > {
> > context->cqe.done = mlx5_ib_umr_done;
> >- context->status = -1;
> >- init_completion(&context->done);
> >+ context->status = IB_WC_STATUS_NONE;
> >+ init_waitqueue_head(&context->wq);
> > }
> >
> > static inline int mlx5_ib_post_send_wait(struct mlx5_ib_dev *dev,
> >@@ -873,7 +873,8 @@ static inline int mlx5_ib_post_send_wait(struct mlx5_ib_dev *dev,
> > if (err) {
> > mlx5_ib_warn(dev, "UMR post send failed, err %d\n", err);
> > } else {
> >- wait_for_completion(&umr_context.done);
> >+ wait_event(umr_context.wq,
> >+ umr_context.status != IB_WC_STATUS_NONE);
>
> How is this simpler?
>
>
> > enum ib_wc_status {
> >+ IB_WC_STATUS_NONE = -1,
> > IB_WC_SUCCESS,
> > IB_WC_LOC_LEN_ERR,
> > IB_WC_LOC_QP_OP_ERR,
> >
>
> Huh? Where did this bogus status came from? IMHO, this is polluting
> the verbs interface for no good reason at all, sorry.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] qedr: Fix possible memory leak in qedr_create_qp()
From: Leon Romanovsky @ 2016-11-02 15:55 UTC (permalink / raw)
To: Amrani, Ram
Cc: Wei Yongjun, Doug Ledford, Borundia, Rajesh, Wei Yongjun,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <SN1PR07MB2207A02F1883380BE4B1CB06F8A10-mikhvbZlbf8TSoR2DauN2+FPX92sqiQdvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]
On Tue, Nov 01, 2016 at 02:42:27PM +0000, Amrani, Ram wrote:
> > While looking on this patch and associated code to it, I noticed the
> > following code stack:
> >
> > qedr_create_qp
> > -->
> > dev->ops->rdma_destroy_qp(dev->rdma_ctx, qp->qed_qp);
> > -->
> > qed_rdma_destroy_qp
> > -->
> > qed_roce_destroy_qp
> > This function will check the QP state and return -EINVAL and comment
> > that this QP needs to be prepared before destroying it.
> >
> > However immediately after returning, you are calling to kfree(qp)
> > without any checks.
> >
> > It looks like an error and it is worth to take a look on it.
> >
>
> That's a deep level of reading... thanks.
You are welcome.
>
> When the QP is created its state is set in ecore_rdma_create_qp():
> qp->cur_state = ECORE_ROCE_QP_STATE_RESET;
>
> When it is ecore_roce_destroy_qp() is invoked, the function *must* be in either RESET or two other states:
> if ((qp->cur_state != QED_ROCE_QP_STATE_RESET) &&
> (qp->cur_state != QED_ROCE_QP_STATE_ERR) &&
> (qp->cur_state != QED_ROCE_QP_STATE_INIT)) {
> DP_NOTICE(p_hwfn,
> "QP must be in error, reset or init state before destroying it\n");
> return -EINVAL;
> }
> So actually, we won't return -EINVAL here.
>
> The bug I see is that I see in our upstream code is that for RESET the normal "destroy" operations continue. But they shouldn't.
> We need here something like this:
> if (qp->cur_state == ECORE_ROCE_QP_STATE_RESET)
> return 0;
>
> Flow will return to qed_rdma_destroy_qp() that will release the qp resource in the qed_roce scope (our real purpose).
> And then return to qedr_create_qp() where the qp resource will be released in the qedr scope.
>
> And as a side issue - an improvement that can be added is to return the error code of the QP create and not of the QP destroy.
I'm happy to hear that it helped.
>
> I'll the first later on and probably the second too.
>
>
>
> > And did I miss the fix to memory leak posted during code review?
> >
> As far as I know, I have supplied patches for all memory leaks. Can you direct me to a specific e-mail?
I failed to find the relevant thread now, so forget it, probably it is
my fault.
>
> Thanks,
> Ram
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] qedr: Fix missing unlock on error in qedr_post_send()
From: Leon Romanovsky @ 2016-11-02 15:51 UTC (permalink / raw)
To: Amrani, Ram
Cc: Wei Yongjun, Doug Ledford, Sean Hefty, Hal Rosenstock,
Borundia, Rajesh, Wei Yongjun,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <SN1PR07MB22074858C222B214F03D8ED7F8A10-mikhvbZlbf8TSoR2DauN2+FPX92sqiQdvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 781 bytes --]
On Tue, Nov 01, 2016 at 02:09:42PM +0000, Amrani, Ram wrote:
> > > index a615142..e7c7417 100644
> > > --- a/drivers/infiniband/hw/qedr/verbs.c
> > > +++ b/drivers/infiniband/hw/qedr/verbs.c
> > > @@ -2983,7 +2983,8 @@ int qedr_post_send(struct ib_qp *ibqp, struct
> > > ib_send_wr *wr,
> > >
> > > if (!wr) {
> > > DP_ERR(dev, "Got an empty post send.\n");
> > > - return -EINVAL;
> > > + rc = -EINVAL;
> > > + goto out_unlock;
> > > }
> >
> > IMHO, this if needs to be moved to be before acquiring spinlock and avoid
> > introducing new labels for this one case only.
> >
>
> Thanks Wei and Leon.
>
> Actually, perhaps we can totally remove the check itself -
> Since this is kernel space, is it safe to presume that all ULPs are trusted to be well coded?
I think so.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] IB/rdmavt: Only put mmap_info ref if it exists
From: Leon Romanovsky @ 2016-11-02 15:40 UTC (permalink / raw)
To: Jim Foraker; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Dennis Dalessandro
In-Reply-To: <1478033052-147252-1-git-send-email-foraker1-i2BcT+NCU+M@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1540 bytes --]
On Tue, Nov 01, 2016 at 01:44:12PM -0700, Jim Foraker wrote:
> rvt_create_qp() creates qp->ip only when a qp creation request comes from
> userspace (udata is not NULL). If we exceed the number of available
> queue pairs however, the error path always attempts to put a kref to this
> structure. If the requestor is inside the kernel, this leads to a crash.
It will be great to have a crash report if you have one.
>
> We fix this by checking that qp->ip is not NULL before caling kref_put().
>
Please add Fixes line which refers to the commit you are fixing.
It will ensure automatic pickup to various stable trees.
Thanks
> Signed-off-by: Jim Foraker <foraker1-i2BcT+NCU+M@public.gmane.org>
> ---
> drivers/infiniband/sw/rdmavt/qp.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c
> index 6500c3b..0004e8b 100644
> --- a/drivers/infiniband/sw/rdmavt/qp.c
> +++ b/drivers/infiniband/sw/rdmavt/qp.c
> @@ -884,7 +884,8 @@ struct ib_qp *rvt_create_qp(struct ib_pd *ibpd,
> return ret;
>
> bail_ip:
> - kref_put(&qp->ip->ref, rvt_release_mmap_info);
> + if (qp->ip)
> + kref_put(&qp->ip->ref, rvt_release_mmap_info);
>
> bail_qpn:
> free_qpn(&rdi->qp_dev->qpn_table, qp->ibqp.qp_num);
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH rdma-next 2/4] IB/core: Support rate limit for packet pacing
From: Leon Romanovsky @ 2016-11-02 15:35 UTC (permalink / raw)
To: Yuval Shaia
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bodong Wang
In-Reply-To: <20161101100607.GB3727-Hxa29pjIrETlQW142y8m19+IiqhCXseY@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]
On Tue, Nov 01, 2016 at 12:06:08PM +0200, Yuval Shaia wrote:
> Two (extremely) minor suggestions inline.
>
> Yuval
>
> On Mon, Oct 31, 2016 at 12:21:35PM +0200, Leon Romanovsky wrote:
> > From: Bodong Wang <bodong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > Add new member rate_limit to ib_qp_attr, it shows the packet pacing rate
>
> Suggesting to replace with:
> Add new member rate_limit to ib_qp_attr which holds the packet pacing rate
>
> > in Kbps, 0 means unlimited.
> >
> > IB_QP_RATE_LIMIT is added to ib_attr_mask, and it could be used by RAW
>
> Suggesting to replace with:
> IB_QP_RATE_LIMIT is added to ib_attr_mask and could be used by RAW
>
> > QPs when changing QP state from RTR to RTS, RTS to RTS.
> >
> > Signed-off-by: Bodong Wang <bodong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Reviewed-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Thanks Yuval,
Doug,
What do you expect from us? respin of this patch?
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] IB/rdmavt: Only put mmap_info ref if it exists
From: Dalessandro, Dennis @ 2016-11-02 15:17 UTC (permalink / raw)
To: foraker1-i2BcT+NCU+M@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1478033052-147252-1-git-send-email-foraker1-i2BcT+NCU+M@public.gmane.org>
On Tue, 2016-11-01 at 13:44 -0700, Jim Foraker wrote:
> rvt_create_qp() creates qp->ip only when a qp creation request comes
> from
> userspace (udata is not NULL). If we exceed the number of available
> queue pairs however, the error path always attempts to put a kref to
> this
> structure. If the requestor is inside the kernel, this leads to a
requestor -> requester
> crash.
>
> We fix this by checking that qp->ip is not NULL before caling
caling -> calling
> kref_put().
>
> Signed-off-by: Jim Foraker <foraker1@llnl.gov>
Thanks Jim!
Cc: Stable <stable@vger.kernel.org> # 4.7+
Acked-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
^ permalink raw reply
* Re: [PATCH v5 0/14] Fix race conditions related to stopping block layer queues
From: Christoph Hellwig @ 2016-11-02 15:17 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Ming Lei, Konrad Rzeszutek Wilk, Roger Pau Monné,
Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <7460e8b2-2cfd-c0d5-7ae7-7f662d89dad3@sandisk.com>
Hi Bart,
this whole series looks great to me:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: nvmet_rdma crash - DISCONNECT event with NULL queue
From: 'Christoph Hellwig' @ 2016-11-02 15:15 UTC (permalink / raw)
To: Steve Wise
Cc: 'Sagi Grimberg', 'Christoph Hellwig',
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <004601d2351a$d9db85b0$8d929110$@opengridcomputing.com>
On Wed, Nov 02, 2016 at 10:07:38AM -0500, Steve Wise wrote:
> I'm not sure I understand the "subsystem" concept. I never noticed before that
> any target device had the same controller ID. The target json config file is
> inserted below. There are 10 ramdisks exported over 2 ports of a cxgb4 40GE
> device and 1 port of an mlx4 RoCE device. For this test, the NVMF host
> connects to all 10 targets over 1 port of the cxgb4 device. Like this:
Yes, you have multiple subsystems. It's sort of the NVMe equivalent
of a target which can have multiple LUNs, aka Namespaces in NVMe.
Btw, I want to actually make the ctrlid global for the target instead of
per-subsystem to ease a few things, and debuggability is just one more
on the list.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: nvmet_rdma crash - DISCONNECT event with NULL queue
From: Steve Wise @ 2016-11-02 15:07 UTC (permalink / raw)
To: 'Sagi Grimberg', 'Christoph Hellwig'
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <3512b8bb-4d29-b90a-49e1-ebf1085c47d7-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
> > Hey Sagi. I hit another crash on the target. This was with 4.8.0 + the
patch
> > to skip disconnect events if the cm_id->qp is NULL. This time the crash is
in
> > _raw_spin_lock_irqsave() called by nvmet_rdma_recv_done(). The log is too
big
> > to include everything inline, but I'm attaching the full log as an
attachment.
> > Looks like at around 4988.169 seconds in the log, we see 5 controllers
created,
> > all named "controller 1"! And 32 queues assigned to controller 1 5 times!
And
> > shortly after that we hit the BUG.
>
> So I think you're creating multiple subsystems and provision each
> subsystem differently correct? the controller ids are unique within
> a subsystem so two different subsystems can have ctrl id 1. Perhaps
> our logging should mention the subsysnqn too?
>
I'm not sure I understand the "subsystem" concept. I never noticed before that
any target device had the same controller ID. The target json config file is
inserted below. There are 10 ramdisks exported over 2 ports of a cxgb4 40GE
device and 1 port of an mlx4 RoCE device. For this test, the NVMF host
connects to all 10 targets over 1 port of the cxgb4 device. Like this:
for i in $(seq 0 9) ; do nvme connect --transport=rdma --trsvcid=4420
--traddr=10.0.1.14 --nqn=test-ram${i}; done
> Anyway, is there traffic going on?
>
Yes, heavy fio on all 10 attached ramdisks.
> The only way we can get recv_done with corrupted data is if we posted
> something after the qp drain completed, can you check if that can happen?
>
Hmm, posting after the drain would result in a synchronous error returned by
ib_post_send() for cxgb4. There is that issue with cxgb4's drain logic in that
it really only guarantees that the CQEs are polled, not that the completion
handler was called. I have a fix in progress for this (actually decided to
support drain like IB does with a small delta from the iWARP spec). I'll also
try and reproduce this on mlx4 to rule out iwarp and cxgb4 anomolies. And I can
try my new drain fix which will be posted for review soon for inclusion into
4.10.
> Can you share your test case?
Of course! This is the same test that was killing the host side very quickly,
until Christoph fixed it with:
http://lists.infradead.org/pipermail/linux-nvme/2016-November/007043.html
Now it runs for ~60-90 minutes before the target dies.
After connecting all 10 ramdisks over 1 cxgb4 port, with 32 core NVMF
host/target nodes, you run this script (note nvme0n1 is a local nvme device, so
the NVMF devices are nvme[1-10]n1):
[root@stevo1 sw]# cat /root/bug30782/fio.sh
for i in $(seq 1 200) ; do
fio --startdelay=1-10 --ioengine=libaio --rw=randwrite --name=randwrite
--size=200m --direct=1 \
--invalidate=1 --fsync_on_close=1 --group_reporting --exitall
--runtime=60 \
--time_based --filename=/dev/nvme10n1 --filename=/dev/nvme1n1 \
--filename=/dev/nvme2n1 --filename=/dev/nvme3n1 --filename=/dev/nvme4n1
\
--filename=/dev/nvme5n1 --filename=/dev/nvme6n1 --filename=/dev/nvme7n1
\
--filename=/dev/nvme8n1 --filename=/dev/nvme9n1 --iodepth=4 --numjobs=32
\
--bs=2K |grep -i "aggrb\|iops"
sleep 3
echo "### Iteration $i Done ###"
done
And then run this script (eth2 is the port handling the NVMF traffic) to force
keep alive timeouts and reconnects:
while : ; do
ifconfig eth2 down
sleep $(( ($RANDOM & 0xf) + 8 ))
ifconfig eth2 up
sleep 30
done
Here is the target json file:
[root@stevo2 ~]# cat /etc/nvmet-10ram.json
{
"hosts": [],
"ports": [
{
"addr": {
"adrfam": "ipv4",
"traddr": "10.0.1.14",
"treq": "not specified",
"trsvcid": "4420",
"trtype": "rdma"
},
"portid": 1,
"referrals": [],
"subsystems": [
"test-ram9",
"test-ram8",
"test-ram7",
"test-ram6",
"test-ram5",
"test-ram4",
"test-ram3",
"test-ram2",
"test-ram1",
"test-ram0"
]
},
{
"addr": {
"adrfam": "ipv4",
"traddr": "10.0.2.14",
"treq": "not specified",
"trsvcid": "4420",
"trtype": "rdma"
},
"portid": 2,
"referrals": [],
"subsystems": [
"test-ram9",
"test-ram8",
"test-ram7",
"test-ram6",
"test-ram5",
"test-ram4",
"test-ram3",
"test-ram2",
"test-ram1",
"test-ram0"
]
},
{
"addr": {
"adrfam": "ipv4",
"traddr": "10.0.5.14",
"treq": "not specified",
"trsvcid": "4420",
"trtype": "rdma"
},
"portid": 5,
"referrals": [],
"subsystems": [
"test-ram9",
"test-ram8",
"test-ram7",
"test-ram6",
"test-ram5",
"test-ram4",
"test-ram3",
"test-ram2",
"test-ram1",
"test-ram0"
]
},
{
"addr": {
"adrfam": "ipv4",
"traddr": "10.0.7.14",
"treq": "not specified",
"trsvcid": "4420",
"trtype": "rdma"
},
"portid": 7,
"referrals": [],
"subsystems": [
"test-ram9",
"test-ram8",
"test-ram7",
"test-ram6",
"test-ram5",
"test-ram4",
"test-ram3",
"test-ram2",
"test-ram1",
"test-ram0"
]
}
],
"subsystems": [
{
"allowed_hosts": [],
"attr": {
"allow_any_host": "1"
},
"namespaces": [
{
"device": {
"nguid": "00000000-0000-0000-0000-000000000000",
"path": "/dev/ram9"
},
"enable": 1,
"nsid": 1
}
],
"nqn": "test-ram9"
},
{
"allowed_hosts": [],
"attr": {
"allow_any_host": "1"
},
"namespaces": [
{
"device": {
"nguid": "00000000-0000-0000-0000-000000000000",
"path": "/dev/ram8"
},
"enable": 1,
"nsid": 1
}
],
"nqn": "test-ram8"
},
{
"allowed_hosts": [],
"attr": {
"allow_any_host": "1"
},
"namespaces": [
{
"device": {
"nguid": "00000000-0000-0000-0000-000000000000",
"path": "/dev/ram7"
},
"enable": 1,
"nsid": 1
}
],
"nqn": "test-ram7"
},
{
"allowed_hosts": [],
"attr": {
"allow_any_host": "1"
},
"namespaces": [
{
"device": {
"nguid": "00000000-0000-0000-0000-000000000000",
"path": "/dev/ram6"
},
"enable": 1,
"nsid": 1
}
],
"nqn": "test-ram6"
},
{
"allowed_hosts": [],
"attr": {
"allow_any_host": "1"
},
"namespaces": [
{
"device": {
"nguid": "00000000-0000-0000-0000-000000000000",
"path": "/dev/ram5"
},
"enable": 1,
"nsid": 1
}
],
"nqn": "test-ram5"
},
{
"allowed_hosts": [],
"attr": {
"allow_any_host": "1"
},
"namespaces": [
{
"device": {
"nguid": "00000000-0000-0000-0000-000000000000",
"path": "/dev/ram4"
},
"enable": 1,
"nsid": 1
}
],
"nqn": "test-ram4"
},
{
"allowed_hosts": [],
"attr": {
"allow_any_host": "1"
},
"namespaces": [
{
"device": {
"nguid": "00000000-0000-0000-0000-000000000000",
"path": "/dev/ram3"
},
"enable": 1,
"nsid": 1
}
],
"nqn": "test-ram3"
},
{
"allowed_hosts": [],
"attr": {
"allow_any_host": "1"
},
"namespaces": [
{
"device": {
"nguid": "00000000-0000-0000-0000-000000000000",
"path": "/dev/ram2"
},
"enable": 1,
"nsid": 1
}
],
"nqn": "test-ram2"
},
{
"allowed_hosts": [],
"attr": {
"allow_any_host": "1"
},
"namespaces": [
{
"device": {
"nguid": "00000000-0000-0000-0000-000000000000",
"path": "/dev/ram1"
},
"enable": 1,
"nsid": 1
}
],
"nqn": "test-ram1"
},
{
"allowed_hosts": [],
"attr": {
"allow_any_host": "1"
},
"namespaces": [
{
"device": {
"nguid": "00000000-0000-0000-0000-000000000000",
"path": "/dev/ram0"
},
"enable": 1,
"nsid": 1
}
],
"nqn": "test-ram0"
}
]
}
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] qedr: remove pointless NULL check in qedr_post_send()
From: Wei Yongjun @ 2016-11-02 13:11 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock, Ram Amrani,
Rajesh Borundia, Leon Romanovsky
Cc: Wei Yongjun, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477672406-31487-1-git-send-email-weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Remove pointless NULL check for 'wr' in qedr_post_send().
Signed-off-by: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
v1 -> v2: remove pointless NULL check as Ram's suggestion
---
drivers/infiniband/hw/qedr/verbs.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index a615142..ed7521a 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -2981,11 +2981,6 @@ int qedr_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
return -EINVAL;
}
- if (!wr) {
- DP_ERR(dev, "Got an empty post send.\n");
- return -EINVAL;
- }
-
while (wr) {
rc = __qedr_post_send(ibqp, wr, bad_wr);
if (rc)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Parav Pandit @ 2016-11-02 4:34 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma, Li Zefan,
Johannes Weiner, Doug Ledford, Christoph Hellwig, Liran Liss,
Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <20161101140732.GC3617-2ukJVAZIZ/Y@public.gmane.org>
On Tue, Nov 1, 2016 at 7:37 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Nov 01, 2016 at 04:33:23PM +0530, Parav Pandit wrote:
>>
>> > 4. Cgroup configuration should be as close as possible to "standard" if
>> > such exists, so all infinite internet guides will work for RDMA too.
>> I didnt follow this comment. Can you please explain? Are you saying
>> rdma cgroup should have define all the objects of IB spec?
>
> It is not related to spec at all. There were comments from Tejun and you that
> other cgroups (CPU, ...) have different semantics and RDMA has something unique
> (I don't remember what was it). I want to see minimal uniqueness RDMA cgroups.
ok. Got it.
Its the weights interface which is is not suitable for stateful rdma
resources which cannot be reclaimed by other cgroup once allocated.
So proposing your idea in different way to have rdma.percentage
interface as described in previous email.
This is applicable for all the resources and allows generic
configuration for average user.
^ permalink raw reply
* Re: [PATCH v5 07/14] blk-mq: Introduce blk_mq_quiesce_queue()
From: Ming Lei @ 2016-11-02 2:08 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Bart Van Assche, Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
Konrad Rzeszutek Wilk, Roger Pau Monné, Laurence Oberman,
linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <c06b66d6-2806-b38e-ade1-89ff5c6804fc-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
On Wed, Nov 2, 2016 at 12:02 AM, Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org> wrote:
> Reviewed-by: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
Reviewed-by: Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-rc] IB/ipoib: print only once when doesn't support IB_QP_CREATE_USE_GFP_NOIO
From: Leon Romanovsky @ 2016-11-02 0:44 UTC (permalink / raw)
To: Yuval Shaia
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Erez Shitrit
In-Reply-To: <20161101143429.GA10771-Hxa29pjIrERMGUUWBy6pNA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2991 bytes --]
On Tue, Nov 01, 2016 at 04:34:31PM +0200, Yuval Shaia wrote:
> On Tue, Nov 01, 2016 at 04:14:54PM +0200, Leon Romanovsky wrote:
> > On Tue, Nov 01, 2016 at 03:48:24PM +0200, Yuval Shaia wrote:
> > > On Tue, Nov 01, 2016 at 02:34:13PM +0200, Leon Romanovsky wrote:
> > > > From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > >
> > > > Currently when the card doesn't support IB_QP_CREATE_USE_GFP_NOIO it warns
> > > > on every QP creation, It becomes worse when driver works in connected mode
> > > > we will see one print on each new connection, instead do it once.
> > > >
> > > > Fixes: 09b93088d7 ('Add a QP creation flag to use GFP_NOIO allocations')
> > > > Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > > ---
> > > > drivers/infiniband/ulp/ipoib/ipoib_cm.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> > > > index 4ad297d..917393b 100644
> > > > --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> > > > +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> > > > @@ -1053,8 +1053,8 @@ static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_
> > > >
> > > > tx_qp = ib_create_qp(priv->pd, &attr);
> > > > if (PTR_ERR(tx_qp) == -EINVAL) {
> > > > - ipoib_warn(priv, "can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
> > > > - priv->ca->name);
> > > > + pr_warn_once("can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
> > > > + priv->ca->name);
> > >
> > > But it will still re-print it for different device, right?
> >
> > Good question,
> >
> > pr_warn_once is defined as alias to printk_once [1]. That printk_once is
> > macro too [2] which will define local static read_once variable.
> >
> > [1] http://lxr.free-electrons.com/source/include/linux/printk.h#L359
> > [2] http://lxr.free-electrons.com/source/include/linux/printk.h#L322
>
> If only one HCA model is installed on the system then it should be fine,
> but wonder if more then one, would we like to see the warning again? i think
> yes.
It is correct and valid question and not for the multiple devices only.
Additional thing which was missed in such a trivial patch is the lost of
context which is printed by ipoib_warn and doesn't print in the case of
pr_warn.
Thanks for pointing and investing time in the review.
Doug,
Please drop it. We will respin it.
>
> >
> > >
> > > > attr.create_flags &= ~IB_QP_CREATE_USE_GFP_NOIO;
> > > > tx_qp = ib_create_qp(priv->pd, &attr);
> > > > }
> > > > --
> > > > 2.7.4
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > > > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: nvmet_rdma crash - DISCONNECT event with NULL queue
From: Sagi Grimberg @ 2016-11-01 22:34 UTC (permalink / raw)
To: Steve Wise, 'Christoph Hellwig'
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <024e01d23476$6668b890$333a29b0$@opengridcomputing.com>
>>> But: I'll try this patch and run for a few hours and see what happens. I
>>> believe regardless of a keep alive issue, the above patch is still needed.
>>
>> In your tests, can you enable dynamic debug on:
>> nvmet_start_keep_alive_timer
>> nvmet_stop_keep_alive_timer
>> nvmet_execute_keep_alive
>
> Hey Sagi. I hit another crash on the target. This was with 4.8.0 + the patch
> to skip disconnect events if the cm_id->qp is NULL. This time the crash is in
> _raw_spin_lock_irqsave() called by nvmet_rdma_recv_done(). The log is too big
> to include everything inline, but I'm attaching the full log as an attachment.
> Looks like at around 4988.169 seconds in the log, we see 5 controllers created,
> all named "controller 1"! And 32 queues assigned to controller 1 5 times! And
> shortly after that we hit the BUG.
So I think you're creating multiple subsystems and provision each
subsystem differently correct? the controller ids are unique within
a subsystem so two different subsystems can have ctrl id 1. Perhaps
our logging should mention the subsysnqn too?
Anyway, is there traffic going on?
The only way we can get recv_done with corrupted data is if we posted
something after the qp drain completed, can you check if that can happen?
Can you share your test case?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] IB/rdmavt: Only put mmap_info ref if it exists
From: Jim Foraker @ 2016-11-01 20:44 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Dennis Dalessandro, Jim Foraker
rvt_create_qp() creates qp->ip only when a qp creation request comes from
userspace (udata is not NULL). If we exceed the number of available
queue pairs however, the error path always attempts to put a kref to this
structure. If the requestor is inside the kernel, this leads to a crash.
We fix this by checking that qp->ip is not NULL before caling kref_put().
Signed-off-by: Jim Foraker <foraker1-i2BcT+NCU+M@public.gmane.org>
---
drivers/infiniband/sw/rdmavt/qp.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c
index 6500c3b..0004e8b 100644
--- a/drivers/infiniband/sw/rdmavt/qp.c
+++ b/drivers/infiniband/sw/rdmavt/qp.c
@@ -884,7 +884,8 @@ struct ib_qp *rvt_create_qp(struct ib_pd *ibpd,
return ret;
bail_ip:
- kref_put(&qp->ip->ref, rvt_release_mmap_info);
+ if (qp->ip)
+ kref_put(&qp->ip->ref, rvt_release_mmap_info);
bail_qpn:
free_qpn(&rdi->qp_dev->qpn_table, qp->ibqp.qp_num);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* RE: nvmet_rdma crash - DISCONNECT event with NULL queue
From: Steve Wise @ 2016-11-01 19:42 UTC (permalink / raw)
To: 'Sagi Grimberg', 'Christoph Hellwig'
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <025201d23476$66812290$338367b0$@opengridcomputing.com>
> > But: I'll try this patch and run for a few hours and see what
happens. I
> > believe regardless of a keep alive issue, the above patch is still
needed.
>
> In your tests, can you enable dynamic debug on:
> nvmet_start_keep_alive_timer
> nvmet_stop_keep_alive_timer
> nvmet_execute_keep_alive
Hey Sagi. I hit another crash on the target. This was with 4.8.0 + the
patch to skip disconnect events if the cm_id->qp is NULL. This time the
crash is in _raw_spin_lock_irqsave() called by nvmet_rdma_recv_done().
The log is too big to include everything inline, but I'm attaching the
full log as an attachment. Looks like at around 4988.169 seconds in the
log, we see 5 controllers created, all named "controller 1"! And 32
queues assigned to controller 1 5 times! And shortly after that we hit
the BUG.
Snipits:
[ 4988.035500] nvmet_start_keep_alive_timer: nvmet: ctrl 1 start
keep-alive timer for 15 secs
[ 4988.035507] nvmet: creating controller 1 for NQN
nqn.2014-08.org.nvmexpress:NVMf:uuid:0c8e193a-9c77-4e40-adff-07a7b8a352af.
[ 4988.040872] nvmet_start_keep_alive_timer: nvmet: ctrl 1 start
keep-alive timer for 15 secs
[ 4988.040877] nvmet: creating controller 1 for NQN
nqn.2014-08.org.nvmexpress:NVMf:uuid:0c8e193a-9c77-4e40-adff-07a7b8a352af.
[ 4988.044614] nvmet_start_keep_alive_timer: nvmet: ctrl 1 start
keep-alive timer for 15 secs
[ 4988.044619] nvmet: creating controller 1 for NQN
nqn.2014-08.org.nvmexpress:NVMf:uuid:0c8e193a-9c77-4e40-adff-07a7b8a352af.
[ 4988.054419] nvmet_start_keep_alive_timer: nvmet: ctrl 1 start
keep-alive timer for 15 secs
[ 4988.054423] nvmet: creating controller 1 for NQN
nqn.2014-08.org.nvmexpress:NVMf:uuid:0c8e193a-9c77-4e40-adff-07a7b8a352af.
[ 4988.054428] nvmet_start_keep_alive_timer: nvmet: ctrl 1 start
keep-alive timer for 15 secs
[ 4988.054433] nvmet: creating controller 1 for NQN
nqn.2014-08.org.nvmexpress:NVMf:uuid:0c8e193a-9c77-4e40-adff-07a7b8a352af.
Queues assigned to the same controller multiple times:
[ 4988.371465] nvmet: adding queue 1 to ctrl 1.
[ 4988.376144] nvmet: adding queue 2 to ctrl 1.
[ 4988.380790] nvmet: adding queue 3 to ctrl 1.
[ 4988.386110] nvmet: adding queue 1 to ctrl 1.
[ 4988.390751] nvmet: adding queue 2 to ctrl 1.
[ 4988.416056] nvmet: adding queue 4 to ctrl 1.
[ 4988.420658] nvmet: adding queue 3 to ctrl 1.
[ 4988.425257] nvmet: adding queue 1 to ctrl 1.
[ 4988.429841] nvmet: adding queue 1 to ctrl 1.
[ 4988.434402] nvmet: adding queue 5 to ctrl 1.
[ 4988.438957] nvmet: adding queue 4 to ctrl 1.
[ 4988.443497] nvmet: adding queue 2 to ctrl 1.
[ 4988.448041] nvmet: adding queue 2 to ctrl 1.
And the BUG in the middle of all this:
[ 4988.964835] BUG: unable to handle kernel
[ 4988.964877] nvmet: adding queue 32 to ctrl 1.
[ 4988.973233] NULL pointer dereference
[ 4988.977047] at 00000000000000b8
[ 4988.978932] nvmet: adding queue 1 to ctrl 1.
[ 4988.978977] nvmet: adding queue 2 to ctrl 1.
...
[ 4988.980539] nvmet: adding queue 32 to ctrl 1.
[ 4989.051155] nvmet_start_keep_alive_timer: nvmet: ctrl 1 start
keep-alive timer for 15 secs
[ 4989.051158] nvmet: creating controller 1 for NQN
nqn.2014-08.org.nvmexpress:NVMf:uuid:0c8e193a-9c77-4e40-adff-07a7b8a352af.
[ 4989.149157] IP: [<ffffffff816e2b32>] _raw_spin_lock_irqsave+0x22/0x40
[ 4989.155978] PGD 858398067 PUD 858109067 PMD 0
[ 4989.160844] Oops: 0002 [#1] SMP
[ 4989.164359] Modules linked in: iw_cxgb4(OE) cxgb4(OE) null_blk brd
nvmet_rdma nvmet ip6table_filter ip6_tables ebtable_nat ebtables
ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat
nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack ipt_REJECT
nf_reject_ipv4
[ 4989.189404] nvmet: adding queue 1 to ctrl 1.
...
[ 4989.825586] nvmet: adding queue 30 to ctrl 1.
[ 4989.825643] nvmet: adding queue 31 to ctrl 1.
[ 4989.825700] nvmet: adding queue 32 to ctrl 1.
[ 4989.979826] [<ffffffff816e2b32>] _raw_spin_lock_irqsave+0x22/0x40
[ 4989.987220] RSP: 0018:ffff880fb5e6bbe0 EFLAGS: 00010046
[ 4989.993071] RAX: 0000000000000000 RBX: 0000000000000006 RCX:
dead000000000200
[ 4990.000777] RDX: 0000000000000001 RSI: 0000000000000212 RDI:
00000000000000b8
[ 4990.008492] RBP: ffff880fb5e6bbe8 R08: ffff880fac0c0228 R09:
ffff880fb5e6bb64
[ 4990.016219] R10: 000000000000086c R11: 0000000000000000 R12:
ffff880fac0c0228
[ 4990.023936] R13: ffff880fb5935828 R14: 0000000000000297 R15:
ffff880fac0c0000
[ 4990.031627] FS: 0000000000000000(0000) GS:ffff88105f380000(0000)
knlGS:0000000000000000
[ 4990.040274] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4990.046538] CR2: 00000000000000b8 CR3: 00000008587e9000 CR4:
00000000000406e0
[ 4990.054178] Stack:
[ 4990.056665] ffff880fb5935800 ffff880fb5e6bc38 ffffffffa0853642
ffff880fb5937cf0
[ 4990.064633] 0000000000000212 0000000000000000 ffff880fb5937c00
0000000000000000
[ 4990.072598] 0000000000010000 0000000000000000 0000000000000000
ffff880fb5e6bc88
[ 4990.080562] Call Trace:
[ 4990.083495] [<ffffffffa0853642>] nvmet_rdma_recv_done+0x162/0x19c
[nvmet_rdma]
[ 4990.091291] [<ffffffffa0255d48>] __ib_process_cq+0x48/0xc0 [ib_core]
[ 4990.098235] [<ffffffffa0255f1a>] ib_cq_poll_work+0x2a/0x70 [ib_core]
[ 4990.105191] [<ffffffff810a1483>] process_one_work+0x183/0x4d0
[ 4990.111555] [<ffffffff816deda0>] ? __schedule+0x1f0/0x5b0
[ 4990.117566] [<ffffffff816df260>] ? schedule+0x40/0xb0
[ 4990.123243] [<ffffffff810a211d>] worker_thread+0x16d/0x530
[ 4990.129367] [<ffffffff810b4785>] ?
trace_event_raw_event_sched_switch+0xe5/0x130
[ 4990.137418] [<ffffffff816deda0>] ? __schedule+0x1f0/0x5b0
[ 4990.143492] [<ffffffff810cb9b6>] ? __wake_up_common+0x56/0x90
[ 4990.149916] [<ffffffff810a1fb0>] ? maybe_create_worker+0x120/0x120
[ 4990.156784] [<ffffffff816df260>] ? schedule+0x40/0xb0
[ 4990.162535] [<ffffffff810a1fb0>] ? maybe_create_worker+0x120/0x120
[ 4990.169441] [<ffffffff810a6c5c>] kthread+0xcc/0xf0
[ 4990.174954] [<ffffffff810b1aae>] ? schedule_tail+0x1e/0xc0
[ 4990.181173] [<ffffffff816e2eff>] ret_from_fork+0x1f/0x40
[ 4990.187237] [<ffffffff810a6b90>] ?
kthread_freezable_should_stop+0x70/0x70
[ 4990.194876] Code: 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 53 66 66 66 66
90 9c 58 66 66 90 66 90 48 89 c3 fa 66 66 90 66 66 90 31 c0 ba 01 00 00 00
<f0> 0f b1 17 85 c0 75 06 48 89 d8 5b c9 c3 89 c6 e8 39 f9 9e ff
[ 4990.216376] RIP [<ffffffff816e2b32>] _raw_spin_lock_irqsave+0x22/0x40
[ 4990.223711] RSP <ffff880fb5e6bbe0>
[ 4990.227996] CR2: 00000000000000b8
Given the way the BUG and stack trace are interleaved with "adding queue"
messages, I guess this was happening on multiple cores. (the nodes have
32 cores).
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] net/mlx5: Simplify a test
From: David Miller @ 2016-11-01 18:12 UTC (permalink / raw)
To: matanb-VPRAkNaXOzVWk0Htik3J/w
Cc: christophe.jaillet-39ZsbGIQGT5GWvitb5QawA,
leonro-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <025cb4e1-f5e8-8bc9-0d4f-3ee8f8d1bb5d-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Date: Tue, 1 Nov 2016 11:38:18 +0200
> On 01/11/2016 09:10, Christophe JAILLET wrote:
>> 'create_root_ns()' does not return an error pointer, so the test can
>> be
>> simplified to be more consistent.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet-39ZsbGIQGT5GWvitb5QawA@public.gmane.org>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>> b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>> index 904853f9cf7a..330955f6badc 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>> @@ -1833,7 +1833,7 @@ static int init_root_ns(struct
>> mlx5_flow_steering *steering)
>> {
>>
>> steering->root_ns = create_root_ns(steering, FS_FT_NIC_RX);
>> - if (IS_ERR_OR_NULL(steering->root_ns))
>> + if (!steering->root_ns)
>> goto cleanup;
>>
>> if (init_root_tree(steering, &root_fs, &steering->root_ns->ns.node))
>>
>
> Thanks.
> Acked-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: nvmet_rdma crash - DISCONNECT event with NULL queue
From: Sagi Grimberg @ 2016-11-01 17:41 UTC (permalink / raw)
To: Steve Wise, 'Christoph Hellwig'
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <01d901d2345f$da0d2e00$8e278a00$@opengridcomputing.com>
> This looks good. But you mentioned the 2 rapid-fire keep alive timeout logs for
> the same controller as being seriously broken. Perhaps that is another problem?
> Maybe keep alives aren't getting stopped correctly or something...
>
> But: I'll try this patch and run for a few hours and see what happens. I
> believe regardless of a keep alive issue, the above patch is still needed.
In your tests, can you enable dynamic debug on:
nvmet_start_keep_alive_timer
nvmet_stop_keep_alive_timer
nvmet_execute_keep_alive
Thanks,
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: nvmet_rdma crash - DISCONNECT event with NULL queue
From: Steve Wise @ 2016-11-01 16:49 UTC (permalink / raw)
To: 'Sagi Grimberg', 'Christoph Hellwig'
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <dbe5f18d-7928-f065-920f-753b30fb99a2-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
> >> So I think that the patch from Bart a few weeks ago was correct:
> >>
> >
> > Not quite. It just guards against a null queue for TIMEWAIT_EXIT, which is
only
> > generated by the IB_CM.
>
> Yes, this is why we need ADDR_CHANGE and DISCONNECTED too
> "(and include all the relevant cases around it)"
>
> The other events we don't get to LIVE state and we don't have
> other error flows that will trigger queue teardown sequence.
>
> --
> nvmet-rdma: Fix possible NULL deref when handling rdma cm
> events
>
> When we initiate queue teardown sequence we call rdma_destroy_qp
> which clears cm_id->qp, afterwards we call rdma_destroy_id, but
> we might see a rdma_cm event in between with a cleared cm_id->qp
> so watch out for that and silently ignore the event because this
> means that the queue teardown sequence is in progress.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
> ---
> drivers/nvme/target/rdma.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
> index b4d648536c3e..240888efd920 100644
> --- a/drivers/nvme/target/rdma.c
> +++ b/drivers/nvme/target/rdma.c
> @@ -1351,7 +1351,13 @@ static int nvmet_rdma_cm_handler(struct
> rdma_cm_id *cm_id,
> case RDMA_CM_EVENT_ADDR_CHANGE:
> case RDMA_CM_EVENT_DISCONNECTED:
> case RDMA_CM_EVENT_TIMEWAIT_EXIT:
> - nvmet_rdma_queue_disconnect(queue);
> + /*
> + * We might end up here when we already freed the qp
> + * which means queue release sequence is in progress,
> + * so don't get in the way...
> + */
> + if (!queue)
> + nvmet_rdma_queue_disconnect(queue);
> break;
> case RDMA_CM_EVENT_DEVICE_REMOVAL:
> ret = nvmet_rdma_device_removal(cm_id, queue);
> --
This looks good. But you mentioned the 2 rapid-fire keep alive timeout logs for
the same controller as being seriously broken. Perhaps that is another problem?
Maybe keep alives aren't getting stopped correctly or something...
But: I'll try this patch and run for a few hours and see what happens. I
believe regardless of a keep alive issue, the above patch is still needed.
Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: nvmet_rdma crash - DISCONNECT event with NULL queue
From: Sagi Grimberg @ 2016-11-01 16:44 UTC (permalink / raw)
To: Steve Wise, 'Christoph Hellwig'
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <01d101d2345e$2f054390$8d0fcab0$@opengridcomputing.com>
>> pphh, somehow managed to miss it...
>>
>> So we have a case where we can call rdma_destroy_qp and
>> then rdma_destroy_id but still get events on the cm_id...
>> Not very nice...
>>
>> So I think that the patch from Bart a few weeks ago was correct:
>>
>
> Not quite. It just guards against a null queue for TIMEWAIT_EXIT, which is only
> generated by the IB_CM.
Yes, this is why we need ADDR_CHANGE and DISCONNECTED too
"(and include all the relevant cases around it)"
The other events we don't get to LIVE state and we don't have
other error flows that will trigger queue teardown sequence.
--
nvmet-rdma: Fix possible NULL deref when handling rdma cm
events
When we initiate queue teardown sequence we call rdma_destroy_qp
which clears cm_id->qp, afterwards we call rdma_destroy_id, but
we might see a rdma_cm event in between with a cleared cm_id->qp
so watch out for that and silently ignore the event because this
means that the queue teardown sequence is in progress.
Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
---
drivers/nvme/target/rdma.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index b4d648536c3e..240888efd920 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -1351,7 +1351,13 @@ static int nvmet_rdma_cm_handler(struct
rdma_cm_id *cm_id,
case RDMA_CM_EVENT_ADDR_CHANGE:
case RDMA_CM_EVENT_DISCONNECTED:
case RDMA_CM_EVENT_TIMEWAIT_EXIT:
- nvmet_rdma_queue_disconnect(queue);
+ /*
+ * We might end up here when we already freed the qp
+ * which means queue release sequence is in progress,
+ * so don't get in the way...
+ */
+ if (!queue)
+ nvmet_rdma_queue_disconnect(queue);
break;
case RDMA_CM_EVENT_DEVICE_REMOVAL:
ret = nvmet_rdma_device_removal(cm_id, queue);
--
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* RE: nvmet_rdma crash - DISCONNECT event with NULL queue
From: Steve Wise @ 2016-11-01 16:37 UTC (permalink / raw)
To: 'Sagi Grimberg', 'Christoph Hellwig'
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <4cc25277-429a-4ab9-470c-b3af1428ce93-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
>
> >>> I just hit an nvmf target NULL pointer deref BUG after a few hours of
> > keep-alive
> >>> timeout testing. It appears that nvmet_rdma_cm_handler() was called with
> >>> cm_id->qp == NULL, so the local nvmet_rdma_queue * variable queue is left
as
> >>> NULL. But then nvmet_rdma_queue_disconnect() is called with queue == NULL
> >> which
> >>> causes the crash.
> >>
> >> AFAICT, the only way cm_id->qp is NULL is for a scenario we didn't even
> >> get to allocate a queue-pair (e.g. calling rdma_create_qp). The teardown
> >> paths does not nullify cm_id->qp...
> >
> > rdma_destroy_qp() nulls out cm_id->qp.
>
> pphh, somehow managed to miss it...
>
> So we have a case where we can call rdma_destroy_qp and
> then rdma_destroy_id but still get events on the cm_id...
> Not very nice...
>
> So I think that the patch from Bart a few weeks ago was correct:
>
Not quite. It just guards against a null queue for TIMEWAIT_EXIT, which is only
generated by the IB_CM.
> ---
> drivers/nvme/target/rdma.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
> index d1aea17..a61e47f 100644
> --- a/drivers/nvme/target/rdma.c
> +++ b/drivers/nvme/target/rdma.c
> @@ -1354,9 +1354,12 @@ static int nvmet_rdma_cm_handler(struct
> rdma_cm_id *cm_id,
> break;
> case RDMA_CM_EVENT_ADDR_CHANGE:
> case RDMA_CM_EVENT_DISCONNECTED:
> - case RDMA_CM_EVENT_TIMEWAIT_EXIT:
> nvmet_rdma_queue_disconnect(queue);
> break;
> + case RDMA_CM_EVENT_TIMEWAIT_EXIT:
> + if (queue)
> + nvmet_rdma_queue_disconnect(queue);
> + break;
> case RDMA_CM_EVENT_DEVICE_REMOVAL:
> ret = nvmet_rdma_device_removal(cm_id, queue);
> break;
> ---
>
> In case this fixes the issue (as expected) I'll queue it up
> with a change log and a code comment on why we need to do
> this (and include all the relevant cases around it)...
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: nvmet_rdma crash - DISCONNECT event with NULL queue
From: Sagi Grimberg @ 2016-11-01 16:34 UTC (permalink / raw)
To: Steve Wise, 'Christoph Hellwig'
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <01cc01d2345b$d445acd0$7cd10670$@opengridcomputing.com>
>>> I just hit an nvmf target NULL pointer deref BUG after a few hours of
> keep-alive
>>> timeout testing. It appears that nvmet_rdma_cm_handler() was called with
>>> cm_id->qp == NULL, so the local nvmet_rdma_queue * variable queue is left as
>>> NULL. But then nvmet_rdma_queue_disconnect() is called with queue == NULL
>> which
>>> causes the crash.
>>
>> AFAICT, the only way cm_id->qp is NULL is for a scenario we didn't even
>> get to allocate a queue-pair (e.g. calling rdma_create_qp). The teardown
>> paths does not nullify cm_id->qp...
>
> rdma_destroy_qp() nulls out cm_id->qp.
pphh, somehow managed to miss it...
So we have a case where we can call rdma_destroy_qp and
then rdma_destroy_id but still get events on the cm_id...
Not very nice...
So I think that the patch from Bart a few weeks ago was correct:
---
drivers/nvme/target/rdma.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index d1aea17..a61e47f 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -1354,9 +1354,12 @@ static int nvmet_rdma_cm_handler(struct
rdma_cm_id *cm_id,
break;
case RDMA_CM_EVENT_ADDR_CHANGE:
case RDMA_CM_EVENT_DISCONNECTED:
- case RDMA_CM_EVENT_TIMEWAIT_EXIT:
nvmet_rdma_queue_disconnect(queue);
break;
+ case RDMA_CM_EVENT_TIMEWAIT_EXIT:
+ if (queue)
+ nvmet_rdma_queue_disconnect(queue);
+ break;
case RDMA_CM_EVENT_DEVICE_REMOVAL:
ret = nvmet_rdma_device_removal(cm_id, queue);
break;
---
In case this fixes the issue (as expected) I'll queue it up
with a change log and a code comment on why we need to do
this (and include all the relevant cases around it)...
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox