[PATCH V2 0/4] Misc changes for siw

[PATCH V2 0/4] Misc changes for siw

[Guoqing Jiang]

V2 changes:
1. Collect tags from Bernard.
2. Use SIW_QP_STATE_COUNT for siw_qp_state_to_ib_qp_state.
3. Update commite message for the last patch.

Thanks for Bernard's review!

Hi,

The first two patches try to reduce memory usage, the third
one fix test_query_rc_qp failure, and the last one clean up
code a bit.

Please review.

Thanks,
Guoqing

Guoqing Jiang (4):
  RDMA/siw: Move tx_cpu ahead
  RDMA/siw: Reduce memory usage of struct siw_rx_stream
  RDMA/siw: Set qp_state in siw_query_qp
  RDMA/siw: Call orq_get_current if possible

 drivers/infiniband/sw/siw/siw.h       | 10 +++++-----
 drivers/infiniband/sw/siw/siw_verbs.c | 10 ++++++++++
 2 files changed, 15 insertions(+), 5 deletions(-)


base-commit: 50af5d12f7e24b85fc10270d7700f4aa1b20b8e4
-- 
2.35.3

[PATCH V2 1/4] RDMA/siw: Move tx_cpu ahead

[Guoqing Jiang]

We can reduce one cacheline for the usage of struct siw_qp.

Before,

	/* size: 1928, cachelines: 31, members: 38 */
	/* sum members: 1920, holes: 2, sum holes: 8 */
	/* paddings: 4, sum paddings: 13 */
	/* forced alignments: 3 */

after

	/* size: 1920, cachelines: 30, members: 38 */
	/* paddings: 4, sum paddings: 13 */
	/* forced alignments: 3 */

Acked-by: Bernard Metzler <bmt@zurich.ibm.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
 drivers/infiniband/sw/siw/siw.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/siw/siw.h b/drivers/infiniband/sw/siw/siw.h
