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

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 09/12/2017 09:26 PM, Ferruh Yigit wrote:
> 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?

Yes, I think it is false positive.

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

Thanks, done.

>
>
> [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
>
>     #

<...>