Caught by clang, this idx value is only used for a debug message when the mbufs allocation fails. No need to use idx as a temporary storage. Fixes: 8f2312474529 ("net/qede: fix performance bottleneck in Rx path") Cc: stable@dpdk.org Signed-off-by: David Marchand <david.marchand@redhat.com> --- drivers/net/qede/qede_rxtx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c index c38cbb9..1fbeba2 100644 --- a/drivers/net/qede/qede_rxtx.c +++ b/drivers/net/qede/qede_rxtx.c @@ -46,8 +46,6 @@ static inline int qede_alloc_rx_bulk_mbufs(struct qede_rx_queue *rxq, int count) int i, ret = 0; uint16_t idx; - idx = rxq->sw_rx_prod & NUM_RX_BDS(rxq); - if (count > QEDE_MAX_BULK_ALLOC_COUNT) count = QEDE_MAX_BULK_ALLOC_COUNT; @@ -56,7 +54,9 @@ static inline int qede_alloc_rx_bulk_mbufs(struct qede_rx_queue *rxq, int count) PMD_RX_LOG(ERR, rxq, "Failed to allocate %d rx buffers " "sw_rx_prod %u sw_rx_cons %u mp entries %u free %u", - count, idx, rxq->sw_rx_cons & NUM_RX_BDS(rxq), + count, + rxq->sw_rx_prod & NUM_RX_BDS(rxq), + rxq->sw_rx_cons & NUM_RX_BDS(rxq), rte_mempool_avail_count(rxq->mb_pool), rte_mempool_in_use_count(rxq->mb_pool)); return -ENOMEM; -- 1.8.3.1
On Fri, Sep 27, 2019 at 4:59 PM David Marchand <david.marchand@redhat.com> wrote: > > Caught by clang, this idx value is only used for a debug message when > the mbufs allocation fails. > No need to use idx as a temporary storage. > > Fixes: 8f2312474529 ("net/qede: fix performance bottleneck in Rx path") > Cc: stable@dpdk.org Rasesh, Shahed, Please review this patch. > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- > drivers/net/qede/qede_rxtx.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c > index c38cbb9..1fbeba2 100644 > --- a/drivers/net/qede/qede_rxtx.c > +++ b/drivers/net/qede/qede_rxtx.c > @@ -46,8 +46,6 @@ static inline int qede_alloc_rx_bulk_mbufs(struct qede_rx_queue *rxq, int count) > int i, ret = 0; > uint16_t idx; > > - idx = rxq->sw_rx_prod & NUM_RX_BDS(rxq); > - > if (count > QEDE_MAX_BULK_ALLOC_COUNT) > count = QEDE_MAX_BULK_ALLOC_COUNT; > > @@ -56,7 +54,9 @@ static inline int qede_alloc_rx_bulk_mbufs(struct qede_rx_queue *rxq, int count) > PMD_RX_LOG(ERR, rxq, > "Failed to allocate %d rx buffers " > "sw_rx_prod %u sw_rx_cons %u mp entries %u free %u", > - count, idx, rxq->sw_rx_cons & NUM_RX_BDS(rxq), > + count, > + rxq->sw_rx_prod & NUM_RX_BDS(rxq), > + rxq->sw_rx_cons & NUM_RX_BDS(rxq), > rte_mempool_avail_count(rxq->mb_pool), > rte_mempool_in_use_count(rxq->mb_pool)); > return -ENOMEM; > -- > 1.8.3.1 >
>From: dev <dev-bounces@dpdk.org> On Behalf Of Jerin Jacob >Sent: Thursday, October 03, 2019 7:40 AM > >On Fri, Sep 27, 2019 at 4:59 PM David Marchand ><david.marchand@redhat.com> wrote: >> >> Caught by clang, this idx value is only used for a debug message when >> the mbufs allocation fails. >> No need to use idx as a temporary storage. >> >> Fixes: 8f2312474529 ("net/qede: fix performance bottleneck in Rx >> path") >> Cc: stable@dpdk.org > >Rasesh, Shahed, > >Please review this patch. Acked. Thanks! -Rasesh > >> >> Signed-off-by: David Marchand <david.marchand@redhat.com> >> --- >> drivers/net/qede/qede_rxtx.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/qede/qede_rxtx.c >> b/drivers/net/qede/qede_rxtx.c index c38cbb9..1fbeba2 100644 >> --- a/drivers/net/qede/qede_rxtx.c >> +++ b/drivers/net/qede/qede_rxtx.c >> @@ -46,8 +46,6 @@ static inline int qede_alloc_rx_bulk_mbufs(struct >qede_rx_queue *rxq, int count) >> int i, ret = 0; >> uint16_t idx; >> >> - idx = rxq->sw_rx_prod & NUM_RX_BDS(rxq); >> - >> if (count > QEDE_MAX_BULK_ALLOC_COUNT) >> count = QEDE_MAX_BULK_ALLOC_COUNT; >> >> @@ -56,7 +54,9 @@ static inline int qede_alloc_rx_bulk_mbufs(struct >qede_rx_queue *rxq, int count) >> PMD_RX_LOG(ERR, rxq, >> "Failed to allocate %d rx buffers " >> "sw_rx_prod %u sw_rx_cons %u mp entries %u free %u", >> - count, idx, rxq->sw_rx_cons & NUM_RX_BDS(rxq), >> + count, >> + rxq->sw_rx_prod & NUM_RX_BDS(rxq), >> + rxq->sw_rx_cons & NUM_RX_BDS(rxq), >> rte_mempool_avail_count(rxq->mb_pool), >> rte_mempool_in_use_count(rxq->mb_pool)); >> return -ENOMEM; >> -- >> 1.8.3.1 >>
On Thu, Oct 3, 2019 at 11:59 PM Rasesh Mody <rmody@marvell.com> wrote: > > >From: David Marchand <david.marchand@redhat.com> > >Sent: Friday, September 27, 2019 4:29 AM > > > >---------------------------------------------------------------------- > >Caught by clang, this idx value is only used for a debug message when the > >mbufs allocation fails. > >No need to use idx as a temporary storage. > > > >Fixes: 8f2312474529 ("net/qede: fix performance bottleneck in Rx path") > >Cc: stable@dpdk.org > > > >Signed-off-by: David Marchand <david.marchand@redhat.com> > >--- > > Acked-by: Rasesh Mody <rmody@marvell.com> Changed the subject to "net/qede: only access SW Rx ring index for debug" to fix check-git-log.sh warning. Applied to dpdk-next-net-mrvl/master. Thanks > > drivers/net/qede/qede_rxtx.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > >diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c > >index c38cbb9..1fbeba2 100644 > >--- a/drivers/net/qede/qede_rxtx.c > >+++ b/drivers/net/qede/qede_rxtx.c > >@@ -46,8 +46,6 @@ static inline int qede_alloc_rx_bulk_mbufs(struct > >qede_rx_queue *rxq, int count) > > int i, ret = 0; > > uint16_t idx; > > > >- idx = rxq->sw_rx_prod & NUM_RX_BDS(rxq); > >- > > if (count > QEDE_MAX_BULK_ALLOC_COUNT) > > count = QEDE_MAX_BULK_ALLOC_COUNT; > > > >@@ -56,7 +54,9 @@ static inline int qede_alloc_rx_bulk_mbufs(struct > >qede_rx_queue *rxq, int count) > > PMD_RX_LOG(ERR, rxq, > > "Failed to allocate %d rx buffers " > > "sw_rx_prod %u sw_rx_cons %u mp entries %u free > >%u", > >- count, idx, rxq->sw_rx_cons & NUM_RX_BDS(rxq), > >+ count, > >+ rxq->sw_rx_prod & NUM_RX_BDS(rxq), > >+ rxq->sw_rx_cons & NUM_RX_BDS(rxq), > > rte_mempool_avail_count(rxq->mb_pool), > > rte_mempool_in_use_count(rxq->mb_pool)); > > return -ENOMEM; > >-- > >1.8.3.1 >