index b36d1ec25327..d14bb965af75 100644
--- a/drivers/infiniband/sw/siw/siw.h
+++ b/drivers/infiniband/sw/siw/siw.h
@@ -417,10 +417,10 @@ struct siw_iwarp_tx {
 struct siw_qp {
 	struct ib_qp base_qp;
 	struct siw_device *sdev;
+	int tx_cpu;
 	struct kref ref;
 	struct completion qp_free;
 	struct list_head devq;
-	int tx_cpu;
 	struct siw_qp_attrs attrs;
 
 	struct siw_cep *cep;
-- 
2.35.3

[PATCH V2 2/4] RDMA/siw: Reduce memory usage of struct siw_rx_stream

[Guoqing Jiang]

We can reduce the memory of the struct by move some of it's
member.

Before,

	/* size: 144, cachelines: 3, members: 17 */
	/* sum members: 124, holes: 3, sum holes: 12 */
	/* sum bitfield members: 7 bits (0 bytes) */
	/* padding: 7 */
	/* bit_padding: 1 bits */

After

	/* size: 128, cachelines: 2, members: 17 */
	/* padding: 3 */
	/* bit_padding: 1 bits */

Acked-by: Bernard Metzler <bmt@zurich.ibm.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
 drivers/infiniband/sw/siw/siw.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw.h b/drivers/infiniband/sw/siw/siw.h
index d14bb965af75..2edba2a864bb 100644
--- a/drivers/infiniband/sw/siw/siw.h
+++ b/drivers/infiniband/sw/siw/siw.h
@@ -288,10 +288,11 @@ struct siw_rx_stream {
 	int skb_offset; /* offset in skb */
 	int skb_copied; /* processed bytes in skb */
 
+	enum siw_rx_state state;
+
 	union iwarp_hdr hdr;
 	struct mpa_trailer trailer;
-
-	enum siw_rx_state state;
+	struct shash_desc *mpa_crc_hd;
 
 	/*
 	 * For each FPDU, main RX loop runs through 3 stages:
@@ -313,7 +314,6 @@ struct siw_rx_stream {
 	u64 ddp_to;
 	u32 inval_stag; /* Stag to be invalidated */
 
-	struct shash_desc *mpa_crc_hd;
 	u8 rx_suspend : 1;
 	u8 pad : 2; /* # of pad bytes expected */
 	u8 rdmap_op : 4; /* opcode of current frame */
-- 
2.35.3

[PATCH V2 3/4] RDMA/siw: Set qp_state in siw_query_qp

[Guoqing Jiang]

Run test_query_rc_qp against siw failed since siw didn't set qp_state
accordingly. To address it, introduce siw_qp_state_to_ib_qp_state
which convert SIW_QP_STATE_IDLE to IB_QPS_INIT which is similar as
in cxgb4.

rdma-core# ./build/bin/run_tests.py --dev siw0 tests.test_qp.QPTest.test_query_rc_qp -v
test_query_rc_qp (tests.test_qp.QPTest)
Queries an RC QP after creation. Verifies that its properties are as ... FAIL

======================================================================
FAIL: test_query_rc_qp (tests.test_qp.QPTest)
Queries an RC QP after creation. Verifies that its properties are as
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/gjiang/rdma-core/tests/test_qp.py", line 284, in test_query_rc_qp
    self.query_qp_common_test(e.IBV_QPT_RC)
  File "/home/gjiang/rdma-core/tests/test_qp.py", line 265, in query_qp_common_test
    self.verify_qp_attrs(caps, e.IBV_QPS_INIT, qp_init_attr, qp_attr)
  File "/home/gjiang/rdma-core/tests/test_qp.py", line 239, in verify_qp_attrs
    self.assertEqual(state, attr.qp_state)
AssertionError: <ibv_qp_state.IBV_QPS_INIT: 1> != 0

----------------------------------------------------------------------
Ran 1 test in 0.057s

FAILED (failures=1)

Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
 drivers/infiniband/sw/siw/siw_verbs.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
index dca6a155523d..ecf0444666b4 100644
--- a/drivers/infiniband/sw/siw/siw_verbs.c
+++ b/drivers/infiniband/sw/siw/siw_verbs.c
@@ -19,6 +19,15 @@
 #include "siw_verbs.h"
 #include "siw_mem.h"
 
+static int siw_qp_state_to_ib_qp_state[SIW_QP_STATE_COUNT] = {
+	[SIW_QP_STATE_IDLE] = IB_QPS_INIT,
+	[SIW_QP_STATE_RTR] = IB_QPS_RTR,
+	[SIW_QP_STATE_RTS] = IB_QPS_RTS,
+	[SIW_QP_STATE_CLOSING] = IB_QPS_SQD,
+	[SIW_QP_STATE_TERMINATE] = IB_QPS_SQE,
+	[SIW_QP_STATE_ERROR] = IB_QPS_ERR
+};
+
 static int ib_qp_state_to_siw_qp_state[IB_QPS_ERR + 1] = {
 	[IB_QPS_RESET] = SIW_QP_STATE_IDLE,
 	[IB_QPS_INIT] = SIW_QP_STATE_IDLE,
@@ -504,6 +513,7 @@ int siw_query_qp(struct ib_qp *base_qp, struct ib_qp_attr *qp_attr,
 	} else {
 		return -EINVAL;
 	}
+	qp_attr->qp_state = siw_qp_state_to_ib_qp_state[qp->attrs.state];
 	qp_attr->cap.max_inline_data = SIW_MAX_INLINE;
 	qp_attr->cap.max_send_wr = qp->attrs.sq_size;
 	qp_attr->cap.max_send_sge = qp->attrs.sq_max_sges;
-- 
2.35.3

RE: [PATCH V2 3/4] RDMA/siw: Set qp_state in siw_query_qp

[Bernard Metzler]

> -----Original Message-----
> From: Guoqing Jiang <guoqing.jiang@linux.dev>
> Sent: Sunday, December 3, 2023 10:27 AM
> To: Bernard Metzler <BMT@zurich.ibm.com>; jgg@ziepe.ca; leon@kernel.org
> Cc: linux-rdma@vger.kernel.org; guoqing.jiang@linux.dev
> Subject: [EXTERNAL] [PATCH V2 3/4] RDMA/siw: Set qp_state in siw_query_qp
> 
> Run test_query_rc_qp against siw failed since siw didn't set qp_state
> accordingly. To address it, introduce siw_qp_state_to_ib_qp_state
> which convert SIW_QP_STATE_IDLE to IB_QPS_INIT which is similar as
> in cxgb4.
> 
> rdma-core# ./build/bin/run_tests.py --dev siw0
> tests.test_qp.QPTest.test_query_rc_qp -v
> test_query_rc_qp (tests.test_qp.QPTest)
> Queries an RC QP after creation. Verifies that its properties are as ...
> FAIL
> 
> ======================================================================
> FAIL: test_query_rc_qp (tests.test_qp.QPTest)
> Queries an RC QP after creation. Verifies that its properties are as
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/home/gjiang/rdma-core/tests/test_qp.py", line 284, in
> test_query_rc_qp
>     self.query_qp_common_test(e.IBV_QPT_RC)
>   File "/home/gjiang/rdma-core/tests/test_qp.py", line 265, in
> query_qp_common_test
>     self.verify_qp_attrs(caps, e.IBV_QPS_INIT, qp_init_attr, qp_attr)
>   File "/home/gjiang/rdma-core/tests/test_qp.py", line 239, in
> verify_qp_attrs
>     self.assertEqual(state, attr.qp_state)
> AssertionError: <ibv_qp_state.IBV_QPS_INIT: 1> != 0
> 
> ----------------------------------------------------------------------
> Ran 1 test in 0.057s
> 
> FAILED (failures=1)
> 
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> ---
>  drivers/infiniband/sw/siw/siw_verbs.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/infiniband/sw/siw/siw_verbs.c
> b/drivers/infiniband/sw/siw/siw_verbs.c
> index dca6a155523d..ecf0444666b4 100644
> --- a/drivers/infiniband/sw/siw/siw_verbs.c
> +++ b/drivers/infiniband/sw/siw/siw_verbs.c
> @@ -19,6 +19,15 @@
>  #include "siw_verbs.h"
>  #include "siw_mem.h"
> 
> +static int siw_qp_state_to_ib_qp_state[SIW_QP_STATE_COUNT] = {
> +	[SIW_QP_STATE_IDLE] = IB_QPS_INIT,
> +	[SIW_QP_STATE_RTR] = IB_QPS_RTR,
> +	[SIW_QP_STATE_RTS] = IB_QPS_RTS,
> +	[SIW_QP_STATE_CLOSING] = IB_QPS_SQD,
> +	[SIW_QP_STATE_TERMINATE] = IB_QPS_SQE,
> +	[SIW_QP_STATE_ERROR] = IB_QPS_ERR
> +};
> +
>  static int ib_qp_state_to_siw_qp_state[IB_QPS_ERR + 1] = {
>  	[IB_QPS_RESET] = SIW_QP_STATE_IDLE,
>  	[IB_QPS_INIT] = SIW_QP_STATE_IDLE,
> @@ -504,6 +513,7 @@ int siw_query_qp(struct ib_qp *base_qp, struct
> ib_qp_attr *qp_attr,
>  	} else {
>  		return -EINVAL;
>  	}
> +	qp_attr->qp_state = siw_qp_state_to_ib_qp_state[qp->attrs.state];
>  	qp_attr->cap.max_inline_data = SIW_MAX_INLINE;
>  	qp_attr->cap.max_send_wr = qp->attrs.sq_size;
>  	qp_attr->cap.max_send_sge = qp->attrs.sq_max_sges;
> --
> 2.35.3

Looks good.

Acked-by: Bernard Metzler <bmt@zurich.ibm.com>

[PATCH V2 4/4] RDMA/siw: Call orq_get_current if possible

[Guoqing Jiang]

Use orq_get_current() in siw_orq_empty().

Acked-by: Bernard Metzler <bmt@zurich.ibm.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
 drivers/infiniband/sw/siw/siw.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/siw/siw.h b/drivers/infiniband/sw/siw/siw.h
index 2edba2a864bb..75253f2b3e3d 100644
--- a/drivers/infiniband/sw/siw/siw.h
+++ b/drivers/infiniband/sw/siw/siw.h
@@ -657,7 +657,7 @@ static inline struct siw_sqe *orq_get_free(struct siw_qp *qp)
 
 static inline int siw_orq_empty(struct siw_qp *qp)
 {
-	return qp->orq[qp->orq_get % qp->attrs.orq_size].flags == 0 ? 1 : 0;
+	return orq_get_current(qp)->flags == 0 ? 1 : 0;
 }
 
 static inline struct siw_sqe *irq_alloc_free(struct siw_qp *qp)
-- 
2.35.3

Re: [PATCH V2 0/4] Misc changes for siw

[Jason Gunthorpe]

On Sun, Dec 03, 2023 at 05:26:51PM +0800, Guoqing Jiang wrote:

> Guoqing Jiang (4):
>   RDMA/siw: Move tx_cpu ahead
>   RDMA/siw: Reduce memory usage of struct siw_rx_stream
>   RDMA/siw: Set qp_state in siw_query_qp
>   RDMA/siw: Call orq_get_current if possible

Applied to for-next, thanks

Jason