net/bnxt: remove useless prefetches
diff mbox series

Message ID 20201104170310.2509-1-stephen@networkplumber.org
State Accepted, archived
Delegated to: Ajit Khaparde
Headers show
Series
  • net/bnxt: remove useless prefetches
Related show

Checks

Context Check Description
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional fail Functional Testing issues
ci/iol-testing success Testing PASS
ci/iol-broadcom-Functional fail Functional Testing issues
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Stephen Hemminger Nov. 4, 2020, 5:03 p.m. UTC
Prefetching only helps performance if it is done several 100
instructions before the actual use. The purpose of the prefetch
is to read ahead, it doesn't help if the next instruction
will block.

The code in the bnxt driver was doing these unnecessary prefetches.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/bnxt/bnxt_rxr.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Lance Richardson Nov. 5, 2020, 3:18 p.m. UTC | #1
On Wed, Nov 4, 2020 at 12:03 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> Prefetching only helps performance if it is done several 100
> instructions before the actual use. The purpose of the prefetch
> is to read ahead, it doesn't help if the next instruction
> will block.
>
> The code in the bnxt driver was doing these unnecessary prefetches.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/net/bnxt/bnxt_rxr.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
> index 2645ed61f402..140d79e0e829 100644
> --- a/drivers/net/bnxt/bnxt_rxr.c
> +++ b/drivers/net/bnxt/bnxt_rxr.c
> @@ -305,7 +305,6 @@ static inline struct rte_mbuf *bnxt_tpa_end(
>         mbuf = tpa_info->mbuf;
>         RTE_ASSERT(mbuf != NULL);
>
> -       rte_prefetch0(mbuf);
>         if (agg_bufs) {
>                 bnxt_rx_pages(rxq, mbuf, raw_cp_cons, agg_bufs, tpa_info);
>         }
> @@ -733,8 +732,6 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
>         if (mbuf == NULL)
>                 return -EBUSY;
>
> -       rte_prefetch0(mbuf);
> -
>         mbuf->data_off = RTE_PKTMBUF_HEADROOM;
>         mbuf->nb_segs = 1;
>         mbuf->next = NULL;
> @@ -867,7 +864,6 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>         /* Handle RX burst request */
>         while (1) {
>                 cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
> -               rte_prefetch0(&cpr->cp_desc_ring[cons]);
>                 rxcmp = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
>
>                 if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct))
> --
> 2.27.0
>
Acked-by: Lance Richardson <lance.richardson@broadcom.com>
Ajit Khaparde Nov. 5, 2020, 10:24 p.m. UTC | #2
On Thu, Nov 5, 2020 at 7:18 AM Lance Richardson <
lance.richardson@broadcom.com> wrote:

> On Wed, Nov 4, 2020 at 12:03 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > Prefetching only helps performance if it is done several 100
> > instructions before the actual use. The purpose of the prefetch
> > is to read ahead, it doesn't help if the next instruction
> > will block.
> >
> > The code in the bnxt driver was doing these unnecessary prefetches.
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  drivers/net/bnxt/bnxt_rxr.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
> > index 2645ed61f402..140d79e0e829 100644
> > --- a/drivers/net/bnxt/bnxt_rxr.c
> > +++ b/drivers/net/bnxt/bnxt_rxr.c
> > @@ -305,7 +305,6 @@ static inline struct rte_mbuf *bnxt_tpa_end(
> >         mbuf = tpa_info->mbuf;
> >         RTE_ASSERT(mbuf != NULL);
> >
> > -       rte_prefetch0(mbuf);
> >         if (agg_bufs) {
> >                 bnxt_rx_pages(rxq, mbuf, raw_cp_cons, agg_bufs,
> tpa_info);
> >         }
> > @@ -733,8 +732,6 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
> >         if (mbuf == NULL)
> >                 return -EBUSY;
> >
> > -       rte_prefetch0(mbuf);
> > -
> >         mbuf->data_off = RTE_PKTMBUF_HEADROOM;
> >         mbuf->nb_segs = 1;
> >         mbuf->next = NULL;
> > @@ -867,7 +864,6 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct
> rte_mbuf **rx_pkts,
> >         /* Handle RX burst request */
> >         while (1) {
> >                 cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
> > -               rte_prefetch0(&cpr->cp_desc_ring[cons]);
> >                 rxcmp = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
> >
> >                 if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct))
> > --
> > 2.27.0
> >
> Acked-by: Lance Richardson <lance.richardson@broadcom.com>
>
Updated commit log with fixes tag and applied to dpdk-next-net-brcm.

Patch
diff mbox series

diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 2645ed61f402..140d79e0e829 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -305,7 +305,6 @@  static inline struct rte_mbuf *bnxt_tpa_end(
 	mbuf = tpa_info->mbuf;
 	RTE_ASSERT(mbuf != NULL);
 
-	rte_prefetch0(mbuf);
 	if (agg_bufs) {
 		bnxt_rx_pages(rxq, mbuf, raw_cp_cons, agg_bufs, tpa_info);
 	}
@@ -733,8 +732,6 @@  static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 	if (mbuf == NULL)
 		return -EBUSY;
 
-	rte_prefetch0(mbuf);
-
 	mbuf->data_off = RTE_PKTMBUF_HEADROOM;
 	mbuf->nb_segs = 1;
 	mbuf->next = NULL;
@@ -867,7 +864,6 @@  uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	/* Handle RX burst request */
 	while (1) {
 		cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
-		rte_prefetch0(&cpr->cp_desc_ring[cons]);
 		rxcmp = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
 
 		if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct))