* [PATCH] i40iw: Set 128B as the only supported RQ WQE size
@ 2016-12-19 20:32 Henry Orosco
[not found] ` <20161219203227.86392-1-henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Henry Orosco @ 2016-12-19 20:32 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Chien Tin Tung
From: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
RQ WQE size other than 128B is not supported. Correct
RQ size calculation to use 128B only.
Since this breaks AIB, add additional code to
provide compatibility with v4 user provider, libi40iw.
Signed-off-by: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_ctrl.c | 25 ++++++++++++++++++++-----
drivers/infiniband/hw/i40iw/i40iw_puda.c | 2 +-
drivers/infiniband/hw/i40iw/i40iw_type.h | 4 +++-
drivers/infiniband/hw/i40iw/i40iw_ucontext.h | 4 ++--
drivers/infiniband/hw/i40iw/i40iw_uk.c | 17 ++++++++++++-----
drivers/infiniband/hw/i40iw/i40iw_user.h | 4 +++-
drivers/infiniband/hw/i40iw/i40iw_verbs.c | 21 +++++++++++----------
drivers/infiniband/hw/i40iw/i40iw_verbs.h | 1 +
8 files changed, 53 insertions(+), 25 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
index 392f783..98923a8 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
@@ -358,13 +358,16 @@ void i40iw_qp_add_qos(struct i40iw_sc_qp *qp)
* @dev: sc device struct
* @pd: sc pd ptr
* @pd_id: pd_id for allocated pd
+ * @abi_ver: ABI version from user context, -1 if not valid
*/
static void i40iw_sc_pd_init(struct i40iw_sc_dev *dev,
struct i40iw_sc_pd *pd,
- u16 pd_id)
+ u16 pd_id,
+ int abi_ver)
{
pd->size = sizeof(*pd);
pd->pd_id = pd_id;
+ pd->abi_ver = abi_ver;
pd->dev = dev;
}
@@ -2252,6 +2255,7 @@ static enum i40iw_status_code i40iw_sc_qp_init(struct i40iw_sc_qp *qp,
offset);
info->qp_uk_init_info.wqe_alloc_reg = wqe_alloc_reg;
+ info->qp_uk_init_info.abi_ver = qp->pd->abi_ver;
ret_code = i40iw_qp_uk_init(&qp->qp_uk, &info->qp_uk_init_info);
if (ret_code)
return ret_code;
@@ -2270,10 +2274,21 @@ static enum i40iw_status_code i40iw_sc_qp_init(struct i40iw_sc_qp *qp,
false);
i40iw_debug(qp->dev, I40IW_DEBUG_WQE, "%s: hw_sq_size[%04d] sq_ring.size[%04d]\n",
__func__, qp->hw_sq_size, qp->qp_uk.sq_ring.size);
- ret_code = i40iw_fragcnt_to_wqesize_rq(qp->qp_uk.max_rq_frag_cnt,
- &wqe_size);
- if (ret_code)
- return ret_code;
+
+ switch (qp->pd->abi_ver) {
+ case 4:
+ ret_code = i40iw_fragcnt_to_wqesize_rq(qp->qp_uk.max_rq_frag_cnt,
+ &wqe_size);
+ if (ret_code)
+ return ret_code;
+ break;
+ case 5: /* fallthrough until next ABI version */
+ default:
+ if (qp->qp_uk.max_rq_frag_cnt > I40IW_MAX_WQ_FRAGMENT_COUNT)
+ return I40IW_ERR_INVALID_FRAG_COUNT;
+ wqe_size = I40IW_MAX_WQE_SIZE_RQ;
+ break;
+ }
qp->hw_rq_size = i40iw_get_encoded_wqe_size(qp->qp_uk.rq_size *
(wqe_size / I40IW_QP_WQE_MIN_SIZE), false);
i40iw_debug(qp->dev, I40IW_DEBUG_WQE,
diff --git a/drivers/infiniband/hw/i40iw/i40iw_puda.c b/drivers/infiniband/hw/i40iw/i40iw_puda.c
index 3b286cd..a368b7d 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_puda.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_puda.c
@@ -930,7 +930,7 @@ enum i40iw_status_code i40iw_puda_create_rsrc(struct i40iw_sc_vsi *vsi,
INIT_LIST_HEAD(&rsrc->txpend);
rsrc->tx_wqe_avail_cnt = info->sq_size - 1;
- dev->iw_pd_ops->pd_init(dev, &rsrc->sc_pd, info->pd_id);
+ dev->iw_pd_ops->pd_init(dev, &rsrc->sc_pd, info->pd_id, -1);
rsrc->qp_id = info->qp_id;
rsrc->cq_id = info->cq_id;
rsrc->sq_size = info->sq_size;
diff --git a/drivers/infiniband/hw/i40iw/i40iw_type.h b/drivers/infiniband/hw/i40iw/i40iw_type.h
index 68502ba..d7a629a 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_type.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_type.h
@@ -280,6 +280,7 @@ struct i40iw_sc_pd {
u32 size;
struct i40iw_sc_dev *dev;
u16 pd_id;
+ int abi_ver;
};
struct i40iw_cqp_quanta {
@@ -852,6 +853,7 @@ struct i40iw_qp_init_info {
u64 host_ctx_pa;
u64 q2_pa;
u64 shadow_area_pa;
+ int abi_ver;
u8 sq_tph_val;
u8 rq_tph_val;
u8 type;
@@ -1051,7 +1053,7 @@ struct i40iw_aeq_ops {
};
struct i40iw_pd_ops {
- void (*pd_init)(struct i40iw_sc_dev *, struct i40iw_sc_pd *, u16);
+ void (*pd_init)(struct i40iw_sc_dev *, struct i40iw_sc_pd *, u16, int);
};
struct i40iw_priv_qp_ops {
diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
index 12acd68..57d3f1d 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
@@ -39,8 +39,8 @@
#include <linux/types.h>
-#define I40IW_ABI_USERSPACE_VER 4
-#define I40IW_ABI_KERNEL_VER 4
+#define I40IW_ABI_VER 5
+
struct i40iw_alloc_ucontext_req {
__u32 reserved32;
__u8 userspace_ver;
diff --git a/drivers/infiniband/hw/i40iw/i40iw_uk.c b/drivers/infiniband/hw/i40iw/i40iw_uk.c
index a614ad2..2c6c442 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_uk.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_uk.c
@@ -967,10 +967,6 @@ enum i40iw_status_code i40iw_qp_uk_init(struct i40iw_qp_uk *qp,
if (ret_code)
return ret_code;
- ret_code = i40iw_get_wqe_shift(info->rq_size, info->max_rq_frag_cnt, 0, &rqshift);
- if (ret_code)
- return ret_code;
-
qp->sq_base = info->sq;
qp->rq_base = info->rq;
qp->shadow_area = info->shadow_area;
@@ -999,8 +995,19 @@ enum i40iw_status_code i40iw_qp_uk_init(struct i40iw_qp_uk *qp,
if (!qp->use_srq) {
qp->rq_size = info->rq_size;
qp->max_rq_frag_cnt = info->max_rq_frag_cnt;
- qp->rq_wqe_size = rqshift;
I40IW_RING_INIT(qp->rq_ring, qp->rq_size);
+ switch (info->abi_ver) {
+ case 4:
+ ret_code = i40iw_get_wqe_shift(info->rq_size, info->max_rq_frag_cnt, 0, &rqshift);
+ if (ret_code)
+ return ret_code;
+ break;
+ case 5: /* fallthrough until next ABI version */
+ default:
+ rqshift = I40IW_MAX_RQ_WQE_SHIFT;
+ break;
+ }
+ qp->rq_wqe_size = rqshift;
qp->rq_wqe_size_multiplier = 4 << rqshift;
}
qp->ops = iw_qp_uk_ops;
diff --git a/drivers/infiniband/hw/i40iw/i40iw_user.h b/drivers/infiniband/hw/i40iw/i40iw_user.h
index 66263fc..ab2f7c1 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_user.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_user.h
@@ -76,6 +76,7 @@ enum i40iw_device_capabilities_const {
I40IW_MAX_ORD_SIZE = 127,
I40IW_MAX_WQ_ENTRIES = 2048,
I40IW_Q2_BUFFER_SIZE = (248 + 100),
+ I40IW_MAX_WQE_SIZE_RQ = 128,
I40IW_QP_CTX_SIZE = 248,
I40IW_MAX_PDS = 32768
};
@@ -103,6 +104,7 @@ enum i40iw_device_capabilities_const {
#define I40IW_STAG_INDEX_FROM_STAG(stag) (((stag) && 0xFFFFFF00) >> 8)
#define I40IW_MAX_MR_SIZE 0x10000000000L
+#define I40IW_MAX_RQ_WQE_SHIFT 2
struct i40iw_qp_uk;
struct i40iw_cq_uk;
@@ -411,7 +413,7 @@ struct i40iw_qp_uk_init_info {
u32 max_sq_frag_cnt;
u32 max_rq_frag_cnt;
u32 max_inline_data;
-
+ int abi_ver;
};
struct i40iw_cq_uk_init_info {
diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
index 855e499..7d1ebf0 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
@@ -145,9 +145,8 @@ static struct ib_ucontext *i40iw_alloc_ucontext(struct ib_device *ibdev,
if (ib_copy_from_udata(&req, udata, sizeof(req)))
return ERR_PTR(-EINVAL);
- if (req.userspace_ver != I40IW_ABI_USERSPACE_VER) {
- i40iw_pr_err("Invalid userspace driver version detected. Detected version %d, should be %d\n",
- req.userspace_ver, I40IW_ABI_USERSPACE_VER);
+ if (req.userspace_ver < 4 || req.userspace_ver > I40IW_ABI_VER) {
+ i40iw_pr_err("Unsupported provider library version %u.\n", req.userspace_ver);
return ERR_PTR(-EINVAL);
}
@@ -155,13 +154,14 @@ static struct ib_ucontext *i40iw_alloc_ucontext(struct ib_device *ibdev,
uresp.max_qps = iwdev->max_qp;
uresp.max_pds = iwdev->max_pd;
uresp.wq_size = iwdev->max_qp_wr * 2;
- uresp.kernel_ver = I40IW_ABI_KERNEL_VER;
+ uresp.kernel_ver = req.userspace_ver;
ucontext = kzalloc(sizeof(*ucontext), GFP_KERNEL);
if (!ucontext)
return ERR_PTR(-ENOMEM);
ucontext->iwdev = iwdev;
+ ucontext->abi_ver = req.userspace_ver;
if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) {
kfree(ucontext);
@@ -333,6 +333,7 @@ static struct ib_pd *i40iw_alloc_pd(struct ib_device *ibdev,
struct i40iw_sc_dev *dev = &iwdev->sc_dev;
struct i40iw_alloc_pd_resp uresp;
struct i40iw_sc_pd *sc_pd;
+ struct i40iw_ucontext *ucontext;
u32 pd_id = 0;
int err;
@@ -353,15 +354,18 @@ static struct ib_pd *i40iw_alloc_pd(struct ib_device *ibdev,
}
sc_pd = &iwpd->sc_pd;
- dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id);
if (context) {
+ ucontext = to_ucontext(context);
+ dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, ucontext->abi_ver);
memset(&uresp, 0, sizeof(uresp));
uresp.pd_id = pd_id;
if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) {
err = -EFAULT;
goto error;
}
+ } else {
+ dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, -1);
}
i40iw_add_pdusecount(iwpd);
@@ -518,7 +522,7 @@ static int i40iw_setup_kmode_qp(struct i40iw_device *iwdev,
struct i40iw_dma_mem *mem = &iwqp->kqp.dma_mem;
u32 sqdepth, rqdepth;
u32 sq_size, rq_size;
- u8 sqshift, rqshift;
+ u8 sqshift;
u32 size;
enum i40iw_status_code status;
struct i40iw_qp_uk_init_info *ukinfo = &info->qp_uk_init_info;
@@ -527,14 +531,11 @@ static int i40iw_setup_kmode_qp(struct i40iw_device *iwdev,
rq_size = i40iw_qp_roundup(ukinfo->rq_size + 1);
status = i40iw_get_wqe_shift(sq_size, ukinfo->max_sq_frag_cnt, ukinfo->max_inline_data, &sqshift);
- if (!status)
- status = i40iw_get_wqe_shift(rq_size, ukinfo->max_rq_frag_cnt, 0, &rqshift);
-
if (status)
return -ENOMEM;
sqdepth = sq_size << sqshift;
- rqdepth = rq_size << rqshift;
+ rqdepth = rq_size << I40IW_MAX_RQ_WQE_SHIFT;
size = sqdepth * sizeof(struct i40iw_sq_uk_wr_trk_info) + (rqdepth << 3);
iwqp->kqp.wrid_mem = kzalloc(size, GFP_KERNEL);
diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.h b/drivers/infiniband/hw/i40iw/i40iw_verbs.h
index 6549c93..07c3fec 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.h
@@ -42,6 +42,7 @@ struct i40iw_ucontext {
spinlock_t cq_reg_mem_list_lock; /* memory list for cq's */
struct list_head qp_reg_mem_list;
spinlock_t qp_reg_mem_list_lock; /* memory list for qp's */
+ int abi_ver;
};
struct i40iw_pd {
--
1.8.3.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] 15+ messages in thread
* Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <20161219203227.86392-1-henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2016-12-20 10:47 ` Yuval Shaia
[not found] ` <20161220104720.GD7351-Hxa29pjIrETlQW142y8m19+IiqhCXseY@public.gmane.org>
2016-12-20 11:24 ` Leon Romanovsky
1 sibling, 1 reply; 15+ messages in thread
From: Yuval Shaia @ 2016-12-20 10:47 UTC (permalink / raw)
To: Henry Orosco
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Chien Tin Tung
On Mon, Dec 19, 2016 at 02:32:27PM -0600, Henry Orosco wrote:
> From: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> RQ WQE size other than 128B is not supported. Correct
> RQ size calculation to use 128B only.
>
> Since this breaks AIB, add additional code to
s/AIB/ABI
> provide compatibility with v4 user provider, libi40iw.
>
> Signed-off-by: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/infiniband/hw/i40iw/i40iw_ctrl.c | 25 ++++++++++++++++++++-----
> drivers/infiniband/hw/i40iw/i40iw_puda.c | 2 +-
> drivers/infiniband/hw/i40iw/i40iw_type.h | 4 +++-
> drivers/infiniband/hw/i40iw/i40iw_ucontext.h | 4 ++--
> drivers/infiniband/hw/i40iw/i40iw_uk.c | 17 ++++++++++++-----
> drivers/infiniband/hw/i40iw/i40iw_user.h | 4 +++-
> drivers/infiniband/hw/i40iw/i40iw_verbs.c | 21 +++++++++++----------
> drivers/infiniband/hw/i40iw/i40iw_verbs.h | 1 +
> 8 files changed, 53 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> index 392f783..98923a8 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> @@ -358,13 +358,16 @@ void i40iw_qp_add_qos(struct i40iw_sc_qp *qp)
> * @dev: sc device struct
> * @pd: sc pd ptr
> * @pd_id: pd_id for allocated pd
> + * @abi_ver: ABI version from user context, -1 if not valid
> */
> static void i40iw_sc_pd_init(struct i40iw_sc_dev *dev,
> struct i40iw_sc_pd *pd,
> - u16 pd_id)
> + u16 pd_id,
> + int abi_ver)
No need for new line here
> {
> pd->size = sizeof(*pd);
> pd->pd_id = pd_id;
> + pd->abi_ver = abi_ver;
> pd->dev = dev;
> }
>
> @@ -2252,6 +2255,7 @@ static enum i40iw_status_code i40iw_sc_qp_init(struct i40iw_sc_qp *qp,
> offset);
>
> info->qp_uk_init_info.wqe_alloc_reg = wqe_alloc_reg;
> + info->qp_uk_init_info.abi_ver = qp->pd->abi_ver;
> ret_code = i40iw_qp_uk_init(&qp->qp_uk, &info->qp_uk_init_info);
> if (ret_code)
> return ret_code;
> @@ -2270,10 +2274,21 @@ static enum i40iw_status_code i40iw_sc_qp_init(struct i40iw_sc_qp *qp,
> false);
> i40iw_debug(qp->dev, I40IW_DEBUG_WQE, "%s: hw_sq_size[%04d] sq_ring.size[%04d]\n",
> __func__, qp->hw_sq_size, qp->qp_uk.sq_ring.size);
> - ret_code = i40iw_fragcnt_to_wqesize_rq(qp->qp_uk.max_rq_frag_cnt,
> - &wqe_size);
> - if (ret_code)
> - return ret_code;
> +
> + switch (qp->pd->abi_ver) {
> + case 4:
> + ret_code = i40iw_fragcnt_to_wqesize_rq(qp->qp_uk.max_rq_frag_cnt,
> + &wqe_size);
> + if (ret_code)
> + return ret_code;
> + break;
> + case 5: /* fallthrough until next ABI version */
> + default:
> + if (qp->qp_uk.max_rq_frag_cnt > I40IW_MAX_WQ_FRAGMENT_COUNT)
> + return I40IW_ERR_INVALID_FRAG_COUNT;
> + wqe_size = I40IW_MAX_WQE_SIZE_RQ;
> + break;
> + }
> qp->hw_rq_size = i40iw_get_encoded_wqe_size(qp->qp_uk.rq_size *
> (wqe_size / I40IW_QP_WQE_MIN_SIZE), false);
> i40iw_debug(qp->dev, I40IW_DEBUG_WQE,
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_puda.c b/drivers/infiniband/hw/i40iw/i40iw_puda.c
> index 3b286cd..a368b7d 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_puda.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_puda.c
> @@ -930,7 +930,7 @@ enum i40iw_status_code i40iw_puda_create_rsrc(struct i40iw_sc_vsi *vsi,
> INIT_LIST_HEAD(&rsrc->txpend);
>
> rsrc->tx_wqe_avail_cnt = info->sq_size - 1;
> - dev->iw_pd_ops->pd_init(dev, &rsrc->sc_pd, info->pd_id);
> + dev->iw_pd_ops->pd_init(dev, &rsrc->sc_pd, info->pd_id, -1);
> rsrc->qp_id = info->qp_id;
> rsrc->cq_id = info->cq_id;
> rsrc->sq_size = info->sq_size;
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_type.h b/drivers/infiniband/hw/i40iw/i40iw_type.h
> index 68502ba..d7a629a 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_type.h
> +++ b/drivers/infiniband/hw/i40iw/i40iw_type.h
> @@ -280,6 +280,7 @@ struct i40iw_sc_pd {
> u32 size;
> struct i40iw_sc_dev *dev;
> u16 pd_id;
> + int abi_ver;
> };
>
> struct i40iw_cqp_quanta {
> @@ -852,6 +853,7 @@ struct i40iw_qp_init_info {
> u64 host_ctx_pa;
> u64 q2_pa;
> u64 shadow_area_pa;
> + int abi_ver;
> u8 sq_tph_val;
> u8 rq_tph_val;
> u8 type;
> @@ -1051,7 +1053,7 @@ struct i40iw_aeq_ops {
> };
>
> struct i40iw_pd_ops {
> - void (*pd_init)(struct i40iw_sc_dev *, struct i40iw_sc_pd *, u16);
> + void (*pd_init)(struct i40iw_sc_dev *, struct i40iw_sc_pd *, u16, int);
> };
>
> struct i40iw_priv_qp_ops {
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> index 12acd68..57d3f1d 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> @@ -39,8 +39,8 @@
>
> #include <linux/types.h>
>
> -#define I40IW_ABI_USERSPACE_VER 4
> -#define I40IW_ABI_KERNEL_VER 4
> +#define I40IW_ABI_VER 5
> +
> struct i40iw_alloc_ucontext_req {
> __u32 reserved32;
> __u8 userspace_ver;
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_uk.c b/drivers/infiniband/hw/i40iw/i40iw_uk.c
> index a614ad2..2c6c442 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_uk.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_uk.c
> @@ -967,10 +967,6 @@ enum i40iw_status_code i40iw_qp_uk_init(struct i40iw_qp_uk *qp,
> if (ret_code)
> return ret_code;
>
> - ret_code = i40iw_get_wqe_shift(info->rq_size, info->max_rq_frag_cnt, 0, &rqshift);
> - if (ret_code)
> - return ret_code;
> -
> qp->sq_base = info->sq;
> qp->rq_base = info->rq;
> qp->shadow_area = info->shadow_area;
> @@ -999,8 +995,19 @@ enum i40iw_status_code i40iw_qp_uk_init(struct i40iw_qp_uk *qp,
> if (!qp->use_srq) {
> qp->rq_size = info->rq_size;
> qp->max_rq_frag_cnt = info->max_rq_frag_cnt;
> - qp->rq_wqe_size = rqshift;
> I40IW_RING_INIT(qp->rq_ring, qp->rq_size);
> + switch (info->abi_ver) {
> + case 4:
> + ret_code = i40iw_get_wqe_shift(info->rq_size, info->max_rq_frag_cnt, 0, &rqshift);
> + if (ret_code)
> + return ret_code;
> + break;
> + case 5: /* fallthrough until next ABI version */
> + default:
> + rqshift = I40IW_MAX_RQ_WQE_SHIFT;
> + break;
> + }
> + qp->rq_wqe_size = rqshift;
> qp->rq_wqe_size_multiplier = 4 << rqshift;
> }
> qp->ops = iw_qp_uk_ops;
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_user.h b/drivers/infiniband/hw/i40iw/i40iw_user.h
> index 66263fc..ab2f7c1 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_user.h
> +++ b/drivers/infiniband/hw/i40iw/i40iw_user.h
> @@ -76,6 +76,7 @@ enum i40iw_device_capabilities_const {
> I40IW_MAX_ORD_SIZE = 127,
> I40IW_MAX_WQ_ENTRIES = 2048,
> I40IW_Q2_BUFFER_SIZE = (248 + 100),
> + I40IW_MAX_WQE_SIZE_RQ = 128,
> I40IW_QP_CTX_SIZE = 248,
> I40IW_MAX_PDS = 32768
> };
> @@ -103,6 +104,7 @@ enum i40iw_device_capabilities_const {
> #define I40IW_STAG_INDEX_FROM_STAG(stag) (((stag) && 0xFFFFFF00) >> 8)
>
> #define I40IW_MAX_MR_SIZE 0x10000000000L
> +#define I40IW_MAX_RQ_WQE_SHIFT 2
>
> struct i40iw_qp_uk;
> struct i40iw_cq_uk;
> @@ -411,7 +413,7 @@ struct i40iw_qp_uk_init_info {
> u32 max_sq_frag_cnt;
> u32 max_rq_frag_cnt;
> u32 max_inline_data;
> -
> + int abi_ver;
> };
>
> struct i40iw_cq_uk_init_info {
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> index 855e499..7d1ebf0 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> @@ -145,9 +145,8 @@ static struct ib_ucontext *i40iw_alloc_ucontext(struct ib_device *ibdev,
> if (ib_copy_from_udata(&req, udata, sizeof(req)))
> return ERR_PTR(-EINVAL);
>
> - if (req.userspace_ver != I40IW_ABI_USERSPACE_VER) {
> - i40iw_pr_err("Invalid userspace driver version detected. Detected version %d, should be %d\n",
> - req.userspace_ver, I40IW_ABI_USERSPACE_VER);
> + if (req.userspace_ver < 4 || req.userspace_ver > I40IW_ABI_VER) {
> + i40iw_pr_err("Unsupported provider library version %u.\n", req.userspace_ver);
> return ERR_PTR(-EINVAL);
> }
>
> @@ -155,13 +154,14 @@ static struct ib_ucontext *i40iw_alloc_ucontext(struct ib_device *ibdev,
> uresp.max_qps = iwdev->max_qp;
> uresp.max_pds = iwdev->max_pd;
> uresp.wq_size = iwdev->max_qp_wr * 2;
> - uresp.kernel_ver = I40IW_ABI_KERNEL_VER;
> + uresp.kernel_ver = req.userspace_ver;
>
> ucontext = kzalloc(sizeof(*ucontext), GFP_KERNEL);
> if (!ucontext)
> return ERR_PTR(-ENOMEM);
>
> ucontext->iwdev = iwdev;
> + ucontext->abi_ver = req.userspace_ver;
>
> if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) {
> kfree(ucontext);
> @@ -333,6 +333,7 @@ static struct ib_pd *i40iw_alloc_pd(struct ib_device *ibdev,
> struct i40iw_sc_dev *dev = &iwdev->sc_dev;
> struct i40iw_alloc_pd_resp uresp;
> struct i40iw_sc_pd *sc_pd;
> + struct i40iw_ucontext *ucontext;
> u32 pd_id = 0;
> int err;
>
> @@ -353,15 +354,18 @@ static struct ib_pd *i40iw_alloc_pd(struct ib_device *ibdev,
> }
>
> sc_pd = &iwpd->sc_pd;
> - dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id);
>
> if (context) {
> + ucontext = to_ucontext(context);
> + dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, ucontext->abi_ver);
> memset(&uresp, 0, sizeof(uresp));
> uresp.pd_id = pd_id;
> if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) {
> err = -EFAULT;
> goto error;
> }
> + } else {
> + dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, -1);
> }
>
> i40iw_add_pdusecount(iwpd);
> @@ -518,7 +522,7 @@ static int i40iw_setup_kmode_qp(struct i40iw_device *iwdev,
> struct i40iw_dma_mem *mem = &iwqp->kqp.dma_mem;
> u32 sqdepth, rqdepth;
> u32 sq_size, rq_size;
> - u8 sqshift, rqshift;
> + u8 sqshift;
> u32 size;
> enum i40iw_status_code status;
> struct i40iw_qp_uk_init_info *ukinfo = &info->qp_uk_init_info;
> @@ -527,14 +531,11 @@ static int i40iw_setup_kmode_qp(struct i40iw_device *iwdev,
> rq_size = i40iw_qp_roundup(ukinfo->rq_size + 1);
>
> status = i40iw_get_wqe_shift(sq_size, ukinfo->max_sq_frag_cnt, ukinfo->max_inline_data, &sqshift);
> - if (!status)
> - status = i40iw_get_wqe_shift(rq_size, ukinfo->max_rq_frag_cnt, 0, &rqshift);
> -
> if (status)
> return -ENOMEM;
>
> sqdepth = sq_size << sqshift;
> - rqdepth = rq_size << rqshift;
> + rqdepth = rq_size << I40IW_MAX_RQ_WQE_SHIFT;
>
> size = sqdepth * sizeof(struct i40iw_sq_uk_wr_trk_info) + (rqdepth << 3);
> iwqp->kqp.wrid_mem = kzalloc(size, GFP_KERNEL);
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.h b/drivers/infiniband/hw/i40iw/i40iw_verbs.h
> index 6549c93..07c3fec 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.h
> +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.h
> @@ -42,6 +42,7 @@ struct i40iw_ucontext {
> spinlock_t cq_reg_mem_list_lock; /* memory list for cq's */
> struct list_head qp_reg_mem_list;
> spinlock_t qp_reg_mem_list_lock; /* memory list for qp's */
> + int abi_ver;
> };
>
> struct i40iw_pd {
> --
> 1.8.3.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] 15+ messages in thread
* Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <20161219203227.86392-1-henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-12-20 10:47 ` Yuval Shaia
@ 2016-12-20 11:24 ` Leon Romanovsky
[not found] ` <20161220112417.GV1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
1 sibling, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2016-12-20 11:24 UTC (permalink / raw)
To: Henry Orosco
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Chien Tin Tung
[-- Attachment #1: Type: text/plain, Size: 11343 bytes --]
On Mon, Dec 19, 2016 at 02:32:27PM -0600, Henry Orosco wrote:
> From: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> RQ WQE size other than 128B is not supported. Correct
> RQ size calculation to use 128B only.
>
> Since this breaks AIB, add additional code to
> provide compatibility with v4 user provider, libi40iw.
>
> Signed-off-by: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/infiniband/hw/i40iw/i40iw_ctrl.c | 25 ++++++++++++++++++++-----
> drivers/infiniband/hw/i40iw/i40iw_puda.c | 2 +-
> drivers/infiniband/hw/i40iw/i40iw_type.h | 4 +++-
> drivers/infiniband/hw/i40iw/i40iw_ucontext.h | 4 ++--
> drivers/infiniband/hw/i40iw/i40iw_uk.c | 17 ++++++++++++-----
> drivers/infiniband/hw/i40iw/i40iw_user.h | 4 +++-
> drivers/infiniband/hw/i40iw/i40iw_verbs.c | 21 +++++++++++----------
> drivers/infiniband/hw/i40iw/i40iw_verbs.h | 1 +
> 8 files changed, 53 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> index 392f783..98923a8 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> @@ -358,13 +358,16 @@ void i40iw_qp_add_qos(struct i40iw_sc_qp *qp)
> * @dev: sc device struct
> * @pd: sc pd ptr
> * @pd_id: pd_id for allocated pd
> + * @abi_ver: ABI version from user context, -1 if not valid
> */
> static void i40iw_sc_pd_init(struct i40iw_sc_dev *dev,
> struct i40iw_sc_pd *pd,
> - u16 pd_id)
> + u16 pd_id,
> + int abi_ver)
> {
> pd->size = sizeof(*pd);
> pd->pd_id = pd_id;
> + pd->abi_ver = abi_ver;
> pd->dev = dev;
> }
>
> @@ -2252,6 +2255,7 @@ static enum i40iw_status_code i40iw_sc_qp_init(struct i40iw_sc_qp *qp,
> offset);
>
> info->qp_uk_init_info.wqe_alloc_reg = wqe_alloc_reg;
> + info->qp_uk_init_info.abi_ver = qp->pd->abi_ver;
> ret_code = i40iw_qp_uk_init(&qp->qp_uk, &info->qp_uk_init_info);
> if (ret_code)
> return ret_code;
> @@ -2270,10 +2274,21 @@ static enum i40iw_status_code i40iw_sc_qp_init(struct i40iw_sc_qp *qp,
> false);
> i40iw_debug(qp->dev, I40IW_DEBUG_WQE, "%s: hw_sq_size[%04d] sq_ring.size[%04d]\n",
> __func__, qp->hw_sq_size, qp->qp_uk.sq_ring.size);
> - ret_code = i40iw_fragcnt_to_wqesize_rq(qp->qp_uk.max_rq_frag_cnt,
> - &wqe_size);
> - if (ret_code)
> - return ret_code;
> +
> + switch (qp->pd->abi_ver) {
> + case 4:
> + ret_code = i40iw_fragcnt_to_wqesize_rq(qp->qp_uk.max_rq_frag_cnt,
> + &wqe_size);
> + if (ret_code)
> + return ret_code;
> + break;
> + case 5: /* fallthrough until next ABI version */
> + default:
> + if (qp->qp_uk.max_rq_frag_cnt > I40IW_MAX_WQ_FRAGMENT_COUNT)
> + return I40IW_ERR_INVALID_FRAG_COUNT;
> + wqe_size = I40IW_MAX_WQE_SIZE_RQ;
> + break;
> + }
> qp->hw_rq_size = i40iw_get_encoded_wqe_size(qp->qp_uk.rq_size *
> (wqe_size / I40IW_QP_WQE_MIN_SIZE), false);
> i40iw_debug(qp->dev, I40IW_DEBUG_WQE,
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_puda.c b/drivers/infiniband/hw/i40iw/i40iw_puda.c
> index 3b286cd..a368b7d 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_puda.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_puda.c
> @@ -930,7 +930,7 @@ enum i40iw_status_code i40iw_puda_create_rsrc(struct i40iw_sc_vsi *vsi,
> INIT_LIST_HEAD(&rsrc->txpend);
>
> rsrc->tx_wqe_avail_cnt = info->sq_size - 1;
> - dev->iw_pd_ops->pd_init(dev, &rsrc->sc_pd, info->pd_id);
> + dev->iw_pd_ops->pd_init(dev, &rsrc->sc_pd, info->pd_id, -1);
> rsrc->qp_id = info->qp_id;
> rsrc->cq_id = info->cq_id;
> rsrc->sq_size = info->sq_size;
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_type.h b/drivers/infiniband/hw/i40iw/i40iw_type.h
> index 68502ba..d7a629a 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_type.h
> +++ b/drivers/infiniband/hw/i40iw/i40iw_type.h
> @@ -280,6 +280,7 @@ struct i40iw_sc_pd {
> u32 size;
> struct i40iw_sc_dev *dev;
> u16 pd_id;
> + int abi_ver;
> };
>
> struct i40iw_cqp_quanta {
> @@ -852,6 +853,7 @@ struct i40iw_qp_init_info {
> u64 host_ctx_pa;
> u64 q2_pa;
> u64 shadow_area_pa;
> + int abi_ver;
> u8 sq_tph_val;
> u8 rq_tph_val;
> u8 type;
> @@ -1051,7 +1053,7 @@ struct i40iw_aeq_ops {
> };
>
> struct i40iw_pd_ops {
> - void (*pd_init)(struct i40iw_sc_dev *, struct i40iw_sc_pd *, u16);
> + void (*pd_init)(struct i40iw_sc_dev *, struct i40iw_sc_pd *, u16, int);
> };
>
> struct i40iw_priv_qp_ops {
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> index 12acd68..57d3f1d 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> @@ -39,8 +39,8 @@
>
> #include <linux/types.h>
>
> -#define I40IW_ABI_USERSPACE_VER 4
> -#define I40IW_ABI_KERNEL_VER 4
> +#define I40IW_ABI_VER 5
> +
Why did you remove defines and move to use constants "4" and "5" instead?
> struct i40iw_alloc_ucontext_req {
> __u32 reserved32;
> __u8 userspace_ver;
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_uk.c b/drivers/infiniband/hw/i40iw/i40iw_uk.c
> index a614ad2..2c6c442 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_uk.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_uk.c
> @@ -967,10 +967,6 @@ enum i40iw_status_code i40iw_qp_uk_init(struct i40iw_qp_uk *qp,
> if (ret_code)
> return ret_code;
>
> - ret_code = i40iw_get_wqe_shift(info->rq_size, info->max_rq_frag_cnt, 0, &rqshift);
> - if (ret_code)
> - return ret_code;
> -
> qp->sq_base = info->sq;
> qp->rq_base = info->rq;
> qp->shadow_area = info->shadow_area;
> @@ -999,8 +995,19 @@ enum i40iw_status_code i40iw_qp_uk_init(struct i40iw_qp_uk *qp,
> if (!qp->use_srq) {
> qp->rq_size = info->rq_size;
> qp->max_rq_frag_cnt = info->max_rq_frag_cnt;
> - qp->rq_wqe_size = rqshift;
> I40IW_RING_INIT(qp->rq_ring, qp->rq_size);
> + switch (info->abi_ver) {
> + case 4:
> + ret_code = i40iw_get_wqe_shift(info->rq_size, info->max_rq_frag_cnt, 0, &rqshift);
> + if (ret_code)
> + return ret_code;
> + break;
> + case 5: /* fallthrough until next ABI version */
> + default:
> + rqshift = I40IW_MAX_RQ_WQE_SHIFT;
> + break;
> + }
> + qp->rq_wqe_size = rqshift;
> qp->rq_wqe_size_multiplier = 4 << rqshift;
> }
> qp->ops = iw_qp_uk_ops;
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_user.h b/drivers/infiniband/hw/i40iw/i40iw_user.h
> index 66263fc..ab2f7c1 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_user.h
> +++ b/drivers/infiniband/hw/i40iw/i40iw_user.h
> @@ -76,6 +76,7 @@ enum i40iw_device_capabilities_const {
> I40IW_MAX_ORD_SIZE = 127,
> I40IW_MAX_WQ_ENTRIES = 2048,
> I40IW_Q2_BUFFER_SIZE = (248 + 100),
> + I40IW_MAX_WQE_SIZE_RQ = 128,
> I40IW_QP_CTX_SIZE = 248,
> I40IW_MAX_PDS = 32768
> };
> @@ -103,6 +104,7 @@ enum i40iw_device_capabilities_const {
> #define I40IW_STAG_INDEX_FROM_STAG(stag) (((stag) && 0xFFFFFF00) >> 8)
>
> #define I40IW_MAX_MR_SIZE 0x10000000000L
> +#define I40IW_MAX_RQ_WQE_SHIFT 2
>
> struct i40iw_qp_uk;
> struct i40iw_cq_uk;
> @@ -411,7 +413,7 @@ struct i40iw_qp_uk_init_info {
> u32 max_sq_frag_cnt;
> u32 max_rq_frag_cnt;
> u32 max_inline_data;
> -
> + int abi_ver;
> };
>
> struct i40iw_cq_uk_init_info {
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> index 855e499..7d1ebf0 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> @@ -145,9 +145,8 @@ static struct ib_ucontext *i40iw_alloc_ucontext(struct ib_device *ibdev,
> if (ib_copy_from_udata(&req, udata, sizeof(req)))
> return ERR_PTR(-EINVAL);
>
> - if (req.userspace_ver != I40IW_ABI_USERSPACE_VER) {
> - i40iw_pr_err("Invalid userspace driver version detected. Detected version %d, should be %d\n",
> - req.userspace_ver, I40IW_ABI_USERSPACE_VER);
> + if (req.userspace_ver < 4 || req.userspace_ver > I40IW_ABI_VER) {
> + i40iw_pr_err("Unsupported provider library version %u.\n", req.userspace_ver);
> return ERR_PTR(-EINVAL);
> }
>
> @@ -155,13 +154,14 @@ static struct ib_ucontext *i40iw_alloc_ucontext(struct ib_device *ibdev,
> uresp.max_qps = iwdev->max_qp;
> uresp.max_pds = iwdev->max_pd;
> uresp.wq_size = iwdev->max_qp_wr * 2;
> - uresp.kernel_ver = I40IW_ABI_KERNEL_VER;
> + uresp.kernel_ver = req.userspace_ver;
>
> ucontext = kzalloc(sizeof(*ucontext), GFP_KERNEL);
> if (!ucontext)
> return ERR_PTR(-ENOMEM);
>
> ucontext->iwdev = iwdev;
> + ucontext->abi_ver = req.userspace_ver;
>
> if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) {
> kfree(ucontext);
> @@ -333,6 +333,7 @@ static struct ib_pd *i40iw_alloc_pd(struct ib_device *ibdev,
> struct i40iw_sc_dev *dev = &iwdev->sc_dev;
> struct i40iw_alloc_pd_resp uresp;
> struct i40iw_sc_pd *sc_pd;
> + struct i40iw_ucontext *ucontext;
> u32 pd_id = 0;
> int err;
>
> @@ -353,15 +354,18 @@ static struct ib_pd *i40iw_alloc_pd(struct ib_device *ibdev,
> }
>
> sc_pd = &iwpd->sc_pd;
> - dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id);
>
> if (context) {
> + ucontext = to_ucontext(context);
> + dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, ucontext->abi_ver);
> memset(&uresp, 0, sizeof(uresp));
> uresp.pd_id = pd_id;
> if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) {
> err = -EFAULT;
> goto error;
> }
> + } else {
> + dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, -1);
> }
>
> i40iw_add_pdusecount(iwpd);
> @@ -518,7 +522,7 @@ static int i40iw_setup_kmode_qp(struct i40iw_device *iwdev,
> struct i40iw_dma_mem *mem = &iwqp->kqp.dma_mem;
> u32 sqdepth, rqdepth;
> u32 sq_size, rq_size;
> - u8 sqshift, rqshift;
> + u8 sqshift;
> u32 size;
> enum i40iw_status_code status;
> struct i40iw_qp_uk_init_info *ukinfo = &info->qp_uk_init_info;
> @@ -527,14 +531,11 @@ static int i40iw_setup_kmode_qp(struct i40iw_device *iwdev,
> rq_size = i40iw_qp_roundup(ukinfo->rq_size + 1);
>
> status = i40iw_get_wqe_shift(sq_size, ukinfo->max_sq_frag_cnt, ukinfo->max_inline_data, &sqshift);
> - if (!status)
> - status = i40iw_get_wqe_shift(rq_size, ukinfo->max_rq_frag_cnt, 0, &rqshift);
> -
> if (status)
> return -ENOMEM;
>
> sqdepth = sq_size << sqshift;
> - rqdepth = rq_size << rqshift;
> + rqdepth = rq_size << I40IW_MAX_RQ_WQE_SHIFT;
>
> size = sqdepth * sizeof(struct i40iw_sq_uk_wr_trk_info) + (rqdepth << 3);
> iwqp->kqp.wrid_mem = kzalloc(size, GFP_KERNEL);
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.h b/drivers/infiniband/hw/i40iw/i40iw_verbs.h
> index 6549c93..07c3fec 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.h
> +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.h
> @@ -42,6 +42,7 @@ struct i40iw_ucontext {
> spinlock_t cq_reg_mem_list_lock; /* memory list for cq's */
> struct list_head qp_reg_mem_list;
> spinlock_t qp_reg_mem_list_lock; /* memory list for qp's */
> + int abi_ver;
> };
>
> struct i40iw_pd {
> --
> 1.8.3.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] 15+ messages in thread
* RE: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <20161220112417.GV1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2016-12-20 12:44 ` Tung, Chien Tin
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBAEEA0-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Tung, Chien Tin @ 2016-12-20 12:44 UTC (permalink / raw)
To: Leon Romanovsky, Orosco, Henry
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
> -----Original Message-----
> From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Leon Romanovsky
> Sent: Tuesday, December 20, 2016 5:24 AM
> To: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-
> rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
>
> > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > index 12acd68..57d3f1d 100644
> > --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > @@ -39,8 +39,8 @@
> >
> > #include <linux/types.h>
> >
> > -#define I40IW_ABI_USERSPACE_VER 4
> > -#define I40IW_ABI_KERNEL_VER 4
> > +#define I40IW_ABI_VER 5
> > +
>
> Why did you remove defines and move to use constants "4" and "5" instead?
[Chien Tin Tung] This is the ABI version change, did you read the commit message? Two defines were not necessary.
--
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] 15+ messages in thread
* Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBAEEA0-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
@ 2016-12-20 13:42 ` Leon Romanovsky
[not found] ` <20161220134203.GW1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2016-12-20 13:42 UTC (permalink / raw)
To: Tung, Chien Tin
Cc: Orosco, Henry, dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
[-- Attachment #1: Type: text/plain, Size: 1647 bytes --]
On Tue, Dec 20, 2016 at 12:44:04PM +0000, Tung, Chien Tin wrote:
>
>
> > -----Original Message-----
> > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> > owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Leon Romanovsky
> > Sent: Tuesday, December 20, 2016 5:24 AM
> > To: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-
> > rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
> >
> > > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > index 12acd68..57d3f1d 100644
> > > --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > @@ -39,8 +39,8 @@
> > >
> > > #include <linux/types.h>
> > >
> > > -#define I40IW_ABI_USERSPACE_VER 4
> > > -#define I40IW_ABI_KERNEL_VER 4
> > > +#define I40IW_ABI_VER 5
> > > +
> >
> > Why did you remove defines and move to use constants "4" and "5" instead?
> [Chien Tin Tung] This is the ABI version change, did you read the commit message? Two defines were not necessary.
>
Thank you Chian Tin Tung for your informative answer and yes, I read commit message.
If you think that these defines are not needed, so can you please remove
the code which uses hardcoded "case 4" and "case 5"?
Or maybe they still needed and should be renamed?
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <20161220134203.GW1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2016-12-20 14:46 ` Tung, Chien Tin
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBAFBBC-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Tung, Chien Tin @ 2016-12-20 14:46 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Orosco, Henry, dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
> -----Original Message-----
> From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> Sent: Tuesday, December 20, 2016 7:42 AM
> To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> linux- rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
>
> On Tue, Dec 20, 2016 at 12:44:04PM +0000, Tung, Chien Tin wrote:
> >
> >
> > > -----Original Message-----
> > > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> > > owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Leon Romanovsky
> > > Sent: Tuesday, December 20, 2016 5:24 AM
> > > To: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-
> > > rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Tung, Chien Tin
> > > <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE
> > > size
> > >
> > > > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > index 12acd68..57d3f1d 100644
> > > > --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > @@ -39,8 +39,8 @@
> > > >
> > > > #include <linux/types.h>
> > > >
> > > > -#define I40IW_ABI_USERSPACE_VER 4
> > > > -#define I40IW_ABI_KERNEL_VER 4
> > > > +#define I40IW_ABI_VER 5
> > > > +
> > >
> > > Why did you remove defines and move to use constants "4" and "5"
> instead?
> > [Chien Tin Tung] This is the ABI version change, did you read the
> > commit
> message? Two defines were not necessary.
> >
>
> Thank you Chian Tin Tung for your informative answer and yes, I read
> commit message.
>
> If you think that these defines are not needed, so can you please
> remove the code which uses hardcoded "case 4" and "case 5"?
[Chien Tin Tung] Those are explicit checks against a particular ABI
version and it is clearer to use the numbers rather than some define.
>
> Or maybe they still needed and should be renamed?
[Chien Tin Tung] They are not needed. Before we were checking against
one version and one version only. Now we are making the provider library
and driver backward compatible. All we need is one define to specify current
ABI version.
Chien
--
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] 15+ messages in thread
* Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <20161220104720.GD7351-Hxa29pjIrETlQW142y8m19+IiqhCXseY@public.gmane.org>
@ 2016-12-20 16:41 ` Henry Orosco
[not found] ` <20161220164118.GA91636-ZmvEvTIhtuUfyugFOqMDN/ooFf0ArEBIu+b9c/7xato@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Henry Orosco @ 2016-12-20 16:41 UTC (permalink / raw)
To: Yuval Shaia
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Tung, Chien Tin
On Tue, Dec 20, 2016 at 02:47:21AM -0800, Yuval Shaia wrote:
> On Mon, Dec 19, 2016 at 02:32:27PM -0600, Henry Orosco wrote:
> > From: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >
> > RQ WQE size other than 128B is not supported. Correct
> > RQ size calculation to use 128B only.
> >
> > Since this breaks AIB, add additional code to
>
> s/AIB/ABI
I will make this change.
>
> > provide compatibility with v4 user provider, libi40iw.
> >
> > Signed-off-by: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> > drivers/infiniband/hw/i40iw/i40iw_ctrl.c | 25 ++++++++++++++++++++-----
> > drivers/infiniband/hw/i40iw/i40iw_puda.c | 2 +-
> > drivers/infiniband/hw/i40iw/i40iw_type.h | 4 +++-
> > drivers/infiniband/hw/i40iw/i40iw_ucontext.h | 4 ++--
> > drivers/infiniband/hw/i40iw/i40iw_uk.c | 17 ++++++++++++-----
> > drivers/infiniband/hw/i40iw/i40iw_user.h | 4 +++-
> > drivers/infiniband/hw/i40iw/i40iw_verbs.c | 21 +++++++++++----------
> > drivers/infiniband/hw/i40iw/i40iw_verbs.h | 1 +
> > 8 files changed, 53 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> > index 392f783..98923a8 100644
> > --- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> > +++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> > @@ -358,13 +358,16 @@ void i40iw_qp_add_qos(struct i40iw_sc_qp *qp)
> > * @dev: sc device struct
> > * @pd: sc pd ptr
> > * @pd_id: pd_id for allocated pd
> > + * @abi_ver: ABI version from user context, -1 if not valid
> > */
> > static void i40iw_sc_pd_init(struct i40iw_sc_dev *dev,
> > struct i40iw_sc_pd *pd,
> > - u16 pd_id)
> > + u16 pd_id,
> > + int abi_ver)
>
> No need for new line here
>
Not sure what you mean here.
> > {
> > pd->size = sizeof(*pd);
> > pd->pd_id = pd_id;
> > + pd->abi_ver = abi_ver;
> > pd->dev = dev;
> > }
> >
> > @@ -2252,6 +2255,7 @@ static enum i40iw_status_code i40iw_sc_qp_init(struct i40iw_sc_qp *qp,
> > offset);
> >
> > info->qp_uk_init_info.wqe_alloc_reg = wqe_alloc_reg;
> > + info->qp_uk_init_info.abi_ver = qp->pd->abi_ver;
> > ret_code = i40iw_qp_uk_init(&qp->qp_uk, &info->qp_uk_init_info);
> > if (ret_code)
> > return ret_code;
> > @@ -2270,10 +2274,21 @@ static enum i40iw_status_code i40iw_sc_qp_init(struct i40iw_sc_qp *qp,
> > false);
> > i40iw_debug(qp->dev, I40IW_DEBUG_WQE, "%s: hw_sq_size[%04d] sq_ring.size[%04d]\n",
> > __func__, qp->hw_sq_size, qp->qp_uk.sq_ring.size);
> > - ret_code = i40iw_fragcnt_to_wqesize_rq(qp->qp_uk.max_rq_frag_cnt,
> > - &wqe_size);
> > - if (ret_code)
> > - return ret_code;
> > +
> > + switch (qp->pd->abi_ver) {
> > + case 4:
> > + ret_code = i40iw_fragcnt_to_wqesize_rq(qp->qp_uk.max_rq_frag_cnt,
> > + &wqe_size);
> > + if (ret_code)
> > + return ret_code;
> > + break;
> > + case 5: /* fallthrough until next ABI version */
> > + default:
> > + if (qp->qp_uk.max_rq_frag_cnt > I40IW_MAX_WQ_FRAGMENT_COUNT)
> > + return I40IW_ERR_INVALID_FRAG_COUNT;
> > + wqe_size = I40IW_MAX_WQE_SIZE_RQ;
> > + break;
> > + }
> > qp->hw_rq_size = i40iw_get_encoded_wqe_size(qp->qp_uk.rq_size *
> > (wqe_size / I40IW_QP_WQE_MIN_SIZE), false);
> > i40iw_debug(qp->dev, I40IW_DEBUG_WQE,
> > diff --git a/drivers/infiniband/hw/i40iw/i40iw_puda.c b/drivers/infiniband/hw/i40iw/i40iw_puda.c
> > index 3b286cd..a368b7d 100644
> > --- a/drivers/infiniband/hw/i40iw/i40iw_puda.c
> > +++ b/drivers/infiniband/hw/i40iw/i40iw_puda.c
> > @@ -930,7 +930,7 @@ enum i40iw_status_code i40iw_puda_create_rsrc(struct i40iw_sc_vsi *vsi,
> > INIT_LIST_HEAD(&rsrc->txpend);
> >
> > rsrc->tx_wqe_avail_cnt = info->sq_size - 1;
> > - dev->iw_pd_ops->pd_init(dev, &rsrc->sc_pd, info->pd_id);
> > + dev->iw_pd_ops->pd_init(dev, &rsrc->sc_pd, info->pd_id, -1);
> > rsrc->qp_id = info->qp_id;
> > rsrc->cq_id = info->cq_id;
> > rsrc->sq_size = info->sq_size;
> > diff --git a/drivers/infiniband/hw/i40iw/i40iw_type.h b/drivers/infiniband/hw/i40iw/i40iw_type.h
> > index 68502ba..d7a629a 100644
> > --- a/drivers/infiniband/hw/i40iw/i40iw_type.h
> > +++ b/drivers/infiniband/hw/i40iw/i40iw_type.h
> > @@ -280,6 +280,7 @@ struct i40iw_sc_pd {
> > u32 size;
> > struct i40iw_sc_dev *dev;
> > u16 pd_id;
> > + int abi_ver;
> > };
> >
> > struct i40iw_cqp_quanta {
> > @@ -852,6 +853,7 @@ struct i40iw_qp_init_info {
> > u64 host_ctx_pa;
> > u64 q2_pa;
> > u64 shadow_area_pa;
> > + int abi_ver;
> > u8 sq_tph_val;
> > u8 rq_tph_val;
> > u8 type;
> > @@ -1051,7 +1053,7 @@ struct i40iw_aeq_ops {
> > };
> >
> > struct i40iw_pd_ops {
> > - void (*pd_init)(struct i40iw_sc_dev *, struct i40iw_sc_pd *, u16);
> > + void (*pd_init)(struct i40iw_sc_dev *, struct i40iw_sc_pd *, u16, int);
> > };
> >
> > struct i40iw_priv_qp_ops {
> > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > index 12acd68..57d3f1d 100644
> > --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > @@ -39,8 +39,8 @@
> >
> > #include <linux/types.h>
> >
> > -#define I40IW_ABI_USERSPACE_VER 4
> > -#define I40IW_ABI_KERNEL_VER 4
> > +#define I40IW_ABI_VER 5
> > +
> > struct i40iw_alloc_ucontext_req {
> > __u32 reserved32;
> > __u8 userspace_ver;
> > diff --git a/drivers/infiniband/hw/i40iw/i40iw_uk.c b/drivers/infiniband/hw/i40iw/i40iw_uk.c
> > index a614ad2..2c6c442 100644
> > --- a/drivers/infiniband/hw/i40iw/i40iw_uk.c
> > +++ b/drivers/infiniband/hw/i40iw/i40iw_uk.c
> > @@ -967,10 +967,6 @@ enum i40iw_status_code i40iw_qp_uk_init(struct i40iw_qp_uk *qp,
> > if (ret_code)
> > return ret_code;
> >
> > - ret_code = i40iw_get_wqe_shift(info->rq_size, info->max_rq_frag_cnt, 0, &rqshift);
> > - if (ret_code)
> > - return ret_code;
> > -
> > qp->sq_base = info->sq;
> > qp->rq_base = info->rq;
> > qp->shadow_area = info->shadow_area;
> > @@ -999,8 +995,19 @@ enum i40iw_status_code i40iw_qp_uk_init(struct i40iw_qp_uk *qp,
> > if (!qp->use_srq) {
> > qp->rq_size = info->rq_size;
> > qp->max_rq_frag_cnt = info->max_rq_frag_cnt;
> > - qp->rq_wqe_size = rqshift;
> > I40IW_RING_INIT(qp->rq_ring, qp->rq_size);
> > + switch (info->abi_ver) {
> > + case 4:
> > + ret_code = i40iw_get_wqe_shift(info->rq_size, info->max_rq_frag_cnt, 0, &rqshift);
> > + if (ret_code)
> > + return ret_code;
> > + break;
> > + case 5: /* fallthrough until next ABI version */
> > + default:
> > + rqshift = I40IW_MAX_RQ_WQE_SHIFT;
> > + break;
> > + }
> > + qp->rq_wqe_size = rqshift;
> > qp->rq_wqe_size_multiplier = 4 << rqshift;
> > }
> > qp->ops = iw_qp_uk_ops;
> > diff --git a/drivers/infiniband/hw/i40iw/i40iw_user.h b/drivers/infiniband/hw/i40iw/i40iw_user.h
> > index 66263fc..ab2f7c1 100644
> > --- a/drivers/infiniband/hw/i40iw/i40iw_user.h
> > +++ b/drivers/infiniband/hw/i40iw/i40iw_user.h
> > @@ -76,6 +76,7 @@ enum i40iw_device_capabilities_const {
> > I40IW_MAX_ORD_SIZE = 127,
> > I40IW_MAX_WQ_ENTRIES = 2048,
> > I40IW_Q2_BUFFER_SIZE = (248 + 100),
> > + I40IW_MAX_WQE_SIZE_RQ = 128,
> > I40IW_QP_CTX_SIZE = 248,
> > I40IW_MAX_PDS = 32768
> > };
> > @@ -103,6 +104,7 @@ enum i40iw_device_capabilities_const {
> > #define I40IW_STAG_INDEX_FROM_STAG(stag) (((stag) && 0xFFFFFF00) >> 8)
> >
> > #define I40IW_MAX_MR_SIZE 0x10000000000L
> > +#define I40IW_MAX_RQ_WQE_SHIFT 2
> >
> > struct i40iw_qp_uk;
> > struct i40iw_cq_uk;
> > @@ -411,7 +413,7 @@ struct i40iw_qp_uk_init_info {
> > u32 max_sq_frag_cnt;
> > u32 max_rq_frag_cnt;
> > u32 max_inline_data;
> > -
> > + int abi_ver;
> > };
> >
> > struct i40iw_cq_uk_init_info {
> > diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> > index 855e499..7d1ebf0 100644
> > --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> > +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> > @@ -145,9 +145,8 @@ static struct ib_ucontext *i40iw_alloc_ucontext(struct ib_device *ibdev,
> > if (ib_copy_from_udata(&req, udata, sizeof(req)))
> > return ERR_PTR(-EINVAL);
> >
> > - if (req.userspace_ver != I40IW_ABI_USERSPACE_VER) {
> > - i40iw_pr_err("Invalid userspace driver version detected. Detected version %d, should be %d\n",
> > - req.userspace_ver, I40IW_ABI_USERSPACE_VER);
> > + if (req.userspace_ver < 4 || req.userspace_ver > I40IW_ABI_VER) {
> > + i40iw_pr_err("Unsupported provider library version %u.\n", req.userspace_ver);
> > return ERR_PTR(-EINVAL);
> > }
> >
> > @@ -155,13 +154,14 @@ static struct ib_ucontext *i40iw_alloc_ucontext(struct ib_device *ibdev,
> > uresp.max_qps = iwdev->max_qp;
> > uresp.max_pds = iwdev->max_pd;
> > uresp.wq_size = iwdev->max_qp_wr * 2;
> > - uresp.kernel_ver = I40IW_ABI_KERNEL_VER;
> > + uresp.kernel_ver = req.userspace_ver;
> >
> > ucontext = kzalloc(sizeof(*ucontext), GFP_KERNEL);
> > if (!ucontext)
> > return ERR_PTR(-ENOMEM);
> >
> > ucontext->iwdev = iwdev;
> > + ucontext->abi_ver = req.userspace_ver;
> >
> > if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) {
> > kfree(ucontext);
> > @@ -333,6 +333,7 @@ static struct ib_pd *i40iw_alloc_pd(struct ib_device *ibdev,
> > struct i40iw_sc_dev *dev = &iwdev->sc_dev;
> > struct i40iw_alloc_pd_resp uresp;
> > struct i40iw_sc_pd *sc_pd;
> > + struct i40iw_ucontext *ucontext;
> > u32 pd_id = 0;
> > int err;
> >
> > @@ -353,15 +354,18 @@ static struct ib_pd *i40iw_alloc_pd(struct ib_device *ibdev,
> > }
> >
> > sc_pd = &iwpd->sc_pd;
> > - dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id);
> >
> > if (context) {
> > + ucontext = to_ucontext(context);
> > + dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, ucontext->abi_ver);
> > memset(&uresp, 0, sizeof(uresp));
> > uresp.pd_id = pd_id;
> > if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) {
> > err = -EFAULT;
> > goto error;
> > }
> > + } else {
> > + dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, -1);
> > }
> >
> > i40iw_add_pdusecount(iwpd);
> > @@ -518,7 +522,7 @@ static int i40iw_setup_kmode_qp(struct i40iw_device *iwdev,
> > struct i40iw_dma_mem *mem = &iwqp->kqp.dma_mem;
> > u32 sqdepth, rqdepth;
> > u32 sq_size, rq_size;
> > - u8 sqshift, rqshift;
> > + u8 sqshift;
> > u32 size;
> > enum i40iw_status_code status;
> > struct i40iw_qp_uk_init_info *ukinfo = &info->qp_uk_init_info;
> > @@ -527,14 +531,11 @@ static int i40iw_setup_kmode_qp(struct i40iw_device *iwdev,
> > rq_size = i40iw_qp_roundup(ukinfo->rq_size + 1);
> >
> > status = i40iw_get_wqe_shift(sq_size, ukinfo->max_sq_frag_cnt, ukinfo->max_inline_data, &sqshift);
> > - if (!status)
> > - status = i40iw_get_wqe_shift(rq_size, ukinfo->max_rq_frag_cnt, 0, &rqshift);
> > -
> > if (status)
> > return -ENOMEM;
> >
> > sqdepth = sq_size << sqshift;
> > - rqdepth = rq_size << rqshift;
> > + rqdepth = rq_size << I40IW_MAX_RQ_WQE_SHIFT;
> >
> > size = sqdepth * sizeof(struct i40iw_sq_uk_wr_trk_info) + (rqdepth << 3);
> > iwqp->kqp.wrid_mem = kzalloc(size, GFP_KERNEL);
> > diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.h b/drivers/infiniband/hw/i40iw/i40iw_verbs.h
> > index 6549c93..07c3fec 100644
> > --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.h
> > +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.h
> > @@ -42,6 +42,7 @@ struct i40iw_ucontext {
> > spinlock_t cq_reg_mem_list_lock; /* memory list for cq's */
> > struct list_head qp_reg_mem_list;
> > spinlock_t qp_reg_mem_list_lock; /* memory list for qp's */
> > + int abi_ver;
> > };
> >
> > struct i40iw_pd {
> > --
> > 1.8.3.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] 15+ messages in thread
* Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <20161220164118.GA91636-ZmvEvTIhtuUfyugFOqMDN/ooFf0ArEBIu+b9c/7xato@public.gmane.org>
@ 2016-12-20 17:30 ` Yuval Shaia
2016-12-20 17:55 ` Henry Orosco
0 siblings, 1 reply; 15+ messages in thread
From: Yuval Shaia @ 2016-12-20 17:30 UTC (permalink / raw)
To: Henry Orosco
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Tung, Chien Tin
On Tue, Dec 20, 2016 at 10:41:18AM -0600, Henry Orosco wrote:
> > >
> > > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> > > index 392f783..98923a8 100644
> > > --- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> > > +++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> > > @@ -358,13 +358,16 @@ void i40iw_qp_add_qos(struct i40iw_sc_qp *qp)
> > > * @dev: sc device struct
> > > * @pd: sc pd ptr
> > > * @pd_id: pd_id for allocated pd
> > > + * @abi_ver: ABI version from user context, -1 if not valid
> > > */
> > > static void i40iw_sc_pd_init(struct i40iw_sc_dev *dev,
> > > struct i40iw_sc_pd *pd,
> > > - u16 pd_id)
> > > + u16 pd_id,
> > > + int abi_ver)
> >
> > No need for new line here
> >
>
> Not sure what you mean here.
>From code-style perspective "u16 pd_id," and "int abi_ver" can be on one line.
>
> > > {
> > > pd->size = sizeof(*pd);
> > > pd->pd_id = pd_id;
> > > + pd->abi_ver = abi_ver;
> > > pd->dev = dev;
> > > }
> > >
--
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] 15+ messages in thread
* Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
2016-12-20 17:30 ` Yuval Shaia
@ 2016-12-20 17:55 ` Henry Orosco
0 siblings, 0 replies; 15+ messages in thread
From: Henry Orosco @ 2016-12-20 17:55 UTC (permalink / raw)
To: Yuval Shaia
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Tung, Chien Tin
On Tue, Dec 20, 2016 at 07:30:45PM +0200, Yuval Shaia wrote:
> On Tue, Dec 20, 2016 at 10:41:18AM -0600, Henry Orosco wrote:
> > > >
> > > > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> > > > index 392f783..98923a8 100644
> > > > --- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> > > > +++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
> > > > @@ -358,13 +358,16 @@ void i40iw_qp_add_qos(struct i40iw_sc_qp *qp)
> > > > * @dev: sc device struct
> > > > * @pd: sc pd ptr
> > > > * @pd_id: pd_id for allocated pd
> > > > + * @abi_ver: ABI version from user context, -1 if not valid
> > > > */
> > > > static void i40iw_sc_pd_init(struct i40iw_sc_dev *dev,
> > > > struct i40iw_sc_pd *pd,
> > > > - u16 pd_id)
> > > > + u16 pd_id,
> > > > + int abi_ver)
> > >
> > > No need for new line here
> > >
> >
> > Not sure what you mean here.
>
> From code-style perspective "u16 pd_id," and "int abi_ver" can be on one line.
We will opt to keep it as originally sent (one parameter per line) as this is
consistent with the rest of the file.
>
> >
> > > > {
> > > > pd->size = sizeof(*pd);
> > > > pd->pd_id = pd_id;
> > > > + pd->abi_ver = abi_ver;
> > > > pd->dev = dev;
> > > > }
> > > >
--
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] 15+ messages in thread
* Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBAFBBC-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
@ 2016-12-20 20:02 ` Leon Romanovsky
[not found] ` <20161220200239.GA1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2016-12-20 20:02 UTC (permalink / raw)
To: Tung, Chien Tin
Cc: Orosco, Henry, dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
[-- Attachment #1: Type: text/plain, Size: 3100 bytes --]
On Tue, Dec 20, 2016 at 02:46:30PM +0000, Tung, Chien Tin wrote:
>
>
> > -----Original Message-----
> > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > Sent: Tuesday, December 20, 2016 7:42 AM
> > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> > linux- rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
> >
> > On Tue, Dec 20, 2016 at 12:44:04PM +0000, Tung, Chien Tin wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> > > > owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Leon Romanovsky
> > > > Sent: Tuesday, December 20, 2016 5:24 AM
> > > > To: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-
> > > > rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Tung, Chien Tin
> > > > <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE
> > > > size
> > > >
> > > > > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > index 12acd68..57d3f1d 100644
> > > > > --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > @@ -39,8 +39,8 @@
> > > > >
> > > > > #include <linux/types.h>
> > > > >
> > > > > -#define I40IW_ABI_USERSPACE_VER 4
> > > > > -#define I40IW_ABI_KERNEL_VER 4
> > > > > +#define I40IW_ABI_VER 5
> > > > > +
> > > >
> > > > Why did you remove defines and move to use constants "4" and "5"
> > instead?
> > > [Chien Tin Tung] This is the ABI version change, did you read the
> > > commit
> > message? Two defines were not necessary.
> > >
> >
> > Thank you Chian Tin Tung for your informative answer and yes, I read
> > commit message.
> >
> > If you think that these defines are not needed, so can you please
> > remove the code which uses hardcoded "case 4" and "case 5"?
> [Chien Tin Tung] Those are explicit checks against a particular ABI
> version and it is clearer to use the numbers rather than some define.
Can you please stop to add [..] in front of your response? This Outlook
style is annoying and not needed.
Regarding you response, you added the same check in two places and from
grep/ctags/cscope/lxr perspective it is easier to spot them with a
define.
>
> >
> > Or maybe they still needed and should be renamed?
> [Chien Tin Tung] They are not needed. Before we were checking against
> one version and one version only. Now we are making the provider library
> and driver backward compatible. All we need is one define to specify current
> ABI version.
>
> Chien
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <20161220200239.GA1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2016-12-21 0:04 ` Tung, Chien Tin
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBB12AF-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Tung, Chien Tin @ 2016-12-21 0:04 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Orosco, Henry, dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
> -----Original Message-----
> From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> Sent: Tuesday, December 20, 2016 2:03 PM
> To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-
> rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
>
> On Tue, Dec 20, 2016 at 02:46:30PM +0000, Tung, Chien Tin wrote:
> >
> >
> > > -----Original Message-----
> > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > Sent: Tuesday, December 20, 2016 7:42 AM
> > > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> > > linux- rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE
> > > size
> > >
> > > On Tue, Dec 20, 2016 at 12:44:04PM +0000, Tung, Chien Tin wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> > > > > owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Leon Romanovsky
> > > > > Sent: Tuesday, December 20, 2016 5:24 AM
> > > > > To: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-
> > > > > rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Tung, Chien Tin
> > > > > <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ
> > > > > WQE size
> > > > >
> > > > > > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > index 12acd68..57d3f1d 100644
> > > > > > --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > @@ -39,8 +39,8 @@
> > > > > >
> > > > > > #include <linux/types.h>
> > > > > >
> > > > > > -#define I40IW_ABI_USERSPACE_VER 4
> > > > > > -#define I40IW_ABI_KERNEL_VER 4
> > > > > > +#define I40IW_ABI_VER 5
> > > > > > +
> > > > >
> > > > > Why did you remove defines and move to use constants "4" and "5"
> > > instead?
> > > > [Chien Tin Tung] This is the ABI version change, did you read the
> > > > commit
> > > message? Two defines were not necessary.
> > > >
> > >
> > > Thank you Chian Tin Tung for your informative answer and yes, I read
> > > commit message.
> > >
> > > If you think that these defines are not needed, so can you please
> > > remove the code which uses hardcoded "case 4" and "case 5"?
> > [Chien Tin Tung] Those are explicit checks against a particular ABI
> > version and it is clearer to use the numbers rather than some define.
>
> Can you please stop to add [..] in front of your response? This Outlook style is
> annoying and not needed.
>
> Regarding you response, you added the same check in two places and from
> grep/ctags/cscope/lxr perspective it is easier to spot them with a define.
You have yet to provide a good argument for changing the 4 and 5 to defines,
so I don't know why you are telling me how to find them. Very puzzling.
I already made my case on this. Enough said.
Chien
>
[Chien]
> >
> > >
> > > Or maybe they still needed and should be renamed?
> > [Chien Tin Tung] They are not needed. Before we were checking against
> > one version and one version only. Now we are making the provider
> > library and driver backward compatible. All we need is one define to
> > specify current ABI version.
> >
> > Chien
--
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] 15+ messages in thread
* Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBB12AF-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
@ 2016-12-21 5:48 ` Leon Romanovsky
[not found] ` <20161221054805.GB1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2016-12-21 5:48 UTC (permalink / raw)
To: Tung, Chien Tin
Cc: Orosco, Henry, dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
[-- Attachment #1: Type: text/plain, Size: 4844 bytes --]
On Wed, Dec 21, 2016 at 12:04:55AM +0000, Tung, Chien Tin wrote:
>
>
> > -----Original Message-----
> > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > Sent: Tuesday, December 20, 2016 2:03 PM
> > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-
> > rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
> >
> > On Tue, Dec 20, 2016 at 02:46:30PM +0000, Tung, Chien Tin wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > > Sent: Tuesday, December 20, 2016 7:42 AM
> > > > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> > > > linux- rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE
> > > > size
> > > >
> > > > On Tue, Dec 20, 2016 at 12:44:04PM +0000, Tung, Chien Tin wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> > > > > > owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Leon Romanovsky
> > > > > > Sent: Tuesday, December 20, 2016 5:24 AM
> > > > > > To: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-
> > > > > > rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Tung, Chien Tin
> > > > > > <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ
> > > > > > WQE size
> > > > > >
> > > > > > > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > index 12acd68..57d3f1d 100644
> > > > > > > --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > @@ -39,8 +39,8 @@
> > > > > > >
> > > > > > > #include <linux/types.h>
> > > > > > >
> > > > > > > -#define I40IW_ABI_USERSPACE_VER 4
> > > > > > > -#define I40IW_ABI_KERNEL_VER 4
> > > > > > > +#define I40IW_ABI_VER 5
> > > > > > > +
> > > > > >
> > > > > > Why did you remove defines and move to use constants "4" and "5"
> > > > instead?
> > > > > [Chien Tin Tung] This is the ABI version change, did you read the
> > > > > commit
> > > > message? Two defines were not necessary.
> > > > >
> > > >
> > > > Thank you Chian Tin Tung for your informative answer and yes, I read
> > > > commit message.
> > > >
> > > > If you think that these defines are not needed, so can you please
> > > > remove the code which uses hardcoded "case 4" and "case 5"?
> > > [Chien Tin Tung] Those are explicit checks against a particular ABI
> > > version and it is clearer to use the numbers rather than some define.
> >
> > Can you please stop to add [..] in front of your response? This Outlook style is
> > annoying and not needed.
> >
> > Regarding you response, you added the same check in two places and from
> > grep/ctags/cscope/lxr perspective it is easier to spot them with a define.
>
> You have yet to provide a good argument for changing the 4 and 5 to defines,
> so I don't know why you are telling me how to find them. Very puzzling.
I'm not telling to you, but asking from you to provide maintainable code
which will be read more than once. Right now, you blindly copy-pasted
hardcoded values in three different places (2 kernel and 1 rdma-core).
Feel free to ask your colleagues (Faisal L. and Shiraz S.) if they
interested in maintainable driver code or they prefer your prone to errors
copy-paste technique.
> I already made my case on this. Enough said.
>
> Chien
>
> >
> [Chien]
> > >
> > > >
> > > > Or maybe they still needed and should be renamed?
> > > [Chien Tin Tung] They are not needed. Before we were checking against
> > > one version and one version only. Now we are making the provider
> > > library and driver backward compatible. All we need is one define to
> > > specify current ABI version.
> > >
> > > Chien
> --
> 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] 15+ messages in thread
* RE: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <20161221054805.GB1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2016-12-21 14:07 ` Tung, Chien Tin
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBB1BA8-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Tung, Chien Tin @ 2016-12-21 14:07 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Orosco, Henry, dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
> -----Original Message-----
> From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> Sent: Tuesday, December 20, 2016 11:48 PM
> To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-
> rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
>
> On Wed, Dec 21, 2016 at 12:04:55AM +0000, Tung, Chien Tin wrote:
> >
> >
> > > -----Original Message-----
> > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > Sent: Tuesday, December 20, 2016 2:03 PM
> > > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> > > linux- rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE
> > > size
> > >
> > > On Tue, Dec 20, 2016 at 02:46:30PM +0000, Tung, Chien Tin wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > > > Sent: Tuesday, December 20, 2016 7:42 AM
> > > > > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> > > > > linux- rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ
> > > > > WQE size
> > > > >
> > > > > On Tue, Dec 20, 2016 at 12:44:04PM +0000, Tung, Chien Tin wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> > > > > > > owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Leon Romanovsky
> > > > > > > Sent: Tuesday, December 20, 2016 5:24 AM
> > > > > > > To: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-
> > > > > > > rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Tung, Chien Tin
> > > > > > > <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported
> > > > > > > RQ WQE size
> > > > > > >
> > > > > > > > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > index 12acd68..57d3f1d 100644
> > > > > > > > --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > @@ -39,8 +39,8 @@
> > > > > > > >
> > > > > > > > #include <linux/types.h>
> > > > > > > >
> > > > > > > > -#define I40IW_ABI_USERSPACE_VER 4
> > > > > > > > -#define I40IW_ABI_KERNEL_VER 4
> > > > > > > > +#define I40IW_ABI_VER 5
> > > > > > > > +
> > > > > > >
> > > > > > > Why did you remove defines and move to use constants "4" and "5"
> > > > > instead?
> > > > > > [Chien Tin Tung] This is the ABI version change, did you read
> > > > > > the commit
> > > > > message? Two defines were not necessary.
> > > > > >
> > > > >
> > > > > Thank you Chian Tin Tung for your informative answer and yes, I
> > > > > read commit message.
> > > > >
> > > > > If you think that these defines are not needed, so can you
> > > > > please remove the code which uses hardcoded "case 4" and "case 5"?
> > > > [Chien Tin Tung] Those are explicit checks against a particular
> > > > ABI version and it is clearer to use the numbers rather than some define.
> > >
> > > Can you please stop to add [..] in front of your response? This
> > > Outlook style is annoying and not needed.
> > >
> > > Regarding you response, you added the same check in two places and
> > > from grep/ctags/cscope/lxr perspective it is easier to spot them with a
> define.
> >
> > You have yet to provide a good argument for changing the 4 and 5 to
> > defines, so I don't know why you are telling me how to find them. Very
> puzzling.
>
> I'm not telling to you, but asking from you to provide maintainable code which
> will be read more than once. Right now, you blindly copy-pasted hardcoded
> values in three different places (2 kernel and 1 rdma-core).
Please make your case by coming up with a define that's better than 4 or 5,
making the code more readable and understandable, then I will consider using it.
Until you can do that, you have no case for your comment. Blindly using a define
is something you tell junior programmers, sometimes, a number is just a number,
nothing wrong with using it as is.
>
> Feel free to ask your colleagues (Faisal L. and Shiraz S.) if they interested in
> maintainable driver code or they prefer your prone to errors copy-paste
> technique.
Why don't you go back and read the comment made against I40IW_SUCCESS?
Maybe you will learn something from that?
>
> > I already made my case on this. Enough said.
> >
> > Chien
> >
> > >
> > [Chien]
> > > >
> > > > >
> > > > > Or maybe they still needed and should be renamed?
> > > > [Chien Tin Tung] They are not needed. Before we were checking
> > > > against one version and one version only. Now we are making the
> > > > provider library and driver backward compatible. All we need is
> > > > one define to specify current ABI version.
> > > >
> > > > Chien
> > --
> > 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] 15+ messages in thread
* Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBB1BA8-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
@ 2016-12-21 15:12 ` Leon Romanovsky
[not found] ` <20161221151250.GA18935-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2016-12-21 15:12 UTC (permalink / raw)
To: Tung, Chien Tin
Cc: Orosco, Henry, dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
[-- Attachment #1: Type: text/plain, Size: 6961 bytes --]
On Wed, Dec 21, 2016 at 02:07:15PM +0000, Tung, Chien Tin wrote:
>
>
> > -----Original Message-----
> > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > Sent: Tuesday, December 20, 2016 11:48 PM
> > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-
> > rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
> >
> > On Wed, Dec 21, 2016 at 12:04:55AM +0000, Tung, Chien Tin wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > > Sent: Tuesday, December 20, 2016 2:03 PM
> > > > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> > > > linux- rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE
> > > > size
> > > >
> > > > On Tue, Dec 20, 2016 at 02:46:30PM +0000, Tung, Chien Tin wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > > > > Sent: Tuesday, December 20, 2016 7:42 AM
> > > > > > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> > > > > > linux- rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > > > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ
> > > > > > WQE size
> > > > > >
> > > > > > On Tue, Dec 20, 2016 at 12:44:04PM +0000, Tung, Chien Tin wrote:
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> > > > > > > > owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Leon Romanovsky
> > > > > > > > Sent: Tuesday, December 20, 2016 5:24 AM
> > > > > > > > To: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > > > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-
> > > > > > > > rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Tung, Chien Tin
> > > > > > > > <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > > > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported
> > > > > > > > RQ WQE size
> > > > > > > >
> > > > > > > > > diff --git a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > > b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > > index 12acd68..57d3f1d 100644
> > > > > > > > > --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > > +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > > @@ -39,8 +39,8 @@
> > > > > > > > >
> > > > > > > > > #include <linux/types.h>
> > > > > > > > >
> > > > > > > > > -#define I40IW_ABI_USERSPACE_VER 4
> > > > > > > > > -#define I40IW_ABI_KERNEL_VER 4
> > > > > > > > > +#define I40IW_ABI_VER 5
> > > > > > > > > +
> > > > > > > >
> > > > > > > > Why did you remove defines and move to use constants "4" and "5"
> > > > > > instead?
> > > > > > > [Chien Tin Tung] This is the ABI version change, did you read
> > > > > > > the commit
> > > > > > message? Two defines were not necessary.
> > > > > > >
> > > > > >
> > > > > > Thank you Chian Tin Tung for your informative answer and yes, I
> > > > > > read commit message.
> > > > > >
> > > > > > If you think that these defines are not needed, so can you
> > > > > > please remove the code which uses hardcoded "case 4" and "case 5"?
> > > > > [Chien Tin Tung] Those are explicit checks against a particular
> > > > > ABI version and it is clearer to use the numbers rather than some define.
> > > >
> > > > Can you please stop to add [..] in front of your response? This
> > > > Outlook style is annoying and not needed.
> > > >
> > > > Regarding you response, you added the same check in two places and
> > > > from grep/ctags/cscope/lxr perspective it is easier to spot them with a
> > define.
> > >
> > > You have yet to provide a good argument for changing the 4 and 5 to
> > > defines, so I don't know why you are telling me how to find them. Very
> > puzzling.
> >
> > I'm not telling to you, but asking from you to provide maintainable code which
> > will be read more than once. Right now, you blindly copy-pasted hardcoded
> > values in three different places (2 kernel and 1 rdma-core).
>
> Please make your case by coming up with a define that's better than 4 or 5,
> making the code more readable and understandable, then I will consider using it.
First of all you are already have define for 5, but who cares, you are
not junior and don't need to read the code which you wrote minute before.
Second, you need to come with new name for number 4 only.
What about following?
#define I40IW_LEGACY_ABI 4
> Until you can do that, you have no case for your comment. Blindly using a define
> is something you tell junior programmers, sometimes, a number is just a number,
> nothing wrong with using it as is.
>
> >
> > Feel free to ask your colleagues (Faisal L. and Shiraz S.) if they interested in
> > maintainable driver code or they prefer your prone to errors copy-paste
> > technique.
>
> Why don't you go back and read the comment made against I40IW_SUCCESS?
> Maybe you will learn something from that?
Your word is a lamp for my feet, a light on my path. Please enlighten me.
Happy Christmas.
>
> >
> > > I already made my case on this. Enough said.
> > >
> > > Chien
> > >
> > > >
> > > [Chien]
> > > > >
> > > > > >
> > > > > > Or maybe they still needed and should be renamed?
> > > > > [Chien Tin Tung] They are not needed. Before we were checking
> > > > > against one version and one version only. Now we are making the
> > > > > provider library and driver backward compatible. All we need is
> > > > > one define to specify current ABI version.
> > > > >
> > > > > Chien
> > > --
> > > 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
[not found] ` <20161221151250.GA18935-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2016-12-21 16:08 ` Tung, Chien Tin
0 siblings, 0 replies; 15+ messages in thread
From: Tung, Chien Tin @ 2016-12-21 16:08 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Orosco, Henry, dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
> -----Original Message-----
> From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Leon Romanovsky
> Sent: Wednesday, December 21, 2016 9:13 AM
> To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-
> rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE size
>
> On Wed, Dec 21, 2016 at 02:07:15PM +0000, Tung, Chien Tin wrote:
> >
> >
> > > -----Original Message-----
> > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > Sent: Tuesday, December 20, 2016 11:48 PM
> > > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> > > linux- rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ WQE
> > > size
> > >
> > > On Wed, Dec 21, 2016 at 12:04:55AM +0000, Tung, Chien Tin wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > > > Sent: Tuesday, December 20, 2016 2:03 PM
> > > > > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> > > > > linux- rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported RQ
> > > > > WQE size
> > > > >
> > > > > On Tue, Dec 20, 2016 at 02:46:30PM +0000, Tung, Chien Tin wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > > > > > Sent: Tuesday, December 20, 2016 7:42 AM
> > > > > > > To: Tung, Chien Tin <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > > > Cc: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>;
> > > > > > > dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org;
> > > > > > > linux- rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > > > > > > e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > > > > > Subject: Re: [PATCH] i40iw: Set 128B as the only supported
> > > > > > > RQ WQE size
> > > > > > >
> > > > > > > On Tue, Dec 20, 2016 at 12:44:04PM +0000, Tung, Chien Tin wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > > > > > > [mailto:linux-rdma- owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of
> > > > > > > > > Leon Romanovsky
> > > > > > > > > Sent: Tuesday, December 20, 2016 5:24 AM
> > > > > > > > > To: Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > > > > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > > > > > > > > e1000- rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Tung, Chien Tin
> > > > > > > > > <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > > > > > Subject: Re: [PATCH] i40iw: Set 128B as the only
> > > > > > > > > supported RQ WQE size
> > > > > > > > >
> > > > > > > > > > diff --git
> > > > > > > > > > a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > > > b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > > > index 12acd68..57d3f1d 100644
> > > > > > > > > > --- a/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > > > +++ b/drivers/infiniband/hw/i40iw/i40iw_ucontext.h
> > > > > > > > > > @@ -39,8 +39,8 @@
> > > > > > > > > >
> > > > > > > > > > #include <linux/types.h>
> > > > > > > > > >
> > > > > > > > > > -#define I40IW_ABI_USERSPACE_VER 4
> > > > > > > > > > -#define I40IW_ABI_KERNEL_VER 4
> > > > > > > > > > +#define I40IW_ABI_VER 5
> > > > > > > > > > +
> > > > > > > > >
> > > > > > > > > Why did you remove defines and move to use constants "4" and
> "5"
> > > > > > > instead?
> > > > > > > > [Chien Tin Tung] This is the ABI version change, did you
> > > > > > > > read the commit
> > > > > > > message? Two defines were not necessary.
> > > > > > > >
> > > > > > >
> > > > > > > Thank you Chian Tin Tung for your informative answer and
> > > > > > > yes, I read commit message.
> > > > > > >
> > > > > > > If you think that these defines are not needed, so can you
> > > > > > > please remove the code which uses hardcoded "case 4" and "case
> 5"?
> > > > > > [Chien Tin Tung] Those are explicit checks against a
> > > > > > particular ABI version and it is clearer to use the numbers rather
> than some define.
> > > > >
> > > > > Can you please stop to add [..] in front of your response? This
> > > > > Outlook style is annoying and not needed.
> > > > >
> > > > > Regarding you response, you added the same check in two places
> > > > > and from grep/ctags/cscope/lxr perspective it is easier to spot
> > > > > them with a
> > > define.
> > > >
> > > > You have yet to provide a good argument for changing the 4 and 5
> > > > to defines, so I don't know why you are telling me how to find
> > > > them. Very
> > > puzzling.
> > >
> > > I'm not telling to you, but asking from you to provide maintainable
> > > code which will be read more than once. Right now, you blindly
> > > copy-pasted hardcoded values in three different places (2 kernel and 1
> rdma-core).
> >
> > Please make your case by coming up with a define that's better than 4
> > or 5, making the code more readable and understandable, then I will
> consider using it.
>
> First of all you are already have define for 5, but who cares, you are not junior
> and don't need to read the code which you wrote minute before.
You should think about the usage for that define. That is the current ABI version.
That means the driver is at ABI version level 5. The value of that define will not stay
as 5 with the next ABI version change.
>
> Second, you need to come with new name for number 4 only.
> What about following?
> #define I40IW_LEGACY_ABI 4
So what do you do with the next ABI version rev? What is LEGACY_ABI?
You are very short sighted with this kind of suggestion.
Again, there is a specific fix for ABI version 4 and ABI version 5 and the
next ABI version. This is the reason I'm checking against specific
version number because it has meaning. Abstracting the actual number
away from the value makes it much less readable.
Chien
>
> > Until you can do that, you have no case for your comment. Blindly
> > using a define is something you tell junior programmers, sometimes, a
> > number is just a number, nothing wrong with using it as is.
> >
> > >
> > > Feel free to ask your colleagues (Faisal L. and Shiraz S.) if they
> > > interested in maintainable driver code or they prefer your prone to
> > > errors copy-paste technique.
> >
> > Why don't you go back and read the comment made against
> I40IW_SUCCESS?
> > Maybe you will learn something from that?
>
> Your word is a lamp for my feet, a light on my path. Please enlighten me.
I hope this clears up your misunderstand regarding my patch. I can't be any more
clear than this.
Chien
>
> Happy Christmas.
>
> >
> > >
> > > > I already made my case on this. Enough said.
> > > >
> > > > Chien
> > > >
> > > > >
> > > > [Chien]
> > > > > >
> > > > > > >
> > > > > > > Or maybe they still needed and should be renamed?
> > > > > > [Chien Tin Tung] They are not needed. Before we were checking
> > > > > > against one version and one version only. Now we are making
> > > > > > the provider library and driver backward compatible. All we
> > > > > > need is one define to specify current ABI version.
> > > > > >
> > > > > > Chien
> > > > --
> > > > 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
--
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] 15+ messages in thread
end of thread, other threads:[~2016-12-21 16:08 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-19 20:32 [PATCH] i40iw: Set 128B as the only supported RQ WQE size Henry Orosco
[not found] ` <20161219203227.86392-1-henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-12-20 10:47 ` Yuval Shaia
[not found] ` <20161220104720.GD7351-Hxa29pjIrETlQW142y8m19+IiqhCXseY@public.gmane.org>
2016-12-20 16:41 ` Henry Orosco
[not found] ` <20161220164118.GA91636-ZmvEvTIhtuUfyugFOqMDN/ooFf0ArEBIu+b9c/7xato@public.gmane.org>
2016-12-20 17:30 ` Yuval Shaia
2016-12-20 17:55 ` Henry Orosco
2016-12-20 11:24 ` Leon Romanovsky
[not found] ` <20161220112417.GV1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2016-12-20 12:44 ` Tung, Chien Tin
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBAEEA0-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2016-12-20 13:42 ` Leon Romanovsky
[not found] ` <20161220134203.GW1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2016-12-20 14:46 ` Tung, Chien Tin
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBAFBBC-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2016-12-20 20:02 ` Leon Romanovsky
[not found] ` <20161220200239.GA1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2016-12-21 0:04 ` Tung, Chien Tin
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBB12AF-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2016-12-21 5:48 ` Leon Romanovsky
[not found] ` <20161221054805.GB1074-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2016-12-21 14:07 ` Tung, Chien Tin
[not found] ` <748B799B6A00724488C603FD7E5E7EB94CBB1BA8-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2016-12-21 15:12 ` Leon Romanovsky
[not found] ` <20161221151250.GA18935-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2016-12-21 16:08 ` Tung, Chien Tin
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.