* [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
* 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
* 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
* 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).