[PATCH net-next v2 0/2] rework the memory barrier for SCRQ entry

[PATCH net-next v2 0/2] rework the memory barrier for SCRQ entry

[Lijun Pan]

This series rework the memory barrier for SCRQ (Sub-Command-Response
Queue) entry.

v2: send to net-next.

Lijun Pan (2):
  ibmvnic: rework to ensure SCRQ entry reads are properly ordered
  ibmvnic: remove unnecessary rmb() inside ibmvnic_poll

 drivers/net/ethernet/ibm/ibmvnic.c | 31 +++++++++++-------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

-- 
2.23.0

[PATCH net-next v2 1/2] ibmvnic: rework to ensure SCRQ entry reads are properly ordered

[Lijun Pan]

Move the dma_rmb() between pending_scrq() and ibmvnic_next_scrq()
into the end of pending_scrq() to save the duplicated code since
this dma_rmb will be used 3 times.

Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
Acked-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 30 +++++++++++-------------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index a40af510018a..331ebca2f57a 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2444,12 +2444,6 @@ static int ibmvnic_poll(struct napi_struct *napi, int budget)
 
 		if (!pending_scrq(adapter, rx_scrq))
 			break;
-		/* The queue entry at the current index is peeked at above
-		 * to determine that there is a valid descriptor awaiting
-		 * processing. We want to be sure that the current slot
-		 * holds a valid descriptor before reading its contents.
-		 */
-		dma_rmb();
 		next = ibmvnic_next_scrq(adapter, rx_scrq);
 		rx_buff =
 		    (struct ibmvnic_rx_buff *)be64_to_cpu(next->
@@ -3189,13 +3183,6 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter *adapter,
 		int total_bytes = 0;
 		int num_packets = 0;
 
-		/* The queue entry at the current index is peeked at above
-		 * to determine that there is a valid descriptor awaiting
-		 * processing. We want to be sure that the current slot
-		 * holds a valid descriptor before reading its contents.
-		 */
-		dma_rmb();
-
 		next = ibmvnic_next_scrq(adapter, scrq);
 		for (i = 0; i < next->tx_comp.num_comps; i++) {
 			if (next->tx_comp.rcs[i])
@@ -3569,11 +3556,16 @@ static int pending_scrq(struct ibmvnic_adapter *adapter,
 			struct ibmvnic_sub_crq_queue *scrq)
 {
 	union sub_crq *entry = &scrq->msgs[scrq->cur];
+	int rc;
 
-	if (entry->generic.first & IBMVNIC_CRQ_CMD_RSP)
-		return 1;
-	else
-		return 0;
+	rc = !!(entry->generic.first & IBMVNIC_CRQ_CMD_RSP);
+
+	/* Ensure that the SCRQ valid flag is loaded prior to loading the
+	 * contents of the SCRQ descriptor
+	 */
+	dma_rmb();
+
+	return rc;
 }
 
 static union sub_crq *ibmvnic_next_scrq(struct ibmvnic_adapter *adapter,
@@ -3592,8 +3584,8 @@ static union sub_crq *ibmvnic_next_scrq(struct ibmvnic_adapter *adapter,
 	}
 	spin_unlock_irqrestore(&scrq->lock, flags);
 
-	/* Ensure that the entire buffer descriptor has been
-	 * loaded before reading its contents
+	/* Ensure that the SCRQ valid flag is loaded prior to loading the
+	 * contents of the SCRQ descriptor
 	 */
 	dma_rmb();
 
-- 
2.23.0

[PATCH net-next v2 2/2] ibmvnic: remove unnecessary rmb() inside ibmvnic_poll

[Lijun Pan]

rmb() can be removed since:
1. pending_scrq() has dma_rmb() at the function end;
2. dma_rmb(), though weaker, is enough here.

Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
Acked-by: Dwip Banerjee <dnbanerg@us.ibm.com>
Acked-by: Thomas Falcon <tlfalcon@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 331ebca2f57a..0ed169ef1cfc 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2510,7 +2510,6 @@ static int ibmvnic_poll(struct napi_struct *napi, int budget)
 		if (napi_complete_done(napi, frames_processed)) {
 			enable_scrq_irq(adapter, rx_scrq);
 			if (pending_scrq(adapter, rx_scrq)) {
-				rmb();
 				if (napi_reschedule(napi)) {
 					disable_scrq_irq(adapter, rx_scrq);
 					goto restart_poll;
-- 
2.23.0

Re: [PATCH net-next v2 0/2] rework the memory barrier for SCRQ entry

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri, 29 Jan 2021 19:19:03 -0600 you wrote:
> This series rework the memory barrier for SCRQ (Sub-Command-Response
> Queue) entry.
> 
> v2: send to net-next.
> 
> Lijun Pan (2):
>   ibmvnic: rework to ensure SCRQ entry reads are properly ordered
>   ibmvnic: remove unnecessary rmb() inside ibmvnic_poll
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/2] ibmvnic: rework to ensure SCRQ entry reads are properly ordered
    https://git.kernel.org/netdev/net-next/c/665ab1eb18d7
  - [net-next,v2,2/2] ibmvnic: remove unnecessary rmb() inside ibmvnic_poll
    https://git.kernel.org/netdev/net-next/c/2719cb445da5

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html