[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6e041187f6284c78bda53bdc7abc46ca@amazon.com>
Date: Tue, 30 Jan 2024 09:39:07 +0000
From: "Arinzon, David" <darinzon@...zon.com>
To: "Nelson, Shannon" <shannon.nelson@....com>, David Miller
<davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "Woodhouse, David" <dwmw@...zon.co.uk>, "Machulsky, Zorik"
<zorik@...zon.com>, "Matushevsky, Alexander" <matua@...zon.com>, "Bshara,
Saeed" <saeedb@...zon.com>, "Wilson, Matt" <msw@...zon.com>, "Liguori,
Anthony" <aliguori@...zon.com>, "Bshara, Nafea" <nafea@...zon.com>,
"Belgazal, Netanel" <netanel@...zon.com>, "Saidi, Ali" <alisaidi@...zon.com>,
"Herrenschmidt, Benjamin" <benh@...zon.com>, "Kiyanovski, Arthur"
<akiyano@...zon.com>, "Dagan, Noam" <ndagan@...zon.com>, "Agroskin, Shay"
<shayagr@...zon.com>, "Itzko, Shahar" <itzko@...zon.com>, "Abboud, Osama"
<osamaabb@...zon.com>, "Ostrovsky, Evgeny" <evostrov@...zon.com>, "Tabachnik,
Ofir" <ofirt@...zon.com>, "Koler, Nati" <nkolder@...zon.com>
Subject: RE: [PATCH v1 net-next 05/11] net: ena: Remove CQ tail pointer update
> On 1/29/2024 12:55 AM, darinzon@...zon.com wrote:
> >
> > From: David Arinzon <darinzon@...zon.com>
> >
> > The functionality was added to allow the drivers to create an SQ and
> > CQ of different sizes.
> > When the RX/TX SQ and CQ have the same size, such update isn't
> > necessary as the device can safely assume it doesn't override
> > unprocessed completions. However, if the SQ is larger than the CQ, the
> > device might "have" more completions it wants to update about than
> > there's room in the CQ.
> >
> > There's no support for different SQ and CQ sizes, therefore, removing
> > the API and its usage.
> >
> > '____cacheline_aligned' compiler attribute was added to 'struct
> > ena_com_io_cq' to ensure that the removal of the 'cq_head_db_reg'
> > field doesn't change the cache-line layout of this struct.
> >
> > Signed-off-by: Shay Agroskin <shayagr@...zon.com>
> > Signed-off-by: David Arinzon <darinzon@...zon.com>
> > ---
> > drivers/net/ethernet/amazon/ena/ena_com.c | 5 ----
> > drivers/net/ethernet/amazon/ena/ena_com.h | 5 +---
> > drivers/net/ethernet/amazon/ena/ena_eth_com.h | 24 -------------------
> > drivers/net/ethernet/amazon/ena/ena_netdev.c | 5 +---
> > drivers/net/ethernet/amazon/ena/ena_xdp.c | 1 -
> > 5 files changed, 2 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c
> > b/drivers/net/ethernet/amazon/ena/ena_com.c
> > index 9a8a43b..675ee72 100644
> > --- a/drivers/net/ethernet/amazon/ena/ena_com.c
> > +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
> > @@ -1427,11 +1427,6 @@ int ena_com_create_io_cq(struct ena_com_dev
> *ena_dev,
> > io_cq->unmask_reg = (u32 __iomem *)((uintptr_t)ena_dev->reg_bar
> +
> > cmd_completion.cq_interrupt_unmask_register_offset);
> >
> > - if (cmd_completion.cq_head_db_register_offset)
> > - io_cq->cq_head_db_reg =
> > - (u32 __iomem *)((uintptr_t)ena_dev->reg_bar +
> > - cmd_completion.cq_head_db_register_offset);
> > -
> > if (cmd_completion.numa_node_register_offset)
> > io_cq->numa_node_cfg_reg =
> > (u32 __iomem *)((uintptr_t)ena_dev->reg_bar +
> > diff --git a/drivers/net/ethernet/amazon/ena/ena_com.h
> > b/drivers/net/ethernet/amazon/ena/ena_com.h
> > index f3176fc..8f90c3c 100644
> > --- a/drivers/net/ethernet/amazon/ena/ena_com.h
> > +++ b/drivers/net/ethernet/amazon/ena/ena_com.h
> > @@ -109,8 +109,6 @@ struct ena_com_io_cq {
> > /* Interrupt unmask register */
> > u32 __iomem *unmask_reg;
> >
> > - /* The completion queue head doorbell register */
> > - u32 __iomem *cq_head_db_reg;
>
> You'll want to remove one of the surrounding blank lines as well so as to not
> end up with multiple blanks in a row.
>
> sln
>
Good catch! Thanks for noticing it. Will fix it in the next patchset.
David
> >
> > /* numa configuration register (for TPH) */
> > u32 __iomem *numa_node_cfg_reg; @@ -118,7 +116,7 @@ struct
> > ena_com_io_cq {
> > /* The value to write to the above register to unmask
> > * the interrupt of this queue
> > */
> > - u32 msix_vector;
> > + u32 msix_vector ____cacheline_aligned;
> >
> > enum queue_direction direction;
> >
> > @@ -134,7 +132,6 @@ struct ena_com_io_cq {
> > /* Device queue index */
> > u16 idx;
> > u16 head;
> > - u16 last_head_update;
> > u8 phase;
> > u8 cdesc_entry_size_in_bytes;
> >
> > diff --git a/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> > b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> > index 372b259..4d65d82 100644
> > --- a/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> > +++ b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> > @@ -8,8 +8,6 @@
> >
> > #include "ena_com.h"
> >
> > -/* head update threshold in units of (queue size /
> > ENA_COMP_HEAD_THRESH) */ -#define ENA_COMP_HEAD_THRESH 4
> > /* we allow 2 DMA descriptors per LLQ entry */
> > #define ENA_LLQ_ENTRY_DESC_CHUNK_SIZE (2 * sizeof(struct
> ena_eth_io_tx_desc))
> > #define ENA_LLQ_HEADER (128UL -
> ENA_LLQ_ENTRY_DESC_CHUNK_SIZE)
> > @@ -172,28 +170,6 @@ static inline int ena_com_write_sq_doorbell(struct
> ena_com_io_sq *io_sq)
> > return 0;
> > }
> >
> > -static inline int ena_com_update_dev_comp_head(struct ena_com_io_cq
> > *io_cq) -{
> > - u16 unreported_comp, head;
> > - bool need_update;
> > -
> > - if (unlikely(io_cq->cq_head_db_reg)) {
> > - head = io_cq->head;
> > - unreported_comp = head - io_cq->last_head_update;
> > - need_update = unreported_comp > (io_cq->q_depth /
> ENA_COMP_HEAD_THRESH);
> > -
> > - if (unlikely(need_update)) {
> > - netdev_dbg(ena_com_io_cq_to_ena_dev(io_cq)-
> >net_device,
> > - "Write completion queue doorbell for queue %d: head:
> %d\n",
> > - io_cq->qid, head);
> > - writel(head, io_cq->cq_head_db_reg);
> > - io_cq->last_head_update = head;
> > - }
> > - }
> > -
> > - return 0;
> > -}
> > -
> > static inline void ena_com_update_numa_node(struct ena_com_io_cq
> *io_cq,
> > u8 numa_node)
> > {
> > diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> > b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> > index 0b7f94f..cd75e5a 100644
> > --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> > +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> > @@ -856,7 +856,6 @@ static int ena_clean_tx_irq(struct ena_ring
> > *tx_ring, u32 budget)
> >
> > tx_ring->next_to_clean = next_to_clean;
> > ena_com_comp_ack(tx_ring->ena_com_io_sq, total_done);
> > - ena_com_update_dev_comp_head(tx_ring->ena_com_io_cq);
> >
> > netdev_tx_completed_queue(txq, tx_pkts, tx_bytes);
> >
> > @@ -1303,10 +1302,8 @@ static int ena_clean_rx_irq(struct ena_ring
> *rx_ring, struct napi_struct *napi,
> > ENA_RX_REFILL_THRESH_PACKET);
> >
> > /* Optimization, try to batch new rx buffers */
> > - if (refill_required > refill_threshold) {
> > - ena_com_update_dev_comp_head(rx_ring->ena_com_io_cq);
> > + if (refill_required > refill_threshold)
> > ena_refill_rx_bufs(rx_ring, refill_required);
> > - }
> >
> > if (xdp_flags & ENA_XDP_REDIRECT)
> > xdp_do_flush();
> > diff --git a/drivers/net/ethernet/amazon/ena/ena_xdp.c
> > b/drivers/net/ethernet/amazon/ena/ena_xdp.c
> > index fc1c4ef..337c435 100644
> > --- a/drivers/net/ethernet/amazon/ena/ena_xdp.c
> > +++ b/drivers/net/ethernet/amazon/ena/ena_xdp.c
> > @@ -412,7 +412,6 @@ static int ena_clean_xdp_irq(struct ena_ring
> > *tx_ring, u32 budget)
> >
> > tx_ring->next_to_clean = next_to_clean;
> > ena_com_comp_ack(tx_ring->ena_com_io_sq, total_done);
> > - ena_com_update_dev_comp_head(tx_ring->ena_com_io_cq);
> >
> > netif_dbg(tx_ring->adapter, tx_done, tx_ring->netdev,
> > "tx_poll: q %d done. total pkts: %d\n",
> > --
> > 2.40.1
> >
> >
Powered by blists - more mailing lists