RE: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-04-04 Thread Kalderon, Michal 
> From: Jason Gunthorpe [mailto:j...@ziepe.ca]
> Sent: Tuesday, April 03, 2018 11:04 PM
> 
> On Tue, Apr 03, 2018 at 07:42:28AM +, Kalderon, Michal wrote:
> > > From: Sinan Kaya [mailto:ok...@codeaurora.org]
> > > Sent: Tuesday, April 03, 2018 5:30 AM
> > > To: linux-r...@vger.kernel.org; ti...@codeaurora.org;
> > > sulr...@codeaurora.org
> > > Cc: linux-arm-...@vger.kernel.org;
> > > linux-arm-ker...@lists.infradead.org;
> > > Kalderon, Michal <michal.kalde...@cavium.com>; Elior, Ariel
> > > <ariel.el...@cavium.com>; Doug Ledford <dledf...@redhat.com>; Jason
> > > Gunthorpe <j...@ziepe.ca>; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers
> > > on weakly-ordered archs #2
> > >
> > > On 3/22/2018 12:26 PM, Sinan Kaya wrote:
> > > > @@ -860,7 +860,7 @@ static void doorbell_cq(struct qedr_cq *cq,
> > > > u32
> > > cons, u8 flags)
> > > > wmb();
> > > > cq->db.data.agg_flags = flags;
> > > > cq->db.data.value = cpu_to_le32(cons);
> > > > -   writeq(cq->db.raw, cq->db_addr);
> > > > +   writeq_relaxed(cq->db.raw, cq->db_addr);
> > >
> > > Given the direction to get rid of wmb() in front of writeX()
> > > functions, I have been reviewing this code. Under normal
> > > circumstances, I can get rid of all
> > > wmb() as follows.
> > >
> > > However, I started having my doubts now. Are these wmb() used as a
> > > SMP barrier too?
> > > I can't find any smp_Xmb() in drivers/infiniband/hw/qedr directory.
> >
> > Your doubts are in place. You initial patch series modified writel to
> > writel_relaxed Simply removing the wmb is dangerous. The wmb before
> > writel are used to make sure the HW observes the changes in memory
> > before we trigger the doorbell. Smp barriers here wouldn't suffice, as
> > on a single processor. we still need to make sure memory is updated and
> not remained in cache when HW accesses it.
> > Reviewing the qedr barriers, I can find places where this may have not
> > been necessary, But definitely you can't simply remove this wmb barriers.
> 
> As Sinan said, the consensus is that wmb();writel(); is redundant if the only
> purpose of the wmb is to order DMA and system memory.
> 
> So can you review these patches on that basis please? Is the WMB doing
> something else, eg SMP related? If yes, please send a patch adding
> appropriate comments.

Thanks Sinan and Jason for the references and explanations, I've reviewed the 
wmb
usages in qedr and am about to send a patch that replaces two of them with 
smp_wmb
and completely removes two of them that given your explanation, turned out to 
be redundant,
thanks.

> 
> Thanks,
> Jason

RE: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-04-04 Thread Kalderon, Michal 
> From: Jason Gunthorpe [mailto:j...@ziepe.ca]
> Sent: Tuesday, April 03, 2018 11:04 PM
> 
> On Tue, Apr 03, 2018 at 07:42:28AM +, Kalderon, Michal wrote:
> > > From: Sinan Kaya [mailto:ok...@codeaurora.org]
> > > Sent: Tuesday, April 03, 2018 5:30 AM
> > > To: linux-r...@vger.kernel.org; ti...@codeaurora.org;
> > > sulr...@codeaurora.org
> > > Cc: linux-arm-...@vger.kernel.org;
> > > linux-arm-ker...@lists.infradead.org;
> > > Kalderon, Michal ; Elior, Ariel
> > > ; Doug Ledford ; Jason
> > > Gunthorpe ; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers
> > > on weakly-ordered archs #2
> > >
> > > On 3/22/2018 12:26 PM, Sinan Kaya wrote:
> > > > @@ -860,7 +860,7 @@ static void doorbell_cq(struct qedr_cq *cq,
> > > > u32
> > > cons, u8 flags)
> > > > wmb();
> > > > cq->db.data.agg_flags = flags;
> > > > cq->db.data.value = cpu_to_le32(cons);
> > > > -   writeq(cq->db.raw, cq->db_addr);
> > > > +   writeq_relaxed(cq->db.raw, cq->db_addr);
> > >
> > > Given the direction to get rid of wmb() in front of writeX()
> > > functions, I have been reviewing this code. Under normal
> > > circumstances, I can get rid of all
> > > wmb() as follows.
> > >
> > > However, I started having my doubts now. Are these wmb() used as a
> > > SMP barrier too?
> > > I can't find any smp_Xmb() in drivers/infiniband/hw/qedr directory.
> >
> > Your doubts are in place. You initial patch series modified writel to
> > writel_relaxed Simply removing the wmb is dangerous. The wmb before
> > writel are used to make sure the HW observes the changes in memory
> > before we trigger the doorbell. Smp barriers here wouldn't suffice, as
> > on a single processor. we still need to make sure memory is updated and
> not remained in cache when HW accesses it.
> > Reviewing the qedr barriers, I can find places where this may have not
> > been necessary, But definitely you can't simply remove this wmb barriers.
> 
> As Sinan said, the consensus is that wmb();writel(); is redundant if the only
> purpose of the wmb is to order DMA and system memory.
> 
> So can you review these patches on that basis please? Is the WMB doing
> something else, eg SMP related? If yes, please send a patch adding
> appropriate comments.

