* [PATCH 1/2] IB/cm: Fix sleeping in atomic when RoCE is used @ 2017-08-29 17:34 Roland Dreier [not found] ` <20170829173444.4289-1-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 5+ messages in thread From: Roland Dreier @ 2017-08-29 17:34 UTC (permalink / raw) To: Doug Ledford, Sean Hefty; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> A couple of places in the CM do spin_lock_irq(&cm_id_priv->lock); ... if (cm_alloc_response_msg(work->port, work->mad_recv_wc, &msg)) However when the underlying transport is RoCE, this leads to a sleeping function being called with the lock held - the callchain is cm_alloc_response_msg() -> ib_create_ah_from_wc() -> ib_init_ah_from_wc() -> rdma_addr_find_l2_eth_by_grh() -> rdma_resolve_ip() and rdma_resolve_ip() starts out by doing req = kzalloc(sizeof *req, GFP_KERNEL); not to mention rdma_addr_find_l2_eth_by_grh() doing wait_for_completion(&ctx.comp); to wait for the task that rdma_resolve_ip() queues up. Fix this by moving the AH creation out of the lock. Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> --- drivers/infiniband/core/cm.c | 63 +++++++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 19 deletions(-) diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c index 2b4d613a3474..f814c5035c74 100644 --- a/drivers/infiniband/core/cm.c +++ b/drivers/infiniband/core/cm.c @@ -373,11 +373,19 @@ static int cm_alloc_msg(struct cm_id_private *cm_id_priv, return ret; } -static int cm_alloc_response_msg(struct cm_port *port, - struct ib_mad_recv_wc *mad_recv_wc, - struct ib_mad_send_buf **msg) +static struct ib_mad_send_buf *cm_alloc_response_msg_no_ah(struct cm_port *port, + struct ib_mad_recv_wc *mad_recv_wc) +{ + return ib_create_send_mad(port->mad_agent, 1, mad_recv_wc->wc->pkey_index, + 0, IB_MGMT_MAD_HDR, IB_MGMT_MAD_DATA, + GFP_ATOMIC, + IB_MGMT_BASE_VERSION); +} + +static int cm_create_response_msg_ah(struct cm_port *port, + struct ib_mad_recv_wc *mad_recv_wc, + struct ib_mad_send_buf *msg) { - struct ib_mad_send_buf *m; struct ib_ah *ah; ah = ib_create_ah_from_wc(port->mad_agent->qp->pd, mad_recv_wc->wc, @@ -385,27 +393,40 @@ static int cm_alloc_response_msg(struct cm_port *port, if (IS_ERR(ah)) return PTR_ERR(ah); - m = ib_create_send_mad(port->mad_agent, 1, mad_recv_wc->wc->pkey_index, - 0, IB_MGMT_MAD_HDR, IB_MGMT_MAD_DATA, - GFP_ATOMIC, - IB_MGMT_BASE_VERSION); - if (IS_ERR(m)) { - rdma_destroy_ah(ah); - return PTR_ERR(m); - } - m->ah = ah; - *msg = m; + msg->ah = ah; return 0; } static void cm_free_msg(struct ib_mad_send_buf *msg) { - rdma_destroy_ah(msg->ah); + if (msg->ah) + rdma_destroy_ah(msg->ah); if (msg->context[0]) cm_deref_id(msg->context[0]); ib_free_send_mad(msg); } +static int cm_alloc_response_msg(struct cm_port *port, + struct ib_mad_recv_wc *mad_recv_wc, + struct ib_mad_send_buf **msg) +{ + struct ib_mad_send_buf *m; + int ret; + + m = cm_alloc_response_msg_no_ah(port, mad_recv_wc); + if (IS_ERR(m)) + return PTR_ERR(m); + + ret = cm_create_response_msg_ah(port, mad_recv_wc, m); + if (ret) { + cm_free_msg(m); + return ret; + } + + *msg = m; + return 0; +} + static void * cm_copy_private_data(const void *private_data, u8 private_data_len) { @@ -2424,7 +2445,8 @@ static int cm_dreq_handler(struct cm_work *work) case IB_CM_TIMEWAIT: atomic_long_inc(&work->port->counter_group[CM_RECV_DUPLICATES]. counter[CM_DREQ_COUNTER]); - if (cm_alloc_response_msg(work->port, work->mad_recv_wc, &msg)) + msg = cm_alloc_response_msg_no_ah(work->port, work->mad_recv_wc); + if (IS_ERR(msg)) goto unlock; cm_format_drep((struct cm_drep_msg *) msg->mad, cm_id_priv, @@ -2432,7 +2454,8 @@ static int cm_dreq_handler(struct cm_work *work) cm_id_priv->private_data_len); spin_unlock_irq(&cm_id_priv->lock); - if (ib_post_send_mad(msg, NULL)) + if (cm_create_response_msg_ah(work->port, work->mad_recv_wc, msg) || + ib_post_send_mad(msg, NULL)) cm_free_msg(msg); goto deref; case IB_CM_DREQ_RCVD: @@ -2980,7 +3003,8 @@ static int cm_lap_handler(struct cm_work *work) case IB_CM_MRA_LAP_SENT: atomic_long_inc(&work->port->counter_group[CM_RECV_DUPLICATES]. counter[CM_LAP_COUNTER]); - if (cm_alloc_response_msg(work->port, work->mad_recv_wc, &msg)) + msg = cm_alloc_response_msg_no_ah(work->port, work->mad_recv_wc); + if (IS_ERR(msg)) goto unlock; cm_format_mra((struct cm_mra_msg *) msg->mad, cm_id_priv, @@ -2990,7 +3014,8 @@ static int cm_lap_handler(struct cm_work *work) cm_id_priv->private_data_len); spin_unlock_irq(&cm_id_priv->lock); - if (ib_post_send_mad(msg, NULL)) + if (cm_create_response_msg_ah(work->port, work->mad_recv_wc, msg) || + ib_post_send_mad(msg, NULL)) cm_free_msg(msg); goto deref; case IB_CM_LAP_RCVD: -- 2.14.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 [flat|nested] 5+ messages in thread
[parent not found: <20170829173444.4289-1-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* [PATCH 2/2] IB/core: Add might_sleep() annotation to ib_init_ah_from_wc() [not found] ` <20170829173444.4289-1-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2017-08-29 17:34 ` Roland Dreier [not found] ` <20170829173444.4289-2-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-08-29 18:37 ` [PATCH 1/2] IB/cm: Fix sleeping in atomic when RoCE is used Hefty, Sean 2017-08-30 14:23 ` Doug Ledford 2 siblings, 1 reply; 5+ messages in thread From: Roland Dreier @ 2017-08-29 17:34 UTC (permalink / raw) To: Doug Ledford, Sean Hefty; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> For RoCE, ib_init_ah_from_wc() can follow the path ib_init_ah_from_wc() -> rdma_addr_find_l2_eth_by_grh() -> rdma_resolve_ip() and rdma_resolve_ip() will sleep in kzalloc() and wait_for_completion(). However, developers will not see any warnings if they use ib_init_ah_from_wc() in an atomic context and test only on IB, because the function doesn't sleep in that case. Add a might_sleep() so that lockdep will catch bugs no matter what hardware is used to test. Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> --- drivers/infiniband/core/verbs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c index b456e3ca1876..84868f76f8b9 100644 --- a/drivers/infiniband/core/verbs.c +++ b/drivers/infiniband/core/verbs.c @@ -478,6 +478,8 @@ int ib_init_ah_from_wc(struct ib_device *device, u8 port_num, union ib_gid dgid; union ib_gid sgid; + might_sleep(); + memset(ah_attr, 0, sizeof *ah_attr); ah_attr->type = rdma_ah_find_type(device, port_num); if (rdma_cap_eth_ah(device, port_num)) { -- 2.14.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 [flat|nested] 5+ messages in thread
[parent not found: <20170829173444.4289-2-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 2/2] IB/core: Add might_sleep() annotation to ib_init_ah_from_wc() [not found] ` <20170829173444.4289-2-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2017-08-30 14:24 ` Doug Ledford 0 siblings, 0 replies; 5+ messages in thread From: Doug Ledford @ 2017-08-30 14:24 UTC (permalink / raw) To: Roland Dreier, Sean Hefty; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1.1: Type: text/plain, Size: 954 bytes --] On 8/29/2017 1:34 PM, Roland Dreier wrote: > From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> > > For RoCE, ib_init_ah_from_wc() can follow the path > > ib_init_ah_from_wc() -> > rdma_addr_find_l2_eth_by_grh() -> > rdma_resolve_ip() > > and rdma_resolve_ip() will sleep in kzalloc() and wait_for_completion(). > > However, developers will not see any warnings if they use ib_init_ah_from_wc() > in an atomic context and test only on IB, because the function doesn't > sleep in that case. > > Add a might_sleep() so that lockdep will catch bugs no matter what hardware is > used to test. > > Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> Thanks, will pick this up today too. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG Key ID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 1/2] IB/cm: Fix sleeping in atomic when RoCE is used [not found] ` <20170829173444.4289-1-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-08-29 17:34 ` [PATCH 2/2] IB/core: Add might_sleep() annotation to ib_init_ah_from_wc() Roland Dreier @ 2017-08-29 18:37 ` Hefty, Sean 2017-08-30 14:23 ` Doug Ledford 2 siblings, 0 replies; 5+ messages in thread From: Hefty, Sean @ 2017-08-29 18:37 UTC (permalink / raw) To: Roland Dreier, Doug Ledford; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA > From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> > > A couple of places in the CM do > > spin_lock_irq(&cm_id_priv->lock); > ... > if (cm_alloc_response_msg(work->port, work->mad_recv_wc, &msg)) > > However when the underlying transport is RoCE, this leads to a > sleeping function being called with the lock held - the callchain is > > cm_alloc_response_msg() -> > ib_create_ah_from_wc() -> > ib_init_ah_from_wc() -> > rdma_addr_find_l2_eth_by_grh() -> > rdma_resolve_ip() > > and rdma_resolve_ip() starts out by doing > > req = kzalloc(sizeof *req, GFP_KERNEL); > > not to mention rdma_addr_find_l2_eth_by_grh() doing > > wait_for_completion(&ctx.comp); > > to wait for the task that rdma_resolve_ip() queues up. > > Fix this by moving the AH creation out of the lock. > > Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> Reviewed-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> -- 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 [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] IB/cm: Fix sleeping in atomic when RoCE is used [not found] ` <20170829173444.4289-1-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-08-29 17:34 ` [PATCH 2/2] IB/core: Add might_sleep() annotation to ib_init_ah_from_wc() Roland Dreier 2017-08-29 18:37 ` [PATCH 1/2] IB/cm: Fix sleeping in atomic when RoCE is used Hefty, Sean @ 2017-08-30 14:23 ` Doug Ledford 2 siblings, 0 replies; 5+ messages in thread From: Doug Ledford @ 2017-08-30 14:23 UTC (permalink / raw) To: Roland Dreier, Sean Hefty; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1.1: Type: text/plain, Size: 1265 bytes --] On 8/29/2017 1:34 PM, Roland Dreier wrote: > From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> > > A couple of places in the CM do > > spin_lock_irq(&cm_id_priv->lock); > ... > if (cm_alloc_response_msg(work->port, work->mad_recv_wc, &msg)) > > However when the underlying transport is RoCE, this leads to a sleeping function > being called with the lock held - the callchain is > > cm_alloc_response_msg() -> > ib_create_ah_from_wc() -> > ib_init_ah_from_wc() -> > rdma_addr_find_l2_eth_by_grh() -> > rdma_resolve_ip() > > and rdma_resolve_ip() starts out by doing > > req = kzalloc(sizeof *req, GFP_KERNEL); > > not to mention rdma_addr_find_l2_eth_by_grh() doing > > wait_for_completion(&ctx.comp); > > to wait for the task that rdma_resolve_ip() queues up. > > Fix this by moving the AH creation out of the lock. > > Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> This looks good Roland, thanks for that. Will apply today. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG Key ID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-08-30 14:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-29 17:34 [PATCH 1/2] IB/cm: Fix sleeping in atomic when RoCE is used Roland Dreier [not found] ` <20170829173444.4289-1-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-08-29 17:34 ` [PATCH 2/2] IB/core: Add might_sleep() annotation to ib_init_ah_from_wc() Roland Dreier [not found] ` <20170829173444.4289-2-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-08-30 14:24 ` Doug Ledford 2017-08-29 18:37 ` [PATCH 1/2] IB/cm: Fix sleeping in atomic when RoCE is used Hefty, Sean 2017-08-30 14:23 ` Doug Ledford
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.