Re: [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native Tx datapath reap

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 9/8/2017 3:15 PM, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  drivers/net/sfc/sfc_ef10_tx.c | 48 ++++++++++++++++++++++++++++++++++++-------
>  drivers/net/sfc/sfc_tweak.h   |  3 +++
>  2 files changed, 44 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/sfc/sfc_ef10_tx.c b/drivers/net/sfc/sfc_ef10_tx.c
> index 182fc23..5127a7a 100644
> --- a/drivers/net/sfc/sfc_ef10_tx.c
> +++ b/drivers/net/sfc/sfc_ef10_tx.c
> @@ -158,17 +158,35 @@ struct sfc_ef10_txq {
>  	pending += sfc_ef10_tx_process_events(txq);
>  
>  	if (pending != completed) {
> +		struct rte_mbuf *bulk[SFC_TX_REAP_BULK_SIZE];
> +		unsigned int nb = 0;
> +
>  		do {
>  			struct sfc_ef10_tx_sw_desc *txd;
> +			struct rte_mbuf *m;
>  
>  			txd = &txq->sw_ring[completed & ptr_mask];
> +			if (txd->mbuf == NULL)
> +				continue;
>  
> -			if (txd->mbuf != NULL) {
> -				rte_pktmbuf_free(txd->mbuf);
> -				txd->mbuf = NULL;
> +			m = rte_pktmbuf_prefree_seg(txd->mbuf);
> +			txd->mbuf = NULL;
> +			if (m == NULL)
> +				continue;
> +
> +			if ((nb == RTE_DIM(bulk)) ||
> +			    ((nb != 0) && (m->pool != bulk[0]->pool))) {

ICC is giving warning [1] here, as far as I can see this is false
positive but can you please double check in case I am missing something?

And unless if you don't see a way to convince icc without effecting
performance, would you mind updating patch to ignore warning [2] ?


[1]
error #3656: variable "bulk" may be used before its value is set

[2]
  diff --git a/drivers/net/sfc/Makefile b/drivers/net/sfc/Makefile
  index 57aa963ba..359314809 100644
  --- a/drivers/net/sfc/Makefile
  +++ b/drivers/net/sfc/Makefile
  @@ -65,6 +65,7 @@ CFLAGS += -Wbad-function-cast
   CFLAGS_BASE_DRIVER += -Wno-empty-body
   else ifeq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)
   CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable
  +CFLAGS_sfc_ef10_tx.o += -wd3656
   endif

   #

> +				rte_mempool_put_bulk(bulk[0]->pool,
> +						     (void *)bulk, nb);
> +				nb = 0;
>  			}
> +
> +			bulk[nb++] = m;
>  		} while (++completed != pending);
>  
> +		if (nb != 0)
> +			rte_mempool_put_bulk(bulk[0]->pool, (void *)bulk, nb);
> +
>  		txq->completed = completed;
>  	}

<...>