Thanks Sinan and Jason for the references and explanations, I've reviewed the 
wmb
usages in qedr and am about to send a patch that replaces two of them with 
smp_wmb
and completely removes two of them that given your explanation, turned out to 
be redundant,
thanks.

> 
> Thanks,
> Jason

Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-04-03 Thread Jason Gunthorpe 
On Tue, Apr 03, 2018 at 07:42:28AM +, Kalderon, Michal wrote:
> > From: Sinan Kaya [mailto:ok...@codeaurora.org]
> > Sent: Tuesday, April 03, 2018 5:30 AM
> > To: linux-r...@vger.kernel.org; ti...@codeaurora.org;
> > sulr...@codeaurora.org
> > Cc: linux-arm-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> > Kalderon, Michal <michal.kalde...@cavium.com>; Elior, Ariel
> > <ariel.el...@cavium.com>; Doug Ledford <dledf...@redhat.com>; Jason
> > Gunthorpe <j...@ziepe.ca>; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on
> > weakly-ordered archs #2
> > 
> > On 3/22/2018 12:26 PM, Sinan Kaya wrote:
> > > @@ -860,7 +860,7 @@ static void doorbell_cq(struct qedr_cq *cq, u32
> > cons, u8 flags)
> > >   wmb();
> > >   cq->db.data.agg_flags = flags;
> > >   cq->db.data.value = cpu_to_le32(cons);
> > > - writeq(cq->db.raw, cq->db_addr);
> > > + writeq_relaxed(cq->db.raw, cq->db_addr);
> > 
> > Given the direction to get rid of wmb() in front of writeX() functions, I 
> > have
> > been reviewing this code. Under normal circumstances, I can get rid of all
> > wmb() as follows.
> > 
> > However, I started having my doubts now. Are these wmb() used as a SMP
> > barrier too?
> > I can't find any smp_Xmb() in drivers/infiniband/hw/qedr directory.
> 
> Your doubts are in place. You initial patch series modified writel to 
> writel_relaxed
> Simply removing the wmb is dangerous. The wmb before writel are used to make 
> sure the
> HW observes the changes in memory before we trigger the doorbell. Smp 
> barriers here
> wouldn't suffice, as on a single processor. we still need to make sure memory 
> is updated
> and not remained in cache when HW accesses it.
> Reviewing the qedr barriers, I can find places where this may have not been 
> necessary, 
> But definitely you can't simply remove this wmb barriers. 

As Sinan said, the consensus is that wmb();writel(); is redundant if
the only purpose of the wmb is to order DMA and system memory.

So can you review these patches on that basis please? Is the WMB doing
something else, eg SMP related? If yes, please send a patch adding
appropriate comments.

