diff mbox

[rdma-core,v3,4/9] libbnxt_re: Add support for posting and polling

Message ID 20170315163454.GD29562@obsidianresearch.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Jason Gunthorpe March 15, 2017, 4:34 p.m. UTC 
On Wed, Mar 15, 2017 at 06:37:28AM -0400, Devesh Sharma wrote:
> +
> +	pthread_spin_lock(&dpi->db_lock);
> +	*dbval = htole64(*dbval);
> +	udma_ordering_write_barrier();

The ordering_write_barrier is basically an equivalent barrier to a
spinlock, so this is an unnecessary pattern.

However, you still always need udma_to_device_barrier before
triggering chip DMA.

There is also no udma_from_device_barrier() in this driver, which
can't be right, please add one around bnxt_re_is_cqe_valid most
likely.

Your github branch doesn't compile, btw.

I ran it through sparse, this is what I suggest..

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Devesh Sharma March 16, 2017, 3:04 a.m. UTC | #1 
On Wed, Mar 15, 2017 at 10:04 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Wed, Mar 15, 2017 at 06:37:28AM -0400, Devesh Sharma wrote:
>> +
>> +     pthread_spin_lock(&dpi->db_lock);
>> +     *dbval = htole64(*dbval);
>> +     udma_ordering_write_barrier();
>
> The ordering_write_barrier is basically an equivalent barrier to a
> spinlock, so this is an unnecessary pattern.
>
> However, you still always need udma_to_device_barrier before
> triggering chip DMA.

I will change this in V4.

>
> There is also no udma_from_device_barrier() in this driver, which
> can't be right, please add one around bnxt_re_is_cqe_valid most
> likely.

Ok, I will put it in next version.

>
> Your github branch doesn't compile, btw.
>
> I ran it through sparse, this is what I suggest..

Thanks for the suggestion, I will run sparse and try to fix all of it.

