From: okaya@codeaurora.org
To: "Belgazal, Netanel" <netanel@amazon.com>
Cc: netdev@vger.kernel.org, timur@codeaurora.org,
sulrich@codeaurora.org, "Kiyanovski, Arthur" <akiyano@amazon.com>,
linux-arm-msm@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, "Bshara,
Saeed" <saeedb@amazon.com>, "Machulsky, Zorik" <zorik@amazon.com>,
"David S. Miller" <davem@davemloft.net>,
Tobias Klauser <tklauser@distanz.ch>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 17/17] net: ena: Eliminate duplicate barriers on weakly-ordered archs
Date: Sun, 25 Mar 2018 09:33:35 -0400 [thread overview]
Message-ID: <bf9846e1234f0211476d2e709a64b1fe@codeaurora.org> (raw)
In-Reply-To: <7BF039D2-9E0F-45BE-9F40-8E785814C17D@amazon.com>
On 2018-03-25 08:06, Belgazal, Netanel wrote:
> I think you should either add a parameter to
> ena_com_write_sq_doorbell() or add ena_com_write_sq_doorbell_rel().
> Right now, you have unused function.
That is true. I got rid of ena_com_write_sq_doorbell_rel.
>
> On 3/20/18, 4:43 AM, "Sinan Kaya" <okaya@codeaurora.org> wrote:
>
> Code includes barrier() followed by writel(). 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.
>
> Create a new wrapper function with relaxed write operator. Use the
> new
> wrapper when a write is following a barrier().
>
> Since code already has an explicit barrier call, changing writel()
> to
> writel_relaxed().
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/net/ethernet/amazon/ena/ena_com.c | 6 ++++--
> drivers/net/ethernet/amazon/ena/ena_eth_com.h | 22
> ++++++++++++++++++++--
> drivers/net/ethernet/amazon/ena/ena_netdev.c | 4 ++--
> 3 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c
> b/drivers/net/ethernet/amazon/ena/ena_com.c
> index bf2de52..b6e628f 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_com.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
> @@ -631,7 +631,8 @@ static u32 ena_com_reg_bar_read32(struct
> ena_com_dev *ena_dev, u16 offset)
> */
> wmb();
>
> - writel(mmio_read_reg, ena_dev->reg_bar +
> ENA_REGS_MMIO_REG_READ_OFF);
> + writel_relaxed(mmio_read_reg,
> + ena_dev->reg_bar + ENA_REGS_MMIO_REG_READ_OFF);
>
> for (i = 0; i < timeout; i++) {
> if (read_resp->req_id == mmio_read->seq_num)
> @@ -1826,7 +1827,8 @@ void ena_com_aenq_intr_handler(struct
> ena_com_dev *dev, void *data)
>
> /* write the aenq doorbell after all AENQ descriptors were read
> */
> mb();
> - writel((u32)aenq->head, dev->reg_bar +
> ENA_REGS_AENQ_HEAD_DB_OFF);
> + writel_relaxed((u32)aenq->head,
> + dev->reg_bar + ENA_REGS_AENQ_HEAD_DB_OFF);
> }
>
> int ena_com_dev_reset(struct ena_com_dev *ena_dev,
> diff --git a/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> index 2f76572..09ef7cd 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> @@ -107,7 +107,8 @@ static inline int
> ena_com_sq_empty_space(struct ena_com_io_sq *io_sq)
> return io_sq->q_depth - 1 - cnt;
> }
>
> -static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq
> *io_sq)
> +static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq
> *io_sq,
> + bool relaxed)
> {
> u16 tail;
>
> @@ -116,7 +117,24 @@ static inline int
> ena_com_write_sq_doorbell(struct ena_com_io_sq *io_sq)
> pr_debug("write submission queue doorbell for queue: %d tail:
> %d\n",
> io_sq->qid, tail);
>
> - writel(tail, io_sq->db_addr);
> + if (relaxed)
> + writel_relaxed(tail, io_sq->db_addr);
> + else
> + writel(tail, io_sq->db_addr);
> +
> + return 0;
> +}
> +
> +static inline int ena_com_write_sq_doorbell_rel(struct
> ena_com_io_sq *io_sq)
> +{
> + u16 tail;
> +
> + tail = io_sq->tail;
> +
> + pr_debug("write submission queue doorbell for queue: %d tail:
> %d\n",
> + io_sq->qid, tail);
> +
> + writel_relaxed(tail, io_sq->db_addr);
>
> return 0;
> }
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 6975150..0530201 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -556,7 +556,7 @@ static int ena_refill_rx_bufs(struct ena_ring
> *rx_ring, u32 num)
> * issue a doorbell
> */
> wmb();
> - ena_com_write_sq_doorbell(rx_ring->ena_com_io_sq);
> + ena_com_write_sq_doorbell(rx_ring->ena_com_io_sq, true);
> }
>
> rx_ring->next_to_use = next_to_use;
> @@ -2151,7 +2151,7 @@ static netdev_tx_t ena_start_xmit(struct
> sk_buff *skb, struct net_device *dev)
>
> if (netif_xmit_stopped(txq) || !skb->xmit_more) {
> /* trigger the dma engine */
> - ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
> + ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq, false);
> u64_stats_update_begin(&tx_ring->syncp);
> tx_ring->tx_stats.doorbells++;
> u64_stats_update_end(&tx_ring->syncp);
> --
> 2.7.4
WARNING: multiple messages have this Message-ID (diff)
From: okaya@codeaurora.org (okaya at codeaurora.org)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 17/17] net: ena: Eliminate duplicate barriers on weakly-ordered archs
Date: Sun, 25 Mar 2018 09:33:35 -0400 [thread overview]
Message-ID: <bf9846e1234f0211476d2e709a64b1fe@codeaurora.org> (raw)
In-Reply-To: <7BF039D2-9E0F-45BE-9F40-8E785814C17D@amazon.com>
On 2018-03-25 08:06, Belgazal, Netanel wrote:
> I think you should either add a parameter to
> ena_com_write_sq_doorbell() or add ena_com_write_sq_doorbell_rel().
> Right now, you have unused function.
That is true. I got rid of ena_com_write_sq_doorbell_rel.
>
> ?On 3/20/18, 4:43 AM, "Sinan Kaya" <okaya@codeaurora.org> wrote:
>
> Code includes barrier() followed by writel(). 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.
>
> Create a new wrapper function with relaxed write operator. Use the
> new
> wrapper when a write is following a barrier().
>
> Since code already has an explicit barrier call, changing writel()
> to
> writel_relaxed().
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/net/ethernet/amazon/ena/ena_com.c | 6 ++++--
> drivers/net/ethernet/amazon/ena/ena_eth_com.h | 22
> ++++++++++++++++++++--
> drivers/net/ethernet/amazon/ena/ena_netdev.c | 4 ++--
> 3 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c
> b/drivers/net/ethernet/amazon/ena/ena_com.c
> index bf2de52..b6e628f 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_com.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
> @@ -631,7 +631,8 @@ static u32 ena_com_reg_bar_read32(struct
> ena_com_dev *ena_dev, u16 offset)
> */
> wmb();
>
> - writel(mmio_read_reg, ena_dev->reg_bar +
> ENA_REGS_MMIO_REG_READ_OFF);
> + writel_relaxed(mmio_read_reg,
> + ena_dev->reg_bar + ENA_REGS_MMIO_REG_READ_OFF);
>
> for (i = 0; i < timeout; i++) {
> if (read_resp->req_id == mmio_read->seq_num)
> @@ -1826,7 +1827,8 @@ void ena_com_aenq_intr_handler(struct
> ena_com_dev *dev, void *data)
>
> /* write the aenq doorbell after all AENQ descriptors were read
> */
> mb();
> - writel((u32)aenq->head, dev->reg_bar +
> ENA_REGS_AENQ_HEAD_DB_OFF);
> + writel_relaxed((u32)aenq->head,
> + dev->reg_bar + ENA_REGS_AENQ_HEAD_DB_OFF);
> }
>
> int ena_com_dev_reset(struct ena_com_dev *ena_dev,
> diff --git a/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> index 2f76572..09ef7cd 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> @@ -107,7 +107,8 @@ static inline int
> ena_com_sq_empty_space(struct ena_com_io_sq *io_sq)
> return io_sq->q_depth - 1 - cnt;
> }
>
> -static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq
> *io_sq)
> +static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq
> *io_sq,
> + bool relaxed)
> {
> u16 tail;
>
> @@ -116,7 +117,24 @@ static inline int
> ena_com_write_sq_doorbell(struct ena_com_io_sq *io_sq)
> pr_debug("write submission queue doorbell for queue: %d tail:
> %d\n",
> io_sq->qid, tail);
>
> - writel(tail, io_sq->db_addr);
> + if (relaxed)
> + writel_relaxed(tail, io_sq->db_addr);
> + else
> + writel(tail, io_sq->db_addr);
> +
> + return 0;
> +}
> +
> +static inline int ena_com_write_sq_doorbell_rel(struct
> ena_com_io_sq *io_sq)
> +{
> + u16 tail;
> +
> + tail = io_sq->tail;
> +
> + pr_debug("write submission queue doorbell for queue: %d tail:
> %d\n",
> + io_sq->qid, tail);
> +
> + writel_relaxed(tail, io_sq->db_addr);
>
> return 0;
> }
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 6975150..0530201 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -556,7 +556,7 @@ static int ena_refill_rx_bufs(struct ena_ring
> *rx_ring, u32 num)
> * issue a doorbell
> */
> wmb();
> - ena_com_write_sq_doorbell(rx_ring->ena_com_io_sq);
> + ena_com_write_sq_doorbell(rx_ring->ena_com_io_sq, true);
> }
>
> rx_ring->next_to_use = next_to_use;
> @@ -2151,7 +2151,7 @@ static netdev_tx_t ena_start_xmit(struct
> sk_buff *skb, struct net_device *dev)
>
> if (netif_xmit_stopped(txq) || !skb->xmit_more) {
> /* trigger the dma engine */
> - ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
> + ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq, false);
> u64_stats_update_begin(&tx_ring->syncp);
> tx_ring->tx_stats.doorbells++;
> u64_stats_update_end(&tx_ring->syncp);
> --
> 2.7.4
next prev parent reply other threads:[~2018-03-25 13:33 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-20 2:42 [PATCH v4 00/17] netdev: Eliminate duplicate barriers on weakly-ordered archs Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [Intel-wired-lan] [PATCH v4 01/17] i40e/i40evf: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [Intel-wired-lan] [PATCH v4 02/17] ixgbe: eliminate " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [Intel-wired-lan] [PATCH v4 03/17] igbvf: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [Intel-wired-lan] [PATCH v4 04/17] igb: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [Intel-wired-lan] [PATCH v4 05/17] ixgbevf: keep writel() closer to wmb() Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [Intel-wired-lan] [PATCH v4 06/17] ixgbevf: eliminate duplicate barriers on weakly-ordered archs Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [Intel-wired-lan] [PATCH v4 07/17] fm10k: Eliminate " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [PATCH v4 08/17] drivers: net: cxgb: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [PATCH v4 09/17] net: qla3xxx: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [PATCH v4 10/17] qlcnic: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [PATCH v4 11/17] bnx2x: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-22 10:10 ` Kalluru, Sudarsana
2018-03-22 10:10 ` Kalluru, Sudarsana
2018-03-20 2:42 ` [PATCH v4 12/17] net: cxgb4/cxgb4vf: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-21 23:03 ` Casey Leedom
2018-03-21 23:03 ` Casey Leedom
2018-03-22 0:00 ` okaya
2018-03-22 0:00 ` okaya at codeaurora.org
2018-03-20 2:42 ` [PATCH v4 13/17] net: cxgb3: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [PATCH v4 14/17] net: qlge: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [PATCH v4 15/17] bnxt_en: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [PATCH v4 16/17] qed/qede: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-20 2:42 ` [PATCH v4 17/17] net: ena: " Sinan Kaya
2018-03-20 2:42 ` Sinan Kaya
2018-03-25 12:06 ` Belgazal, Netanel
2018-03-25 12:06 ` Belgazal, Netanel
2018-03-25 13:33 ` okaya [this message]
2018-03-25 13:33 ` okaya at codeaurora.org
2018-03-21 15:56 ` [PATCH v4 00/17] netdev: " David Miller
2018-03-21 15:56 ` David Miller
2018-03-21 19:06 ` Sinan Kaya
2018-03-21 19:06 ` Sinan Kaya
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bf9846e1234f0211476d2e709a64b1fe@codeaurora.org \
--to=okaya@codeaurora.org \
--cc=akiyano@amazon.com \
--cc=davem@davemloft.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netanel@amazon.com \
--cc=netdev@vger.kernel.org \
--cc=saeedb@amazon.com \
--cc=sulrich@codeaurora.org \
--cc=timur@codeaurora.org \
--cc=tklauser@distanz.ch \
--cc=zorik@amazon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.