Thanks,
Jason

Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-04-03 Thread Jason Gunthorpe 
On Tue, Apr 03, 2018 at 07:42:28AM +, Kalderon, Michal wrote:
> > From: Sinan Kaya [mailto:ok...@codeaurora.org]
> > Sent: Tuesday, April 03, 2018 5:30 AM
> > To: linux-r...@vger.kernel.org; ti...@codeaurora.org;
> > sulr...@codeaurora.org
> > Cc: linux-arm-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> > Kalderon, Michal ; Elior, Ariel
> > ; Doug Ledford ; Jason
> > Gunthorpe ; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on
> > weakly-ordered archs #2
> > 
> > On 3/22/2018 12:26 PM, Sinan Kaya wrote:
> > > @@ -860,7 +860,7 @@ static void doorbell_cq(struct qedr_cq *cq, u32
> > cons, u8 flags)
> > >   wmb();
> > >   cq->db.data.agg_flags = flags;
> > >   cq->db.data.value = cpu_to_le32(cons);
> > > - writeq(cq->db.raw, cq->db_addr);
> > > + writeq_relaxed(cq->db.raw, cq->db_addr);
> > 
> > Given the direction to get rid of wmb() in front of writeX() functions, I 
> > have
> > been reviewing this code. Under normal circumstances, I can get rid of all
> > wmb() as follows.
> > 
> > However, I started having my doubts now. Are these wmb() used as a SMP
> > barrier too?
> > I can't find any smp_Xmb() in drivers/infiniband/hw/qedr directory.
> 
> Your doubts are in place. You initial patch series modified writel to 
> writel_relaxed
> Simply removing the wmb is dangerous. The wmb before writel are used to make 
> sure the
> HW observes the changes in memory before we trigger the doorbell. Smp 
> barriers here
> wouldn't suffice, as on a single processor. we still need to make sure memory 
> is updated
> and not remained in cache when HW accesses it.
> Reviewing the qedr barriers, I can find places where this may have not been 
> necessary, 
> But definitely you can't simply remove this wmb barriers. 

As Sinan said, the consensus is that wmb();writel(); is redundant if
the only purpose of the wmb is to order DMA and system memory.

So can you review these patches on that basis please? Is the WMB doing
something else, eg SMP related? If yes, please send a patch adding
appropriate comments.

Thanks,
Jason

Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-04-03 Thread Sinan Kaya 
On 4/3/2018 3:42 AM, Kalderon, Michal wrote:
> The wmb before writel are used to make sure the
> HW observes the changes in memory before we trigger the doorbell. 

According to Linus, writel() guarantees observability. No extra
barrier is necessary.

https://www.mail-archive.com/netdev@vger.kernel.org/msg225806.html

There shouldn't be any wmb() in drivers unless it is used for a
very well-known reason. 

APIs like readX() and writeX() guarantee observability.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-04-03 Thread Sinan Kaya 
On 4/3/2018 3:42 AM, Kalderon, Michal wrote:
> The wmb before writel are used to make sure the
> HW observes the changes in memory before we trigger the doorbell. 

According to Linus, writel() guarantees observability. No extra
barrier is necessary.

https://www.mail-archive.com/netdev@vger.kernel.org/msg225806.html

There shouldn't be any wmb() in drivers unless it is used for a
very well-known reason. 

APIs like readX() and writeX() guarantee observability.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

RE: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-04-03 Thread Kalderon, Michal 
> From: Sinan Kaya [mailto:ok...@codeaurora.org]
> Sent: Tuesday, April 03, 2018 5:30 AM
> To: linux-r...@vger.kernel.org; ti...@codeaurora.org;
> sulr...@codeaurora.org
> Cc: linux-arm-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> Kalderon, Michal <michal.kalde...@cavium.com>; Elior, Ariel
> <ariel.el...@cavium.com>; Doug Ledford <dledf...@redhat.com>; Jason
> Gunthorpe <j...@ziepe.ca>; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on
> weakly-ordered archs #2
> 
> On 3/22/2018 12:26 PM, Sinan Kaya wrote:
> > @@ -860,7 +860,7 @@ static void doorbell_cq(struct qedr_cq *cq, u32
> cons, u8 flags)
> > wmb();
> > cq->db.data.agg_flags = flags;
> > cq->db.data.value = cpu_to_le32(cons);
> > -   writeq(cq->db.raw, cq->db_addr);
> > +   writeq_relaxed(cq->db.raw, cq->db_addr);
> 
> Given the direction to get rid of wmb() in front of writeX() functions, I have
> been reviewing this code. Under normal circumstances, I can get rid of all
> wmb() as follows.
> 
> However, I started having my doubts now. Are these wmb() used as a SMP
> barrier too?
> I can't find any smp_Xmb() in drivers/infiniband/hw/qedr directory.