>
> diff --git a/providers/bnxt_re/bnxt_re-abi.h b/providers/bnxt_re/bnxt_re-abi.h
> index 8dbb7b985fa0cd..8fcb2517a23028 100644
> --- a/providers/bnxt_re/bnxt_re-abi.h
> +++ b/providers/bnxt_re/bnxt_re-abi.h
> @@ -243,7 +243,7 @@ struct bnxt_re_req_cqe {
>
>  struct bnxt_re_rc_cqe {
>         __u32 length;
> -       __u32 imm_key;
> +       __be32 imm_key;
>         __u64 qp_handle;
>         __u64 mr_handle;
>  };
> @@ -282,7 +282,7 @@ struct bnxt_re_bsqe {
>  };
>
>  struct bnxt_re_psns {
> -       __u32 opc_spsn;
> +       __le32 opc_spsn;
>         __u32 flg_npsn;
>  };
>
> diff --git a/providers/bnxt_re/verbs.c b/providers/bnxt_re/verbs.c
> index dafe55b74aefe8..3e31845b4d6b47 100644
> --- a/providers/bnxt_re/verbs.c
> +++ b/providers/bnxt_re/verbs.c
> @@ -581,9 +581,10 @@ static int bnxt_re_poll_flush_wcs(struct bnxt_re_queue *que,
>                 wrid = &wridp[head];
>                 if (wrid->psns) {
>                         psns = wrid->psns;
> -                       opcode = (psns->opc_spsn >> BNXT_RE_PSNS_OPCD_SHIFT) &
> -                                 BNXT_RE_PSNS_OPCD_MASK;
> -               }
> +                        opcode = (le32toh(psns->opc_spsn) >>
> +                                  BNXT_RE_PSNS_OPCD_SHIFT) &
> +                                 BNXT_RE_PSNS_OPCD_MASK;
> +                }
>
>                 ibvwc->status = IBV_WC_WR_FLUSH_ERR;
>                 ibvwc->opcode = opcode;
> @@ -1032,10 +1033,11 @@ static void bnxt_re_fill_psns(struct bnxt_re_qp *qp, struct bnxt_re_psns *psns,
>                               uint8_t opcode, uint32_t len)
>  {
>         uint32_t pkt_cnt = 0, nxt_psn;
> +       uint32_t opc_spsn = 0;
>
>         memset(psns, 0, sizeof(*psns));
>         if (qp->qptyp == IBV_QPT_RC) {
> -               psns->opc_spsn = qp->sq_psn & BNXT_RE_PSNS_SPSN_MASK;
> +               opc_spsn = qp->sq_psn & BNXT_RE_PSNS_SPSN_MASK;
>                 pkt_cnt = (len / qp->mtu);
>                 if (len % qp->mtu)
>                         pkt_cnt++;
> @@ -1044,9 +1046,9 @@ static void bnxt_re_fill_psns(struct bnxt_re_qp *qp, struct bnxt_re_psns *psns,
>                 qp->sq_psn = nxt_psn;
>         }
>         opcode = bnxt_re_ibv_wr_to_wc_opcd(opcode);
> -       psns->opc_spsn |= ((opcode & BNXT_RE_PSNS_OPCD_MASK) <<
> -                           BNXT_RE_PSNS_OPCD_SHIFT);
> -       *(uint64_t *)psns = htole64(*(uint64_t *)psns);
> +        opc_spsn |=
> +            ((opcode & BNXT_RE_PSNS_OPCD_MASK) << BNXT_RE_PSNS_OPCD_SHIFT);
> +        psns->opc_spsn = htole32(opc_spsn);
>  }
>
>  static void bnxt_re_fill_wrid(struct bnxt_re_wrid *wrid, struct ibv_send_wr *wr,
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe March 16, 2017, 4:17 p.m. UTC | #2 
On Thu, Mar 16, 2017 at 08:34:12AM +0530, Devesh Sharma wrote:

> > Your github branch doesn't compile, btw.
> >
> > I ran it through sparse, this is what I suggest..
> 
> Thanks for the suggestion, I will run sparse and try to fix all of it.

You will need my sparse branch to do this:

https://github.com/linux-rdma/rdma-core/pull/100

There were a few places still producing warnings, at least two should
be fixed with __force

You should make a PR for this once you get Leon's changes and rebase
it the current master.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Devesh Sharma March 16, 2017, 4:53 p.m. UTC | #3 
Okay, I will get it done.

On Thu, Mar 16, 2017 at 9:47 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Thu, Mar 16, 2017 at 08:34:12AM +0530, Devesh Sharma wrote:
>
>> > Your github branch doesn't compile, btw.
>> >
>> > I ran it through sparse, this is what I suggest..
>>
>> Thanks for the suggestion, I will run sparse and try to fix all of it.
>
> You will need my sparse branch to do this:
>
> https://github.com/linux-rdma/rdma-core/pull/100
>
> There were a few places still producing warnings, at least two should
> be fixed with __force
>
> You should make a PR for this once you get Leon's changes and rebase
> it the current master.
>
> Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/providers/bnxt_re/bnxt_re-abi.h b/providers/bnxt_re/bnxt_re-abi.h
index 8dbb7b985fa0cd..8fcb2517a23028 100644
--- a/providers/bnxt_re/bnxt_re-abi.h
+++ b/providers/bnxt_re/bnxt_re-abi.h
@@ -243,7 +243,7 @@  struct bnxt_re_req_cqe {
 
 struct bnxt_re_rc_cqe {
 	__u32 length;
-	__u32 imm_key;
+	__be32 imm_key;
 	__u64 qp_handle;
 	__u64 mr_handle;
 };
@@ -282,7 +282,7 @@  struct bnxt_re_bsqe {
 };
 
 struct bnxt_re_psns {
-	__u32 opc_spsn;
+	__le32 opc_spsn;
 	__u32 flg_npsn;
 };
 
diff --git a/providers/bnxt_re/verbs.c b/providers/bnxt_re/verbs.c
index dafe55b74aefe8..3e31845b4d6b47 100644
--- a/providers/bnxt_re/verbs.c
+++ b/providers/bnxt_re/verbs.c
@@ -581,9 +581,10 @@  static int bnxt_re_poll_flush_wcs(struct bnxt_re_queue *que,
 		wrid = &wridp[head];
 		if (wrid->psns) {
 			psns = wrid->psns;
-			opcode = (psns->opc_spsn >> BNXT_RE_PSNS_OPCD_SHIFT) &
-				  BNXT_RE_PSNS_OPCD_MASK;
-		}
+                        opcode = (le32toh(psns->opc_spsn) >>
+                                  BNXT_RE_PSNS_OPCD_SHIFT) &
+                                 BNXT_RE_PSNS_OPCD_MASK;
+                }
 
 		ibvwc->status = IBV_WC_WR_FLUSH_ERR;
 		ibvwc->opcode = opcode;
@@ -1032,10 +1033,11 @@  static void bnxt_re_fill_psns(struct bnxt_re_qp *qp, struct bnxt_re_psns *psns,
 			      uint8_t opcode, uint32_t len)
 {
 	uint32_t pkt_cnt = 0, nxt_psn;
+	uint32_t opc_spsn = 0;
 
 	memset(psns, 0, sizeof(*psns));
 	if (qp->qptyp == IBV_QPT_RC) {
-		psns->opc_spsn = qp->sq_psn & BNXT_RE_PSNS_SPSN_MASK;
+		opc_spsn = qp->sq_psn & BNXT_RE_PSNS_SPSN_MASK;
 		pkt_cnt = (len / qp->mtu);
 		if (len % qp->mtu)
 			pkt_cnt++;
@@ -1044,9 +1046,9 @@  static void bnxt_re_fill_psns(struct bnxt_re_qp *qp, struct bnxt_re_psns *psns,
 		qp->sq_psn = nxt_psn;
 	}
 	opcode = bnxt_re_ibv_wr_to_wc_opcd(opcode);
-	psns->opc_spsn |= ((opcode & BNXT_RE_PSNS_OPCD_MASK) <<
-			    BNXT_RE_PSNS_OPCD_SHIFT);
-	*(uint64_t *)psns = htole64(*(uint64_t *)psns);
+        opc_spsn |=
+            ((opcode & BNXT_RE_PSNS_OPCD_MASK) << BNXT_RE_PSNS_OPCD_SHIFT);
+        psns->opc_spsn = htole32(opc_spsn);
 }
 
 static void bnxt_re_fill_wrid(struct bnxt_re_wrid *wrid, struct ibv_send_wr *wr,