RE: [PATCH v1 net-next 05/11] net: ena: Remove CQ tail pointer update

["Arinzon, David" <darinzon@amazon.com>]

> On 1/29/2024 12:55 AM, darinzon@amazon.com wrote:
> >
> > From: David Arinzon <darinzon@amazon.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@amazon.com>
> > Signed-off-by: David Arinzon <darinzon@amazon.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
> >
> >