Your doubts are in place. You initial patch series modified writel to 
writel_relaxed
Simply removing the wmb is dangerous. The wmb before writel are used to make 
sure the
HW observes the changes in memory before we trigger the doorbell. Smp barriers 
here
wouldn't suffice, as on a single processor. we still need to make sure memory 
is updated
and not remained in cache when HW accesses it.
Reviewing the qedr barriers, I can find places where this may have not been 
necessary, 
But definitely you can't simply remove this wmb barriers. 

> 
> static void doorbell_cq(struct qedr_cq *cq, u32 cons, u8 flags)  {
> -   /* Flush data before signalling doorbell */
> -   wmb();
> cq->db.data.agg_flags = flags;
> cq->db.data.value = cpu_to_le32(cons);
> writeq(cq->db.raw, cq->db_addr); @@ -1870,8 +1868,7 @@ static int
> qedr_update_qp_state(struct qedr_dev *dev,
>  */
> 
> if (rdma_protocol_roce(>ibdev, 1)) {
> -   wmb();
> -   writel_relaxed(qp->rq.db_data.raw, qp->rq.db);
> +   writel(qp->rq.db_data.raw, qp->rq.db);
> /* Make sure write takes effect */
> mmiowb();
> }
> @@ -3275,8 +3272,7 @@ int qedr_post_send(struct ib_qp *ibqp, struct
> ib_send_wr *wr,
>  * unchanged). For performance reasons we avoid checking for this
>  * redundant doorbell.
>  */
> -   wmb();
> -   writel_relaxed(qp->sq.db_data.raw, qp->sq.db);
> +   writel(qp->sq.db_data.raw, qp->sq.db);
> 
> /* Make sure write sticks */
> mmiowb();
> @@ -3362,9 +3358,6 @@ int qedr_post_recv(struct ib_qp *ibqp, struct
> ib_recv_wr *wr,
> 
> qedr_inc_sw_prod(>rq);
> 
> -   /* Flush all the writes before signalling doorbell */
> -   wmb();
> 
> 
> 
> 
> 
> --
> Sinan Kaya
> Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
> Technologies, Inc.
> Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux
> Foundation Collaborative Project.

RE: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-04-03 Thread Kalderon, Michal 
> From: Sinan Kaya [mailto:ok...@codeaurora.org]
> Sent: Tuesday, April 03, 2018 5:30 AM
> To: linux-r...@vger.kernel.org; ti...@codeaurora.org;
> sulr...@codeaurora.org
> Cc: linux-arm-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> Kalderon, Michal ; Elior, Ariel
> ; Doug Ledford ; Jason
> Gunthorpe ; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on
> weakly-ordered archs #2
> 
> On 3/22/2018 12:26 PM, Sinan Kaya wrote:
> > @@ -860,7 +860,7 @@ static void doorbell_cq(struct qedr_cq *cq, u32
> cons, u8 flags)
> > wmb();
> > cq->db.data.agg_flags = flags;
> > cq->db.data.value = cpu_to_le32(cons);
> > -   writeq(cq->db.raw, cq->db_addr);
> > +   writeq_relaxed(cq->db.raw, cq->db_addr);
> 
> Given the direction to get rid of wmb() in front of writeX() functions, I have
> been reviewing this code. Under normal circumstances, I can get rid of all
> wmb() as follows.
> 
> However, I started having my doubts now. Are these wmb() used as a SMP
> barrier too?
> I can't find any smp_Xmb() in drivers/infiniband/hw/qedr directory.

Your doubts are in place. You initial patch series modified writel to 
writel_relaxed
Simply removing the wmb is dangerous. The wmb before writel are used to make 
sure the
HW observes the changes in memory before we trigger the doorbell. Smp barriers 
here
wouldn't suffice, as on a single processor. we still need to make sure memory 
is updated
and not remained in cache when HW accesses it.
Reviewing the qedr barriers, I can find places where this may have not been 
necessary, 
But definitely you can't simply remove this wmb barriers. 

> 
> static void doorbell_cq(struct qedr_cq *cq, u32 cons, u8 flags)  {
> -   /* Flush data before signalling doorbell */
> -   wmb();
> cq->db.data.agg_flags = flags;
> cq->db.data.value = cpu_to_le32(cons);
> writeq(cq->db.raw, cq->db_addr); @@ -1870,8 +1868,7 @@ static int
> qedr_update_qp_state(struct qedr_dev *dev,
>  */
> 
> if (rdma_protocol_roce(>ibdev, 1)) {
> -   wmb();
> -   writel_relaxed(qp->rq.db_data.raw, qp->rq.db);
> +   writel(qp->rq.db_data.raw, qp->rq.db);
> /* Make sure write takes effect */
> mmiowb();
> }
> @@ -3275,8 +3272,7 @@ int qedr_post_send(struct ib_qp *ibqp, struct
> ib_send_wr *wr,
>  * unchanged). For performance reasons we avoid checking for this
>  * redundant doorbell.
>  */
> -   wmb();
> -   writel_relaxed(qp->sq.db_data.raw, qp->sq.db);
> +   writel(qp->sq.db_data.raw, qp->sq.db);
> 
> /* Make sure write sticks */
> mmiowb();
> @@ -3362,9 +3358,6 @@ int qedr_post_recv(struct ib_qp *ibqp, struct
> ib_recv_wr *wr,
> 
> qedr_inc_sw_prod(>rq);
> 
> -   /* Flush all the writes before signalling doorbell */
> -   wmb();
> 
> 
> 
> 
> 
> --
> Sinan Kaya
> Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
> Technologies, Inc.
> Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux
> Foundation Collaborative Project.

Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-04-02 Thread Sinan Kaya 
On 3/22/2018 12:26 PM, Sinan Kaya wrote:
> @@ -860,7 +860,7 @@ static void doorbell_cq(struct qedr_cq *cq, u32 cons, u8 
> flags)
>   wmb();
>   cq->db.data.agg_flags = flags;
>   cq->db.data.value = cpu_to_le32(cons);
> - writeq(cq->db.raw, cq->db_addr);
> + writeq_relaxed(cq->db.raw, cq->db_addr);

Given the direction to get rid of wmb() in front of writeX() functions, I have 
been
reviewing this code. Under normal circumstances, I can get rid of all wmb() as 
follows.

However, I started having my doubts now. Are these wmb() used as a SMP barrier 
too?
I can't find any smp_Xmb() in drivers/infiniband/hw/qedr directory.

static void doorbell_cq(struct qedr_cq *cq, u32 cons, u8 flags)
 {
-   /* Flush data before signalling doorbell */
-   wmb();
cq->db.data.agg_flags = flags;
cq->db.data.value = cpu_to_le32(cons);
writeq(cq->db.raw, cq->db_addr);
@@ -1870,8 +1868,7 @@ static int qedr_update_qp_state(struct qedr_dev *dev,
 */

if (rdma_protocol_roce(>ibdev, 1)) {
-   wmb();
-   writel_relaxed(qp->rq.db_data.raw, qp->rq.db);
+   writel(qp->rq.db_data.raw, qp->rq.db);
/* Make sure write takes effect */
mmiowb();
}
@@ -3275,8 +3272,7 @@ int qedr_post_send(struct ib_qp *ibqp, struct ib_send_wr 
*wr,
 * unchanged). For performance reasons we avoid checking for this
 * redundant doorbell.
 */
-   wmb();
-   writel_relaxed(qp->sq.db_data.raw, qp->sq.db);
+   writel(qp->sq.db_data.raw, qp->sq.db);

/* Make sure write sticks */
mmiowb();
@@ -3362,9 +3358,6 @@ int qedr_post_recv(struct ib_qp *ibqp, struct ib_recv_wr 
*wr,

qedr_inc_sw_prod(>rq);

-   /* Flush all the writes before signalling doorbell */
-   wmb();





-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

Re: [PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-04-02 Thread Sinan Kaya 
On 3/22/2018 12:26 PM, Sinan Kaya wrote:
> @@ -860,7 +860,7 @@ static void doorbell_cq(struct qedr_cq *cq, u32 cons, u8 
> flags)
>   wmb();
>   cq->db.data.agg_flags = flags;
>   cq->db.data.value = cpu_to_le32(cons);
> - writeq(cq->db.raw, cq->db_addr);
> + writeq_relaxed(cq->db.raw, cq->db_addr);

Given the direction to get rid of wmb() in front of writeX() functions, I have 
been
reviewing this code. Under normal circumstances, I can get rid of all wmb() as 
follows.

However, I started having my doubts now. Are these wmb() used as a SMP barrier 
too?
I can't find any smp_Xmb() in drivers/infiniband/hw/qedr directory.

static void doorbell_cq(struct qedr_cq *cq, u32 cons, u8 flags)
 {
-   /* Flush data before signalling doorbell */
-   wmb();
cq->db.data.agg_flags = flags;
cq->db.data.value = cpu_to_le32(cons);
writeq(cq->db.raw, cq->db_addr);
@@ -1870,8 +1868,7 @@ static int qedr_update_qp_state(struct qedr_dev *dev,
 */

if (rdma_protocol_roce(>ibdev, 1)) {
-   wmb();
-   writel_relaxed(qp->rq.db_data.raw, qp->rq.db);
+   writel(qp->rq.db_data.raw, qp->rq.db);
/* Make sure write takes effect */
mmiowb();
}
@@ -3275,8 +3272,7 @@ int qedr_post_send(struct ib_qp *ibqp, struct ib_send_wr 
*wr,
 * unchanged). For performance reasons we avoid checking for this
 * redundant doorbell.
 */
-   wmb();
-   writel_relaxed(qp->sq.db_data.raw, qp->sq.db);
+   writel(qp->sq.db_data.raw, qp->sq.db);

/* Make sure write sticks */
mmiowb();
@@ -3362,9 +3358,6 @@ int qedr_post_recv(struct ib_qp *ibqp, struct ib_recv_wr 
*wr,

qedr_inc_sw_prod(>rq);

-   /* Flush all the writes before signalling doorbell */
-   wmb();





-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

[PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-03-22 Thread Sinan Kaya 
Code includes wmb() followed by writel() in multiple places. writel()
already has a barrier on some architectures like arm64.

This ends up CPU observing two barriers back to back before executing the
register write.

Since code already has an explicit barrier call, changing writel() to
writel_relaxed().

Signed-off-by: Sinan Kaya 
---
 drivers/infiniband/hw/qedr/verbs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/qedr/verbs.c 
b/drivers/infiniband/hw/qedr/verbs.c
index 53f00db..d6bd950 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -860,7 +860,7 @@ static void doorbell_cq(struct qedr_cq *cq, u32 cons, u8 
flags)
wmb();
cq->db.data.agg_flags = flags;
cq->db.data.value = cpu_to_le32(cons);
-   writeq(cq->db.raw, cq->db_addr);
+   writeq_relaxed(cq->db.raw, cq->db_addr);
 
/* Make sure write would stick */
mmiowb();
@@ -3338,7 +3338,7 @@ int qedr_post_recv(struct ib_qp *ibqp, struct ib_recv_wr 
*wr,
 
qp->rq.db_data.data.value++;
 
-   writel(qp->rq.db_data.raw, qp->rq.db);
+   writel_relaxed(qp->rq.db_data.raw, qp->rq.db);
 
/* Make sure write sticks */
mmiowb();
-- 
2.7.4

[PATCH v5 3/3] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs #2

2018-03-22 Thread Sinan Kaya 
Code includes wmb() followed by writel() in multiple places. writel()
already has a barrier on some architectures like arm64.

This ends up CPU observing two barriers back to back before executing the
register write.

Since code already has an explicit barrier call, changing writel() to
writel_relaxed().

Signed-off-by: Sinan Kaya 
---
 drivers/infiniband/hw/qedr/verbs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/qedr/verbs.c 
b/drivers/infiniband/hw/qedr/verbs.c
index 53f00db..d6bd950 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -860,7 +860,7 @@ static void doorbell_cq(struct qedr_cq *cq, u32 cons, u8 
flags)
wmb();
cq->db.data.agg_flags = flags;
cq->db.data.value = cpu_to_le32(cons);
-   writeq(cq->db.raw, cq->db_addr);
+   writeq_relaxed(cq->db.raw, cq->db_addr);
 
/* Make sure write would stick */
mmiowb();
@@ -3338,7 +3338,7 @@ int qedr_post_recv(struct ib_qp *ibqp, struct ib_recv_wr 
*wr,
 
qp->rq.db_data.data.value++;
 
-   writel(qp->rq.db_data.raw, qp->rq.db);
+   writel_relaxed(qp->rq.db_data.raw, qp->rq.db);
 
/* Make sure write sticks */
mmiowb();
-- 
2.7.4