* [PATCH rdma-rc] IB/bnxt_re: Fix frame stack compilation warning @ 2017-09-19 10:22 Leon Romanovsky [not found] ` <20170919102213.30911-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Leon Romanovsky @ 2017-09-19 10:22 UTC (permalink / raw) To: Doug Ledford Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky, Selvin Xavier Reduce stack size by dynamically allocating memory instead of declaring large struct on the stack: drivers/infiniband/hw/bnxt_re/ib_verbs.c: In function ‘bnxt_re_query_qp’: drivers/infiniband/hw/bnxt_re/ib_verbs.c:1600:1: warning: the frame size of 1216 bytes is larger than 1024 bytes [-Wframe-larger-than=] } ^ Cc: Selvin Xavier <selvin.xavier-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver") Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> --- drivers/infiniband/hw/bnxt_re/ib_verbs.c | 67 +++++++++++++++++--------------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c index 01eee15bbd65..03caf48af8e2 100644 --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c @@ -1551,43 +1551,46 @@ int bnxt_re_query_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr, { struct bnxt_re_qp *qp = container_of(ib_qp, struct bnxt_re_qp, ib_qp); struct bnxt_re_dev *rdev = qp->rdev; - struct bnxt_qplib_qp qplib_qp; + struct bnxt_qplib_qp *qplib_qp; int rc; - memset(&qplib_qp, 0, sizeof(struct bnxt_qplib_qp)); - qplib_qp.id = qp->qplib_qp.id; - qplib_qp.ah.host_sgid_index = qp->qplib_qp.ah.host_sgid_index; + qplib_qp = kzalloc(sizeof(*qplib_qp), GFP_KERNEL); + if (!qplib_qp) + return -ENOMEM; + + qplib_qp->id = qp->qplib_qp.id; + qplib_qp->ah.host_sgid_index = qp->qplib_qp.ah.host_sgid_index; - rc = bnxt_qplib_query_qp(&rdev->qplib_res, &qplib_qp); + rc = bnxt_qplib_query_qp(&rdev->qplib_res, qplib_qp); if (rc) { dev_err(rdev_to_dev(rdev), "Failed to query HW QP"); - return rc; + goto out; } - qp_attr->qp_state = __to_ib_qp_state(qplib_qp.state); - qp_attr->en_sqd_async_notify = qplib_qp.en_sqd_async_notify ? 1 : 0; - qp_attr->qp_access_flags = __to_ib_access_flags(qplib_qp.access); - qp_attr->pkey_index = qplib_qp.pkey_index; - qp_attr->qkey = qplib_qp.qkey; + qp_attr->qp_state = __to_ib_qp_state(qplib_qp->state); + qp_attr->en_sqd_async_notify = qplib_qp->en_sqd_async_notify ? 1 : 0; + qp_attr->qp_access_flags = __to_ib_access_flags(qplib_qp->access); + qp_attr->pkey_index = qplib_qp->pkey_index; + qp_attr->qkey = qplib_qp->qkey; qp_attr->ah_attr.type = RDMA_AH_ATTR_TYPE_ROCE; - rdma_ah_set_grh(&qp_attr->ah_attr, NULL, qplib_qp.ah.flow_label, - qplib_qp.ah.host_sgid_index, - qplib_qp.ah.hop_limit, - qplib_qp.ah.traffic_class); - rdma_ah_set_dgid_raw(&qp_attr->ah_attr, qplib_qp.ah.dgid.data); - rdma_ah_set_sl(&qp_attr->ah_attr, qplib_qp.ah.sl); - ether_addr_copy(qp_attr->ah_attr.roce.dmac, qplib_qp.ah.dmac); - qp_attr->path_mtu = __to_ib_mtu(qplib_qp.path_mtu); - qp_attr->timeout = qplib_qp.timeout; - qp_attr->retry_cnt = qplib_qp.retry_cnt; - qp_attr->rnr_retry = qplib_qp.rnr_retry; - qp_attr->min_rnr_timer = qplib_qp.min_rnr_timer; - qp_attr->rq_psn = qplib_qp.rq.psn; - qp_attr->max_rd_atomic = qplib_qp.max_rd_atomic; - qp_attr->sq_psn = qplib_qp.sq.psn; - qp_attr->max_dest_rd_atomic = qplib_qp.max_dest_rd_atomic; - qp_init_attr->sq_sig_type = qplib_qp.sig_type ? IB_SIGNAL_ALL_WR : - IB_SIGNAL_REQ_WR; - qp_attr->dest_qp_num = qplib_qp.dest_qpn; + rdma_ah_set_grh(&qp_attr->ah_attr, NULL, qplib_qp->ah.flow_label, + qplib_qp->ah.host_sgid_index, + qplib_qp->ah.hop_limit, + qplib_qp->ah.traffic_class); + rdma_ah_set_dgid_raw(&qp_attr->ah_attr, qplib_qp->ah.dgid.data); + rdma_ah_set_sl(&qp_attr->ah_attr, qplib_qp->ah.sl); + ether_addr_copy(qp_attr->ah_attr.roce.dmac, qplib_qp->ah.dmac); + qp_attr->path_mtu = __to_ib_mtu(qplib_qp->path_mtu); + qp_attr->timeout = qplib_qp->timeout; + qp_attr->retry_cnt = qplib_qp->retry_cnt; + qp_attr->rnr_retry = qplib_qp->rnr_retry; + qp_attr->min_rnr_timer = qplib_qp->min_rnr_timer; + qp_attr->rq_psn = qplib_qp->rq.psn; + qp_attr->max_rd_atomic = qplib_qp->max_rd_atomic; + qp_attr->sq_psn = qplib_qp->sq.psn; + qp_attr->max_dest_rd_atomic = qplib_qp->max_dest_rd_atomic; + qp_init_attr->sq_sig_type = qplib_qp->sig_type ? IB_SIGNAL_ALL_WR : + IB_SIGNAL_REQ_WR; + qp_attr->dest_qp_num = qplib_qp->dest_qpn; qp_attr->cap.max_send_wr = qp->qplib_qp.sq.max_wqe; qp_attr->cap.max_send_sge = qp->qplib_qp.sq.max_sge; @@ -1596,7 +1599,9 @@ int bnxt_re_query_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr, qp_attr->cap.max_inline_data = qp->qplib_qp.max_inline_data; qp_init_attr->cap = qp_attr->cap; - return 0; +out: + kfree(qplib_qp); + return rc; } /* Routine for sending QP1 packets for RoCE V1 an V2 -- 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] 6+ messages in thread
[parent not found: <20170919102213.30911-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH rdma-rc] IB/bnxt_re: Fix frame stack compilation warning [not found] ` <20170919102213.30911-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2017-09-19 18:24 ` Jonathan Toppins [not found] ` <b73b905c-baa2-0cb6-bccc-5f259ccedc48-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2017-09-22 17:19 ` Doug Ledford 1 sibling, 1 reply; 6+ messages in thread From: Jonathan Toppins @ 2017-09-19 18:24 UTC (permalink / raw) To: Leon Romanovsky, Doug Ledford Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Selvin Xavier On 09/19/2017 06:22 AM, Leon Romanovsky wrote: > Reduce stack size by dynamically allocating memory instead > of declaring large struct on the stack: > > drivers/infiniband/hw/bnxt_re/ib_verbs.c: In function ‘bnxt_re_query_qp’: > drivers/infiniband/hw/bnxt_re/ib_verbs.c:1600:1: warning: the frame size of 1216 bytes is larger than 1024 bytes [-Wframe-larger-than=] > } > ^ > > Cc: Selvin Xavier <selvin.xavier-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> > Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver") > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > --- > drivers/infiniband/hw/bnxt_re/ib_verbs.c | 67 +++++++++++++++++--------------- > 1 file changed, 36 insertions(+), 31 deletions(-) > > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c > index 01eee15bbd65..03caf48af8e2 100644 > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c > @@ -1551,43 +1551,46 @@ int bnxt_re_query_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr, > { > struct bnxt_re_qp *qp = container_of(ib_qp, struct bnxt_re_qp, ib_qp); > struct bnxt_re_dev *rdev = qp->rdev; > - struct bnxt_qplib_qp qplib_qp; > + struct bnxt_qplib_qp *qplib_qp; > int rc; > > - memset(&qplib_qp, 0, sizeof(struct bnxt_qplib_qp)); > - qplib_qp.id = qp->qplib_qp.id; > - qplib_qp.ah.host_sgid_index = qp->qplib_qp.ah.host_sgid_index; > + qplib_qp = kzalloc(sizeof(*qplib_qp), GFP_KERNEL); ^^^^ Can this process block? Further down in the code I see a call to kzalloc and they specifically use GFP_ATOMIC. bnxt_qplib_query_qp() -> bnxt_qplib_rcfw_alloc_sbuf() Looking through the rest of the function and what it calls I am thinking this function is assumed to not block so I think GFP_ATOMIC should be used here. > + if (!qplib_qp) > + return -ENOMEM; > + > + qplib_qp->id = qp->qplib_qp.id; > + qplib_qp->ah.host_sgid_index = qp->qplib_qp.ah.host_sgid_index; > > - rc = bnxt_qplib_query_qp(&rdev->qplib_res, &qplib_qp); > + rc = bnxt_qplib_query_qp(&rdev->qplib_res, qplib_qp); > if (rc) { > dev_err(rdev_to_dev(rdev), "Failed to query HW QP"); > - return rc; > + goto out; > } > - qp_attr->qp_state = __to_ib_qp_state(qplib_qp.state); > - qp_attr->en_sqd_async_notify = qplib_qp.en_sqd_async_notify ? 1 : 0; > - qp_attr->qp_access_flags = __to_ib_access_flags(qplib_qp.access); > - qp_attr->pkey_index = qplib_qp.pkey_index; > - qp_attr->qkey = qplib_qp.qkey; > + qp_attr->qp_state = __to_ib_qp_state(qplib_qp->state); > + qp_attr->en_sqd_async_notify = qplib_qp->en_sqd_async_notify ? 1 : 0; > + qp_attr->qp_access_flags = __to_ib_access_flags(qplib_qp->access); > + qp_attr->pkey_index = qplib_qp->pkey_index; > + qp_attr->qkey = qplib_qp->qkey; > qp_attr->ah_attr.type = RDMA_AH_ATTR_TYPE_ROCE; > - rdma_ah_set_grh(&qp_attr->ah_attr, NULL, qplib_qp.ah.flow_label, > - qplib_qp.ah.host_sgid_index, > - qplib_qp.ah.hop_limit, > - qplib_qp.ah.traffic_class); > - rdma_ah_set_dgid_raw(&qp_attr->ah_attr, qplib_qp.ah.dgid.data); > - rdma_ah_set_sl(&qp_attr->ah_attr, qplib_qp.ah.sl); > - ether_addr_copy(qp_attr->ah_attr.roce.dmac, qplib_qp.ah.dmac); > - qp_attr->path_mtu = __to_ib_mtu(qplib_qp.path_mtu); > - qp_attr->timeout = qplib_qp.timeout; > - qp_attr->retry_cnt = qplib_qp.retry_cnt; > - qp_attr->rnr_retry = qplib_qp.rnr_retry; > - qp_attr->min_rnr_timer = qplib_qp.min_rnr_timer; > - qp_attr->rq_psn = qplib_qp.rq.psn; > - qp_attr->max_rd_atomic = qplib_qp.max_rd_atomic; > - qp_attr->sq_psn = qplib_qp.sq.psn; > - qp_attr->max_dest_rd_atomic = qplib_qp.max_dest_rd_atomic; > - qp_init_attr->sq_sig_type = qplib_qp.sig_type ? IB_SIGNAL_ALL_WR : > - IB_SIGNAL_REQ_WR; > - qp_attr->dest_qp_num = qplib_qp.dest_qpn; > + rdma_ah_set_grh(&qp_attr->ah_attr, NULL, qplib_qp->ah.flow_label, > + qplib_qp->ah.host_sgid_index, > + qplib_qp->ah.hop_limit, > + qplib_qp->ah.traffic_class); > + rdma_ah_set_dgid_raw(&qp_attr->ah_attr, qplib_qp->ah.dgid.data); > + rdma_ah_set_sl(&qp_attr->ah_attr, qplib_qp->ah.sl); > + ether_addr_copy(qp_attr->ah_attr.roce.dmac, qplib_qp->ah.dmac); > + qp_attr->path_mtu = __to_ib_mtu(qplib_qp->path_mtu); > + qp_attr->timeout = qplib_qp->timeout; > + qp_attr->retry_cnt = qplib_qp->retry_cnt; > + qp_attr->rnr_retry = qplib_qp->rnr_retry; > + qp_attr->min_rnr_timer = qplib_qp->min_rnr_timer; > + qp_attr->rq_psn = qplib_qp->rq.psn; > + qp_attr->max_rd_atomic = qplib_qp->max_rd_atomic; > + qp_attr->sq_psn = qplib_qp->sq.psn; > + qp_attr->max_dest_rd_atomic = qplib_qp->max_dest_rd_atomic; > + qp_init_attr->sq_sig_type = qplib_qp->sig_type ? IB_SIGNAL_ALL_WR : > + IB_SIGNAL_REQ_WR; > + qp_attr->dest_qp_num = qplib_qp->dest_qpn; > > qp_attr->cap.max_send_wr = qp->qplib_qp.sq.max_wqe; > qp_attr->cap.max_send_sge = qp->qplib_qp.sq.max_sge; > @@ -1596,7 +1599,9 @@ int bnxt_re_query_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr, > qp_attr->cap.max_inline_data = qp->qplib_qp.max_inline_data; > qp_init_attr->cap = qp_attr->cap; > > - return 0; > +out: > + kfree(qplib_qp); > + return rc; > } > > /* Routine for sending QP1 packets for RoCE V1 an V2 > -- > 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 > -- 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] 6+ messages in thread
[parent not found: <b73b905c-baa2-0cb6-bccc-5f259ccedc48-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH rdma-rc] IB/bnxt_re: Fix frame stack compilation warning [not found] ` <b73b905c-baa2-0cb6-bccc-5f259ccedc48-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2017-09-19 20:56 ` Leon Romanovsky [not found] ` <20170919205647.GN5788-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Leon Romanovsky @ 2017-09-19 20:56 UTC (permalink / raw) To: Jonathan Toppins Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Selvin Xavier [-- Attachment #1: Type: text/plain, Size: 6180 bytes --] On Tue, Sep 19, 2017 at 02:24:19PM -0400, Jonathan Toppins wrote: > On 09/19/2017 06:22 AM, Leon Romanovsky wrote: > > Reduce stack size by dynamically allocating memory instead > > of declaring large struct on the stack: > > > > drivers/infiniband/hw/bnxt_re/ib_verbs.c: In function ‘bnxt_re_query_qp’: > > drivers/infiniband/hw/bnxt_re/ib_verbs.c:1600:1: warning: the frame size of 1216 bytes is larger than 1024 bytes [-Wframe-larger-than=] > > } > > ^ > > > > Cc: Selvin Xavier <selvin.xavier-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> > > Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver") > > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > > --- > > drivers/infiniband/hw/bnxt_re/ib_verbs.c | 67 +++++++++++++++++--------------- > > 1 file changed, 36 insertions(+), 31 deletions(-) > > > > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c > > index 01eee15bbd65..03caf48af8e2 100644 > > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c > > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c > > @@ -1551,43 +1551,46 @@ int bnxt_re_query_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr, > > { > > struct bnxt_re_qp *qp = container_of(ib_qp, struct bnxt_re_qp, ib_qp); > > struct bnxt_re_dev *rdev = qp->rdev; > > - struct bnxt_qplib_qp qplib_qp; > > + struct bnxt_qplib_qp *qplib_qp; > > int rc; > > > > - memset(&qplib_qp, 0, sizeof(struct bnxt_qplib_qp)); > > - qplib_qp.id = qp->qplib_qp.id; > > - qplib_qp.ah.host_sgid_index = qp->qplib_qp.ah.host_sgid_index; > > + qplib_qp = kzalloc(sizeof(*qplib_qp), GFP_KERNEL); > ^^^^ > Can this process block? Further down in the code I see a call to kzalloc > and they specifically use GFP_ATOMIC. > > bnxt_qplib_query_qp() -> > bnxt_qplib_rcfw_alloc_sbuf() > > Looking through the rest of the function and what it calls I am thinking > this function is assumed to not block so I think GFP_ATOMIC should be > used here. bnxt_re_query_qp() is called as callback of ibdev->query_qp and it is unlikely that this function will be called in non-blocking context. For me, It looks like an error to use GFP_ATOMIC down the call stack. P.S. Our mlx4/mlx5 use GFP_KERNEL in query_qp callbacks. > > > + if (!qplib_qp) > > + return -ENOMEM; > > + > > + qplib_qp->id = qp->qplib_qp.id; > > + qplib_qp->ah.host_sgid_index = qp->qplib_qp.ah.host_sgid_index; > > > > - rc = bnxt_qplib_query_qp(&rdev->qplib_res, &qplib_qp); > > + rc = bnxt_qplib_query_qp(&rdev->qplib_res, qplib_qp); > > if (rc) { > > dev_err(rdev_to_dev(rdev), "Failed to query HW QP"); > > - return rc; > > + goto out; > > } > > - qp_attr->qp_state = __to_ib_qp_state(qplib_qp.state); > > - qp_attr->en_sqd_async_notify = qplib_qp.en_sqd_async_notify ? 1 : 0; > > - qp_attr->qp_access_flags = __to_ib_access_flags(qplib_qp.access); > > - qp_attr->pkey_index = qplib_qp.pkey_index; > > - qp_attr->qkey = qplib_qp.qkey; > > + qp_attr->qp_state = __to_ib_qp_state(qplib_qp->state); > > + qp_attr->en_sqd_async_notify = qplib_qp->en_sqd_async_notify ? 1 : 0; > > + qp_attr->qp_access_flags = __to_ib_access_flags(qplib_qp->access); > > + qp_attr->pkey_index = qplib_qp->pkey_index; > > + qp_attr->qkey = qplib_qp->qkey; > > qp_attr->ah_attr.type = RDMA_AH_ATTR_TYPE_ROCE; > > - rdma_ah_set_grh(&qp_attr->ah_attr, NULL, qplib_qp.ah.flow_label, > > - qplib_qp.ah.host_sgid_index, > > - qplib_qp.ah.hop_limit, > > - qplib_qp.ah.traffic_class); > > - rdma_ah_set_dgid_raw(&qp_attr->ah_attr, qplib_qp.ah.dgid.data); > > - rdma_ah_set_sl(&qp_attr->ah_attr, qplib_qp.ah.sl); > > - ether_addr_copy(qp_attr->ah_attr.roce.dmac, qplib_qp.ah.dmac); > > - qp_attr->path_mtu = __to_ib_mtu(qplib_qp.path_mtu); > > - qp_attr->timeout = qplib_qp.timeout; > > - qp_attr->retry_cnt = qplib_qp.retry_cnt; > > - qp_attr->rnr_retry = qplib_qp.rnr_retry; > > - qp_attr->min_rnr_timer = qplib_qp.min_rnr_timer; > > - qp_attr->rq_psn = qplib_qp.rq.psn; > > - qp_attr->max_rd_atomic = qplib_qp.max_rd_atomic; > > - qp_attr->sq_psn = qplib_qp.sq.psn; > > - qp_attr->max_dest_rd_atomic = qplib_qp.max_dest_rd_atomic; > > - qp_init_attr->sq_sig_type = qplib_qp.sig_type ? IB_SIGNAL_ALL_WR : > > - IB_SIGNAL_REQ_WR; > > - qp_attr->dest_qp_num = qplib_qp.dest_qpn; > > + rdma_ah_set_grh(&qp_attr->ah_attr, NULL, qplib_qp->ah.flow_label, > > + qplib_qp->ah.host_sgid_index, > > + qplib_qp->ah.hop_limit, > > + qplib_qp->ah.traffic_class); > > + rdma_ah_set_dgid_raw(&qp_attr->ah_attr, qplib_qp->ah.dgid.data); > > + rdma_ah_set_sl(&qp_attr->ah_attr, qplib_qp->ah.sl); > > + ether_addr_copy(qp_attr->ah_attr.roce.dmac, qplib_qp->ah.dmac); > > + qp_attr->path_mtu = __to_ib_mtu(qplib_qp->path_mtu); > > + qp_attr->timeout = qplib_qp->timeout; > > + qp_attr->retry_cnt = qplib_qp->retry_cnt; > > + qp_attr->rnr_retry = qplib_qp->rnr_retry; > > + qp_attr->min_rnr_timer = qplib_qp->min_rnr_timer; > > + qp_attr->rq_psn = qplib_qp->rq.psn; > > + qp_attr->max_rd_atomic = qplib_qp->max_rd_atomic; > > + qp_attr->sq_psn = qplib_qp->sq.psn; > > + qp_attr->max_dest_rd_atomic = qplib_qp->max_dest_rd_atomic; > > + qp_init_attr->sq_sig_type = qplib_qp->sig_type ? IB_SIGNAL_ALL_WR : > > + IB_SIGNAL_REQ_WR; > > + qp_attr->dest_qp_num = qplib_qp->dest_qpn; > > > > qp_attr->cap.max_send_wr = qp->qplib_qp.sq.max_wqe; > > qp_attr->cap.max_send_sge = qp->qplib_qp.sq.max_sge; > > @@ -1596,7 +1599,9 @@ int bnxt_re_query_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr, > > qp_attr->cap.max_inline_data = qp->qplib_qp.max_inline_data; > > qp_init_attr->cap = qp_attr->cap; > > > > - return 0; > > +out: > > + kfree(qplib_qp); > > + return rc; > > } > > > > /* Routine for sending QP1 packets for RoCE V1 an V2 > > -- > > 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 > > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <20170919205647.GN5788-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>]
* Re: [PATCH rdma-rc] IB/bnxt_re: Fix frame stack compilation warning [not found] ` <20170919205647.GN5788-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org> @ 2017-09-20 6:31 ` Selvin Xavier 2017-09-20 15:27 ` Jonathan Toppins 0 siblings, 1 reply; 6+ messages in thread From: Selvin Xavier @ 2017-09-20 6:31 UTC (permalink / raw) To: Leon Romanovsky Cc: Jonathan Toppins, Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA On Wed, Sep 20, 2017 at 2:26 AM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: > On Tue, Sep 19, 2017 at 02:24:19PM -0400, Jonathan Toppins wrote: >> On 09/19/2017 06:22 AM, Leon Romanovsky wrote: >> > Reduce stack size by dynamically allocating memory instead >> > of declaring large struct on the stack: >> > >> > drivers/infiniband/hw/bnxt_re/ib_verbs.c: In function ‘bnxt_re_query_qp’: >> > drivers/infiniband/hw/bnxt_re/ib_verbs.c:1600:1: warning: the frame size of 1216 bytes is larger than 1024 bytes [-Wframe-larger-than=] >> > } >> > ^ >> > >> > - memset(&qplib_qp, 0, sizeof(struct bnxt_qplib_qp)); >> > - qplib_qp.id = qp->qplib_qp.id; >> > - qplib_qp.ah.host_sgid_index = qp->qplib_qp.ah.host_sgid_index; >> > + qplib_qp = kzalloc(sizeof(*qplib_qp), GFP_KERNEL); >> ^^^^ >> Can this process block? Further down in the code I see a call to kzalloc >> and they specifically use GFP_ATOMIC. >> >> bnxt_qplib_query_qp() -> >> bnxt_qplib_rcfw_alloc_sbuf() >> >> Looking through the rest of the function and what it calls I am thinking >> this function is assumed to not block so I think GFP_ATOMIC should be >> used here. > > bnxt_re_query_qp() is called as callback of ibdev->query_qp and it is > unlikely that this function will be called in non-blocking context. > > For me, It looks like an error to use GFP_ATOMIC down the call stack. > > P.S. Our mlx4/mlx5 use GFP_KERNEL in query_qp callbacks. > Thanks Leon and Jonathan for your comments. Agree that bnxt_qplib_rcfw_alloc_sbuf implementation can use GFP_KERNEL. GFP_ATOMIC was used because this is a generic function. But none of the current callers of this function are in atomic context. I will post a patch to correct it. Thanks Leon for taking care of this. Acked-by: Selvin Xavier <selvin.xavier-dY08KVG/lbpWk0Htik3J/w@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] 6+ messages in thread
* Re: [PATCH rdma-rc] IB/bnxt_re: Fix frame stack compilation warning 2017-09-20 6:31 ` Selvin Xavier @ 2017-09-20 15:27 ` Jonathan Toppins 0 siblings, 0 replies; 6+ messages in thread From: Jonathan Toppins @ 2017-09-20 15:27 UTC (permalink / raw) To: Selvin Xavier, Leon Romanovsky Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA On 09/20/2017 02:31 AM, Selvin Xavier wrote: > On Wed, Sep 20, 2017 at 2:26 AM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: >> On Tue, Sep 19, 2017 at 02:24:19PM -0400, Jonathan Toppins wrote: >>> On 09/19/2017 06:22 AM, Leon Romanovsky wrote: >>>> Reduce stack size by dynamically allocating memory instead >>>> of declaring large struct on the stack: >>>> >>>> drivers/infiniband/hw/bnxt_re/ib_verbs.c: In function ‘bnxt_re_query_qp’: >>>> drivers/infiniband/hw/bnxt_re/ib_verbs.c:1600:1: warning: the frame size of 1216 bytes is larger than 1024 bytes [-Wframe-larger-than=] >>>> } >>>> ^ >>>> > >>>> - memset(&qplib_qp, 0, sizeof(struct bnxt_qplib_qp)); >>>> - qplib_qp.id = qp->qplib_qp.id; >>>> - qplib_qp.ah.host_sgid_index = qp->qplib_qp.ah.host_sgid_index; >>>> + qplib_qp = kzalloc(sizeof(*qplib_qp), GFP_KERNEL); >>> ^^^^ >>> Can this process block? Further down in the code I see a call to kzalloc >>> and they specifically use GFP_ATOMIC. >>> >>> bnxt_qplib_query_qp() -> >>> bnxt_qplib_rcfw_alloc_sbuf() >>> >>> Looking through the rest of the function and what it calls I am thinking >>> this function is assumed to not block so I think GFP_ATOMIC should be >>> used here. >> >> bnxt_re_query_qp() is called as callback of ibdev->query_qp and it is >> unlikely that this function will be called in non-blocking context. >> >> For me, It looks like an error to use GFP_ATOMIC down the call stack. >> >> P.S. Our mlx4/mlx5 use GFP_KERNEL in query_qp callbacks. >> > Thanks Leon and Jonathan for your comments. > Agree that bnxt_qplib_rcfw_alloc_sbuf implementation can use GFP_KERNEL. > GFP_ATOMIC was used because this is a generic function. But none of the > current callers of this function are in atomic context. I will post a > patch to correct it. > > Thanks Leon for taking care of this. > > Acked-by: Selvin Xavier <selvin.xavier-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> > With Broadcom's response I am good with the changes. Reviewed-by: Jonathan Toppins <jtoppins-H+wXaHxf7aLQT0dZR+AlfA@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] 6+ messages in thread
* Re: [PATCH rdma-rc] IB/bnxt_re: Fix frame stack compilation warning [not found] ` <20170919102213.30911-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-09-19 18:24 ` Jonathan Toppins @ 2017-09-22 17:19 ` Doug Ledford 1 sibling, 0 replies; 6+ messages in thread From: Doug Ledford @ 2017-09-22 17:19 UTC (permalink / raw) To: Leon Romanovsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Selvin Xavier On Tue, 2017-09-19 at 13:22 +0300, Leon Romanovsky wrote: > Reduce stack size by dynamically allocating memory instead > of declaring large struct on the stack: > > drivers/infiniband/hw/bnxt_re/ib_verbs.c: In function > ‘bnxt_re_query_qp’: > drivers/infiniband/hw/bnxt_re/ib_verbs.c:1600:1: warning: the frame > size of 1216 bytes is larger than 1024 bytes [-Wframe-larger-than=] > } > ^ > > Cc: Selvin Xavier <selvin.xavier-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> > Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver") > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Thanks, applied. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD -- 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] 6+ messages in thread
end of thread, other threads:[~2017-09-22 17:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-19 10:22 [PATCH rdma-rc] IB/bnxt_re: Fix frame stack compilation warning Leon Romanovsky [not found] ` <20170919102213.30911-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-09-19 18:24 ` Jonathan Toppins [not found] ` <b73b905c-baa2-0cb6-bccc-5f259ccedc48-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2017-09-19 20:56 ` Leon Romanovsky [not found] ` <20170919205647.GN5788-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org> 2017-09-20 6:31 ` Selvin Xavier 2017-09-20 15:27 ` Jonathan Toppins 2017-09-22 17:19 ` Doug Ledford
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).