From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from BLU004-OMC4S10.hotmail.com (blu004-omc4s10.hotmail.com [65.55.111.149]) by dpdk.org (Postfix) with ESMTP id 7A753C5E0 for ; Sun, 28 Jun 2015 17:23:40 +0200 (CEST) Received: from BLU436-SMTP81 ([65.55.111.137]) by BLU004-OMC4S10.hotmail.com over TLS secured channel with Microsoft SMTPSVC(7.5.7601.22751); Sun, 28 Jun 2015 08:23:40 -0700 X-TMN: [pU404fI5BcfcAt4qUUoa+E6JaiyqGAJP] X-Originating-Email: [dong.wang.pro@hotmail.com] Message-ID: From: WangDong To: dev@dpdk.org Date: Sun, 28 Jun 2015 23:23:18 +0800 X-Mailer: git-send-email 2.1.0 In-Reply-To: <1435504998-15566-1-git-send-email-dong.wang.pro@hotmail.com> References: <1435504998-15566-1-git-send-email-dong.wang.pro@hotmail.com> X-OriginalArrivalTime: 28 Jun 2015 15:23:39.0159 (UTC) FILETIME=[68A59270:01D0B1B6] MIME-Version: 1.0 Content-Type: text/plain Subject: [dpdk-dev] [PATCH 2/2] ixgbe:replace compiler memory barrier and rte_wmb with rte_dma_rmb and rte_dma_wmb. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 28 Jun 2015 15:23:40 -0000 --- drivers/net/ixgbe/ixgbe_rxtx.c | 30 +++++++++--------------------- drivers/net/ixgbe/ixgbe_rxtx_vec.c | 3 +++ 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c index 3ace8a8..3316488 100644 --- a/drivers/net/ixgbe/ixgbe_rxtx.c +++ b/drivers/net/ixgbe/ixgbe_rxtx.c @@ -130,6 +130,7 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq) /* check DD bit on threshold descriptor */ status = txq->tx_ring[txq->tx_next_dd].wb.status; + rte_dma_rmb(); if (! (status & IXGBE_ADVTXD_STAT_DD)) return 0; @@ -320,7 +321,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, txq->tx_tail = 0; /* update tail pointer */ - rte_wmb(); + rte_dma_wmb(); IXGBE_PCI_REG_WRITE(txq->tdt_reg_addr, txq->tx_tail); return nb_pkts; @@ -841,7 +842,6 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, txd->read.cmd_type_len |= rte_cpu_to_le_32(cmd_type_len); } end_of_tx: - rte_wmb(); /* * Set the Transmit Descriptor Tail (TDT) @@ -849,6 +849,7 @@ end_of_tx: PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u", (unsigned) txq->port_id, (unsigned) txq->queue_id, (unsigned) tx_id, (unsigned) nb_tx); + rte_dma_wmb(); IXGBE_PCI_REG_WRITE(txq->tdt_reg_addr, tx_id); txq->tx_tail = tx_id; @@ -975,6 +976,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq) /* Compute how many status bits were set */ nb_dd = 0; + rte_dma_rmb(); for (j = 0; j < LOOK_AHEAD; ++j) nb_dd += s[j] & IXGBE_RXDADV_STAT_DD; @@ -1138,7 +1140,7 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, } /* update tail pointer */ - rte_wmb(); + rte_dma_wmb(); IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, cur_free_trigger); } @@ -1229,13 +1231,10 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, /* * The order of operations here is important as the DD status * bit must not be read after any other descriptor fields. - * rx_ring and rxdp are pointing to volatile data so the order - * of accesses cannot be reordered by the compiler. If they were - * not volatile, they could be reordered which could lead to - * using invalid descriptor fields when read from rxd. */ rxdp = &rx_ring[rx_id]; staterr = rxdp->wb.upper.status_error; + rte_dma_rmb(); if (! (staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD))) break; rxd = *rxdp; @@ -1373,6 +1372,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, (unsigned) nb_rx); rx_id = (uint16_t) ((rx_id == 0) ? (rxq->nb_rx_desc - 1) : (rx_id - 1)); + rte_dma_wmb(); IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rx_id); nb_hold = 0; } @@ -1494,17 +1494,6 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts, next_desc: /* - * The code in this whole file uses the volatile pointer to - * ensure the read ordering of the status and the rest of the - * descriptor fields (on the compiler level only!!!). This is so - * UGLY - why not to just use the compiler barrier instead? DPDK - * even has the rte_compiler_barrier() for that. - * - * But most importantly this is just wrong because this doesn't - * ensure memory ordering in a general case at all. For - * instance, DPDK is supposed to work on Power CPUs where - * compiler barrier may just not be enough! - * * I tried to write only this function properly to have a * starting point (as a part of an LRO/RSC series) but the * compiler cursed at me when I tried to cast away the @@ -1519,12 +1508,11 @@ next_desc: * TODO: * - Get rid of "volatile" crap and let the compiler do its * job. - * - Use the proper memory barrier (rte_rmb()) to ensure the - * memory ordering below. */ rxdp = &rx_ring[rx_id]; staterr = rte_le_to_cpu_32(rxdp->wb.upper.status_error); + rte_dma_rmb(); if (!(staterr & IXGBE_RXDADV_STAT_DD)) break; @@ -1704,7 +1692,7 @@ next_desc: "nb_hold=%u nb_rx=%u", rxq->port_id, rxq->queue_id, rx_id, nb_hold, nb_rx); - rte_wmb(); + rte_dma_wmb(); IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, prev_id); nb_hold = 0; } diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c index abd10f6..af4d779 100644 --- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c @@ -123,6 +123,7 @@ ixgbe_rxq_rearm(struct ixgbe_rx_queue *rxq) (rxq->nb_rx_desc - 1) : (rxq->rxrearm_start - 1)); /* Update the tail pointer on the NIC */ + rte_dma_wmb(); IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rx_id); } @@ -528,6 +529,7 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq) /* check DD bit on threshold descriptor */ status = txq->tx_ring[txq->tx_next_dd].wb.status; + rte_dma_rmb(); if (!(status & IXGBE_ADVTXD_STAT_DD)) return 0; @@ -645,6 +647,7 @@ ixgbe_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts, txq->tx_tail = tx_id; + rte_dma_wmb(); IXGBE_PCI_REG_WRITE(txq->tdt_reg_addr, txq->tx_tail); return nb_pkts; -- 2.1.0