* [dpdk-dev] [PATCH 1/3] ring: fix build for gcc O1 optimization
@ 2020-05-07 12:02 Ferruh Yigit
2020-05-07 12:02 ` [dpdk-dev] [PATCH 2/3] mempool/octeontx2: " Ferruh Yigit
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-07 12:02 UTC (permalink / raw)
To: Honnappa Nagarahalli, Konstantin Ananyev
Cc: dev, Ferruh Yigit, Thomas Monjalon, David Marchand
Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
Two build errors:
1)
In file included from .../build/include/rte_ring_elem.h:1093,
from .../lib/librte_rcu/rte_rcu_qsbr.c:21:
../lib/librte_rcu/rte_rcu_qsbr.c: In function ‘rte_rcu_qsbr_dq_reclaim’:
.../build/include/rte_ring_peek.h:282:22:
error: ‘avail’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
282 | *available = avail - n;
| ~~~~~~^~~
./build/include/rte_ring_peek.h:259:11: note: ‘avail’ was declared here
259 | uint32_t avail, head, next;
| ^~~~~
2)
In file included from .../build/include/rte_ring_elem.h:1093,
from .../build/include/rte_ring.h:405,
from .../app/test/test_ring_stress.h:13,
from .../app/test/test_ring_stress_impl.h:5,
from .../app/test/test_ring_peek_stress.c:5:
.../app/test/test_ring_peek_stress.c: In function ‘_st_ring_enqueue_bulk’:
.../build/include/rte_ring_peek.h:80:22:
error: ‘free’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
80 | *free_space = free - n;
| ~~~~~^~~
.../build/include/rte_ring_peek.h:60:11: note: ‘free’ was declared here
60 | uint32_t free, head, next;
| ^~~~
The cases shouldn't be hit, and it looks like there is already logic
error if it has been hit, but assigning 'avail' & 'free' to '0' to fix
the build error.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
lib/librte_ring/rte_ring_peek.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_ring/rte_ring_peek.h b/lib/librte_ring/rte_ring_peek.h
index d5e6ea1cf3..45f707dc7e 100644
--- a/lib/librte_ring/rte_ring_peek.h
+++ b/lib/librte_ring/rte_ring_peek.h
@@ -74,6 +74,7 @@ __rte_ring_do_enqueue_start(struct rte_ring *r, uint32_t n,
/* unsupported mode, shouldn't be here */
RTE_ASSERT(0);
n = 0;
+ free = 0;
}
if (free_space != NULL)
@@ -273,6 +274,7 @@ __rte_ring_do_dequeue_start(struct rte_ring *r, void *obj_table,
/* unsupported mode, shouldn't be here */
RTE_ASSERT(0);
n = 0;
+ avail = 0;
}
if (n != 0)
--
2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH 2/3] mempool/octeontx2: fix build for gcc O1 optimization
2020-05-07 12:02 [dpdk-dev] [PATCH 1/3] ring: fix build for gcc O1 optimization Ferruh Yigit
@ 2020-05-07 12:02 ` Ferruh Yigit
2020-05-07 14:05 ` Ananyev, Konstantin
2020-05-07 12:02 ` [dpdk-dev] [PATCH 3/3] net/ena: fix build for " Ferruh Yigit
` (3 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-07 12:02 UTC (permalink / raw)
To: Jerin Jacob, Nithin Dabilpuram
Cc: dev, Ferruh Yigit, Thomas Monjalon, David Marchand
Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
Build error:
In file included from .../drivers/mempool/octeontx2/otx2_mempool.h:13,
from .../drivers/mempool/octeontx2/otx2_mempool_ops.c:8:
.../drivers/mempool/octeontx2/otx2_mempool_ops.c:
In function ‘otx2_npa_alloc’:
.../drivers/common/octeontx2/otx2_common.h:94:2:
error: ‘aura_handle’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
94 | rte_log(RTE_LOG_DEBUG, otx2_logtype_ ## subsystem, \
| ^~~~~~~
.../drivers/mempool/octeontx2/otx2_mempool_ops.c:643:11:
note: ‘aura_handle’ was declared here
643 | uint64_t aura_handle;
| ^~~~~~~~~~~
This looks like false positive, assigning an initial value to
'aura_handle' to fix the build error.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
This is assuming assigning initial value won't have a performance affect
if it does, we need to find another fix.
And overall not sure why this false positive warning is generated.
---
drivers/mempool/octeontx2/otx2_mempool_ops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mempool/octeontx2/otx2_mempool_ops.c b/drivers/mempool/octeontx2/otx2_mempool_ops.c
index 162b7f01da..078ffac3e2 100644
--- a/drivers/mempool/octeontx2/otx2_mempool_ops.c
+++ b/drivers/mempool/octeontx2/otx2_mempool_ops.c
@@ -640,7 +640,7 @@ otx2_npa_alloc(struct rte_mempool *mp)
struct otx2_npa_lf *lf;
struct npa_aura_s aura;
struct npa_pool_s pool;
- uint64_t aura_handle;
+ uint64_t aura_handle = 0;
size_t padding;
int rc;
--
2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH 3/3] net/ena: fix build for O1 optimization
2020-05-07 12:02 [dpdk-dev] [PATCH 1/3] ring: fix build for gcc O1 optimization Ferruh Yigit
2020-05-07 12:02 ` [dpdk-dev] [PATCH 2/3] mempool/octeontx2: " Ferruh Yigit
@ 2020-05-07 12:02 ` Ferruh Yigit
2020-05-08 13:59 ` Michał Krawczyk
2020-05-07 13:47 ` [dpdk-dev] [PATCH 1/3] ring: fix build for gcc " Ananyev, Konstantin
` (2 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-07 12:02 UTC (permalink / raw)
To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin,
Igor Chauskin
Cc: dev, Ferruh Yigit, Thomas Monjalon, David Marchand
Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
Build error:
.../drivers/net/ena/ena_ethdev.c: In function ‘eth_ena_dev_init’:
.../drivers/net/ena/ena_ethdev.c:1815:20:
error: ‘wd_state’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
1815 | adapter->wd_state = wd_state;
| ~~~~~~~~~~~~~~~~~~^~~~~~~~~~
This looks like false positive, fixing by assigning initial value to
'wd_state' variable.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
drivers/net/ena/ena_ethdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index c3fd3a4ac0..fbddc79f79 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1756,7 +1756,7 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
int rc;
static int adapters_found;
bool disable_meta_caching;
- bool wd_state;
+ bool wd_state = false;
eth_dev->dev_ops = &ena_dev_ops;
eth_dev->rx_pkt_burst = ð_ena_recv_pkts;
--
2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dpdk-dev] [PATCH 1/3] ring: fix build for gcc O1 optimization
2020-05-07 12:02 [dpdk-dev] [PATCH 1/3] ring: fix build for gcc O1 optimization Ferruh Yigit
2020-05-07 12:02 ` [dpdk-dev] [PATCH 2/3] mempool/octeontx2: " Ferruh Yigit
2020-05-07 12:02 ` [dpdk-dev] [PATCH 3/3] net/ena: fix build for " Ferruh Yigit
@ 2020-05-07 13:47 ` Ananyev, Konstantin
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 1/4] " Ferruh Yigit
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc " Ferruh Yigit
4 siblings, 0 replies; 20+ messages in thread
From: Ananyev, Konstantin @ 2020-05-07 13:47 UTC (permalink / raw)
To: Yigit, Ferruh, Honnappa Nagarahalli; +Cc: dev, Thomas Monjalon, David Marchand
>
> Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
> gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
>
> Two build errors:
> 1)
> In file included from .../build/include/rte_ring_elem.h:1093,
> from .../lib/librte_rcu/rte_rcu_qsbr.c:21:
> ../lib/librte_rcu/rte_rcu_qsbr.c: In function ‘rte_rcu_qsbr_dq_reclaim’:
> .../build/include/rte_ring_peek.h:282:22:
> error: ‘avail’ may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
> 282 | *available = avail - n;
> | ~~~~~~^~~
> ./build/include/rte_ring_peek.h:259:11: note: ‘avail’ was declared here
> 259 | uint32_t avail, head, next;
> | ^~~~~
>
> 2)
> In file included from .../build/include/rte_ring_elem.h:1093,
> from .../build/include/rte_ring.h:405,
> from .../app/test/test_ring_stress.h:13,
> from .../app/test/test_ring_stress_impl.h:5,
> from .../app/test/test_ring_peek_stress.c:5:
> .../app/test/test_ring_peek_stress.c: In function ‘_st_ring_enqueue_bulk’:
> .../build/include/rte_ring_peek.h:80:22:
> error: ‘free’ may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
> 80 | *free_space = free - n;
> | ~~~~~^~~
> .../build/include/rte_ring_peek.h:60:11: note: ‘free’ was declared here
> 60 | uint32_t free, head, next;
> | ^~~~
>
> The cases shouldn't be hit, and it looks like there is already logic
> error if it has been hit, but assigning 'avail' & 'free' to '0' to fix
> the build error.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> lib/librte_ring/rte_ring_peek.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/lib/librte_ring/rte_ring_peek.h b/lib/librte_ring/rte_ring_peek.h
> index d5e6ea1cf3..45f707dc7e 100644
> --- a/lib/librte_ring/rte_ring_peek.h
> +++ b/lib/librte_ring/rte_ring_peek.h
> @@ -74,6 +74,7 @@ __rte_ring_do_enqueue_start(struct rte_ring *r, uint32_t n,
> /* unsupported mode, shouldn't be here */
> RTE_ASSERT(0);
> n = 0;
> + free = 0;
> }
>
> if (free_space != NULL)
> @@ -273,6 +274,7 @@ __rte_ring_do_dequeue_start(struct rte_ring *r, void *obj_table,
> /* unsupported mode, shouldn't be here */
> RTE_ASSERT(0);
> n = 0;
> + avail = 0;
> }
>
> if (n != 0)
> --
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dpdk-dev] [PATCH 2/3] mempool/octeontx2: fix build for gcc O1 optimization
2020-05-07 12:02 ` [dpdk-dev] [PATCH 2/3] mempool/octeontx2: " Ferruh Yigit
@ 2020-05-07 14:05 ` Ananyev, Konstantin
2020-05-08 16:20 ` Ferruh Yigit
0 siblings, 1 reply; 20+ messages in thread
From: Ananyev, Konstantin @ 2020-05-07 14:05 UTC (permalink / raw)
To: Yigit, Ferruh, Jerin Jacob, Nithin Dabilpuram
Cc: dev, Yigit, Ferruh, Thomas Monjalon, David Marchand
Hi Ferruh, Jerin
> Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
> gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
>
> Build error:
> In file included from .../drivers/mempool/octeontx2/otx2_mempool.h:13,
> from .../drivers/mempool/octeontx2/otx2_mempool_ops.c:8:
> .../drivers/mempool/octeontx2/otx2_mempool_ops.c:
> In function ‘otx2_npa_alloc’:
> .../drivers/common/octeontx2/otx2_common.h:94:2:
> error: ‘aura_handle’ may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
> 94 | rte_log(RTE_LOG_DEBUG, otx2_logtype_ ## subsystem, \
> | ^~~~~~~
> .../drivers/mempool/octeontx2/otx2_mempool_ops.c:643:11:
> note: ‘aura_handle’ was declared here
> 643 | uint64_t aura_handle;
> | ^~~~~~~~~~~
>
> This looks like false positive, assigning an initial value to
> 'aura_handle' to fix the build error.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> This is assuming assigning initial value won't have a performance affect
> if it does, we need to find another fix.
> And overall not sure why this false positive warning is generated.
> ---
> drivers/mempool/octeontx2/otx2_mempool_ops.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mempool/octeontx2/otx2_mempool_ops.c b/drivers/mempool/octeontx2/otx2_mempool_ops.c
> index 162b7f01da..078ffac3e2 100644
> --- a/drivers/mempool/octeontx2/otx2_mempool_ops.c
> +++ b/drivers/mempool/octeontx2/otx2_mempool_ops.c
> @@ -640,7 +640,7 @@ otx2_npa_alloc(struct rte_mempool *mp)
> struct otx2_npa_lf *lf;
> struct npa_aura_s aura;
> struct npa_pool_s pool;
> - uint64_t aura_handle;
> + uint64_t aura_handle = 0;
> size_t padding;
> int rc;
>
> --
Confirm that it helps, though I am seeing one more issue with EXTRA_CFLAGS='-O1' (gcc 7.3.0).
This time with drivers/event/octeontx2/otx2_evdev_stats.h:
In file included from /local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/ot
x2_evdev.c:15:0:
/local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/otx2_evdev_stats.h: In function ‘otx2_sso_xstats_get’:
/local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/otx2_evdev_stats.h:124:9: error: ‘xstats’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
xstat = &xstats[ids[i] - start_offset];
~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/otx2_evdev.c: At top level:
cc1: error: unrecognized command line option ‘-Wno-address-of-packed-member’ [-Werror]
cc1: all warnings being treated as errors
As I can see xtstats is not set for RTE_EVENT_DEV_XSTATS_DEVICE case.
Just to make gcc quiet, I added:
--- a/drivers/event/octeontx2/otx2_evdev_stats.h
+++ b/drivers/event/octeontx2/otx2_evdev_stats.h
@@ -67,6 +67,7 @@ otx2_sso_xstats_get(const struct rte_eventdev *event_dev,
switch (mode) {
case RTE_EVENT_DEV_XSTATS_DEVICE:
+ xstats = NULL;
break;
case RTE_EVENT_DEV_XSTATS_PORT:
if (queue_port_id >= (signed int)dev->nb_event_ports)
Konstantin
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dpdk-dev] [PATCH 3/3] net/ena: fix build for O1 optimization
2020-05-07 12:02 ` [dpdk-dev] [PATCH 3/3] net/ena: fix build for " Ferruh Yigit
@ 2020-05-08 13:59 ` Michał Krawczyk
0 siblings, 0 replies; 20+ messages in thread
From: Michał Krawczyk @ 2020-05-08 13:59 UTC (permalink / raw)
To: Ferruh Yigit
Cc: Marcin Wojtas, Guy Tzalik, Evgeny Schemeilin, Igor Chauskin, dev,
Thomas Monjalon, David Marchand
czw., 7 maj 2020 o 14:03 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
> gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
>
> Build error:
> .../drivers/net/ena/ena_ethdev.c: In function ‘eth_ena_dev_init’:
> .../drivers/net/ena/ena_ethdev.c:1815:20:
> error: ‘wd_state’ may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
> 1815 | adapter->wd_state = wd_state;
> | ~~~~~~~~~~~~~~~~~~^~~~~~~~~~
>
> This looks like false positive, fixing by assigning initial value to
> 'wd_state' variable.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
> ---
> drivers/net/ena/ena_ethdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index c3fd3a4ac0..fbddc79f79 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -1756,7 +1756,7 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
> int rc;
> static int adapters_found;
> bool disable_meta_caching;
> - bool wd_state;
> + bool wd_state = false;
>
> eth_dev->dev_ops = &ena_dev_ops;
> eth_dev->rx_pkt_burst = ð_ena_recv_pkts;
> --
> 2.25.4
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dpdk-dev] [PATCH 2/3] mempool/octeontx2: fix build for gcc O1 optimization
2020-05-07 14:05 ` Ananyev, Konstantin
@ 2020-05-08 16:20 ` Ferruh Yigit
2020-05-08 16:39 ` Ferruh Yigit
0 siblings, 1 reply; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-08 16:20 UTC (permalink / raw)
To: Ananyev, Konstantin, Jerin Jacob, Nithin Dabilpuram
Cc: dev, Thomas Monjalon, David Marchand
On 5/7/2020 3:05 PM, Ananyev, Konstantin wrote:
> Hi Ferruh, Jerin
>
>> Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
>> gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
>>
>> Build error:
>> In file included from .../drivers/mempool/octeontx2/otx2_mempool.h:13,
>> from .../drivers/mempool/octeontx2/otx2_mempool_ops.c:8:
>> .../drivers/mempool/octeontx2/otx2_mempool_ops.c:
>> In function ‘otx2_npa_alloc’:
>> .../drivers/common/octeontx2/otx2_common.h:94:2:
>> error: ‘aura_handle’ may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>> 94 | rte_log(RTE_LOG_DEBUG, otx2_logtype_ ## subsystem, \
>> | ^~~~~~~
>> .../drivers/mempool/octeontx2/otx2_mempool_ops.c:643:11:
>> note: ‘aura_handle’ was declared here
>> 643 | uint64_t aura_handle;
>> | ^~~~~~~~~~~
>>
>> This looks like false positive, assigning an initial value to
>> 'aura_handle' to fix the build error.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> This is assuming assigning initial value won't have a performance affect
>> if it does, we need to find another fix.
>> And overall not sure why this false positive warning is generated.
>> ---
>> drivers/mempool/octeontx2/otx2_mempool_ops.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mempool/octeontx2/otx2_mempool_ops.c b/drivers/mempool/octeontx2/otx2_mempool_ops.c
>> index 162b7f01da..078ffac3e2 100644
>> --- a/drivers/mempool/octeontx2/otx2_mempool_ops.c
>> +++ b/drivers/mempool/octeontx2/otx2_mempool_ops.c
>> @@ -640,7 +640,7 @@ otx2_npa_alloc(struct rte_mempool *mp)
>> struct otx2_npa_lf *lf;
>> struct npa_aura_s aura;
>> struct npa_pool_s pool;
>> -uint64_t aura_handle;
>> +uint64_t aura_handle = 0;
>> size_t padding;
>> int rc;
>>
>> --
>
>
> Confirm that it helps, though I am seeing one more issue with EXTRA_CFLAGS='-O1' (gcc 7.3.0).
> This time with drivers/event/octeontx2/otx2_evdev_stats.h:
>
> In file included from /local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/ot
> x2_evdev.c:15:0:
> /local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/otx2_evdev_stats.h: In function ‘otx2_sso_xstats_get’:
> /local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/otx2_evdev_stats.h:124:9: error: ‘xstats’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> xstat = &xstats[ids[i] - start_offset];
> ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/otx2_evdev.c: At top level:
> cc1: error: unrecognized command line option ‘-Wno-address-of-packed-member’ [-Werror]
> cc1: all warnings being treated as errors
>
> As I can see xtstats is not set for RTE_EVENT_DEV_XSTATS_DEVICE case.
> Just to make gcc quiet, I added:
> --- a/drivers/event/octeontx2/otx2_evdev_stats.h
> +++ b/drivers/event/octeontx2/otx2_evdev_stats.h
> @@ -67,6 +67,7 @@ otx2_sso_xstats_get(const struct rte_eventdev *event_dev,
>
> switch (mode) {
> case RTE_EVENT_DEV_XSTATS_DEVICE:
> + xstats = NULL;
> break;
> case RTE_EVENT_DEV_XSTATS_PORT:
> if (queue_port_id >= (signed int)dev->nb_event_ports)
>
> Konstantin
>
>
>
Thanks Konstantin,
I will make a v2 including above as you suggested.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dpdk-dev] [PATCH 2/3] mempool/octeontx2: fix build for gcc O1 optimization
2020-05-08 16:20 ` Ferruh Yigit
@ 2020-05-08 16:39 ` Ferruh Yigit
0 siblings, 0 replies; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-08 16:39 UTC (permalink / raw)
To: Ananyev, Konstantin, Jerin Jacob, Nithin Dabilpuram
Cc: dev, Thomas Monjalon, David Marchand
On 5/8/2020 5:20 PM, Ferruh Yigit wrote:
> On 5/7/2020 3:05 PM, Ananyev, Konstantin wrote:
>> Hi Ferruh, Jerin
>>
>>> Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
>>> gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
>>>
>>> Build error:
>>> In file included from .../drivers/mempool/octeontx2/otx2_mempool.h:13,
>>> from .../drivers/mempool/octeontx2/otx2_mempool_ops.c:8:
>>> .../drivers/mempool/octeontx2/otx2_mempool_ops.c:
>>> In function ‘otx2_npa_alloc’:
>>> .../drivers/common/octeontx2/otx2_common.h:94:2:
>>> error: ‘aura_handle’ may be used uninitialized in this function
>>> [-Werror=maybe-uninitialized]
>>> 94 | rte_log(RTE_LOG_DEBUG, otx2_logtype_ ## subsystem, \
>>> | ^~~~~~~
>>> .../drivers/mempool/octeontx2/otx2_mempool_ops.c:643:11:
>>> note: ‘aura_handle’ was declared here
>>> 643 | uint64_t aura_handle;
>>> | ^~~~~~~~~~~
>>>
>>> This looks like false positive, assigning an initial value to
>>> 'aura_handle' to fix the build error.
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>> ---
>>> This is assuming assigning initial value won't have a performance affect
>>> if it does, we need to find another fix.
>>> And overall not sure why this false positive warning is generated.
>>> ---
>>> drivers/mempool/octeontx2/otx2_mempool_ops.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mempool/octeontx2/otx2_mempool_ops.c b/drivers/mempool/octeontx2/otx2_mempool_ops.c
>>> index 162b7f01da..078ffac3e2 100644
>>> --- a/drivers/mempool/octeontx2/otx2_mempool_ops.c
>>> +++ b/drivers/mempool/octeontx2/otx2_mempool_ops.c
>>> @@ -640,7 +640,7 @@ otx2_npa_alloc(struct rte_mempool *mp)
>>> struct otx2_npa_lf *lf;
>>> struct npa_aura_s aura;
>>> struct npa_pool_s pool;
>>> -uint64_t aura_handle;
>>> +uint64_t aura_handle = 0;
>>> size_t padding;
>>> int rc;
>>>
>>> --
>>
>>
>> Confirm that it helps, though I am seeing one more issue with EXTRA_CFLAGS='-O1' (gcc 7.3.0).
>> This time with drivers/event/octeontx2/otx2_evdev_stats.h:
>>
>> In file included from /local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/ot
>> x2_evdev.c:15:0:
>> /local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/otx2_evdev_stats.h: In function ‘otx2_sso_xstats_get’:
>> /local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/otx2_evdev_stats.h:124:9: error: ‘xstats’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>> xstat = &xstats[ids[i] - start_offset];
>> ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> /local/kananye1/dpdk.2005.rngf1/drivers/event/octeontx2/otx2_evdev.c: At top level:
>> cc1: error: unrecognized command line option ‘-Wno-address-of-packed-member’ [-Werror]
>> cc1: all warnings being treated as errors
>>
>> As I can see xtstats is not set for RTE_EVENT_DEV_XSTATS_DEVICE case.
>> Just to make gcc quiet, I added:
>> --- a/drivers/event/octeontx2/otx2_evdev_stats.h
>> +++ b/drivers/event/octeontx2/otx2_evdev_stats.h
>> @@ -67,6 +67,7 @@ otx2_sso_xstats_get(const struct rte_eventdev *event_dev,
>>
>> switch (mode) {
>> case RTE_EVENT_DEV_XSTATS_DEVICE:
>> + xstats = NULL;
>> break;
>> case RTE_EVENT_DEV_XSTATS_PORT:
>> if (queue_port_id >= (signed int)dev->nb_event_ports)
>>
>> Konstantin
>>
>>
>>
> Thanks Konstantin,
>
> I will make a v2 including above as you suggested.
>
Thinking twice, if compiler thinks 'xstats' may be used uninitialized, when is
assigned to NULL it should complain about null pointer de-reference.
Btw, this is also false positive, 'xstats_mode_count' prevents taking the loop
and accessing to 'xstats'. Not sure what is the problem with "-O1" option ...
But for the 'RTE_EVENT_DEV_XSTATS_DEVICE' case, the loop is skipped and
functions returns '0', I will update as following to get more close to the
intention, also this is less work for function.
Can you please test below when I send the patch, I can't reproduce this?
@@ -67,7 +67,7 @@ otx2_sso_xstats_get(const struct rte_eventdev *event_dev,
switch (mode) {
case RTE_EVENT_DEV_XSTATS_DEVICE:
- break;
+ return 0;
case RTE_EVENT_DEV_XSTATS_PORT:
if (queue_port_id >= (signed int)dev->nb_event_ports)
goto invalid_value;
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH v2 1/4] ring: fix build for gcc O1 optimization
2020-05-07 12:02 [dpdk-dev] [PATCH 1/3] ring: fix build for gcc O1 optimization Ferruh Yigit
` (2 preceding siblings ...)
2020-05-07 13:47 ` [dpdk-dev] [PATCH 1/3] ring: fix build for gcc " Ananyev, Konstantin
@ 2020-05-08 16:45 ` Ferruh Yigit
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 2/4] mempool/octeontx2: " Ferruh Yigit
` (2 more replies)
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc " Ferruh Yigit
4 siblings, 3 replies; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-08 16:45 UTC (permalink / raw)
To: Honnappa Nagarahalli, Konstantin Ananyev
Cc: dev, Ferruh Yigit, Thomas Monjalon, David Marchand
Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
Two build errors:
1)
In file included from .../build/include/rte_ring_elem.h:1093,
from .../lib/librte_rcu/rte_rcu_qsbr.c:21:
../lib/librte_rcu/rte_rcu_qsbr.c: In function ‘rte_rcu_qsbr_dq_reclaim’:
.../build/include/rte_ring_peek.h:282:22:
error: ‘avail’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
282 | *available = avail - n;
| ~~~~~~^~~
./build/include/rte_ring_peek.h:259:11: note: ‘avail’ was declared here
259 | uint32_t avail, head, next;
| ^~~~~
2)
In file included from .../build/include/rte_ring_elem.h:1093,
from .../build/include/rte_ring.h:405,
from .../app/test/test_ring_stress.h:13,
from .../app/test/test_ring_stress_impl.h:5,
from .../app/test/test_ring_peek_stress.c:5:
.../app/test/test_ring_peek_stress.c: In function ‘_st_ring_enqueue_bulk’:
.../build/include/rte_ring_peek.h:80:22:
error: ‘free’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
80 | *free_space = free - n;
| ~~~~~^~~
.../build/include/rte_ring_peek.h:60:11: note: ‘free’ was declared here
60 | uint32_t free, head, next;
| ^~~~
The cases shouldn't be hit, and it looks like there is already logic
error if it has been hit, but assigning 'avail' & 'free' to '0' to fix
the build error.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
lib/librte_ring/rte_ring_peek.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_ring/rte_ring_peek.h b/lib/librte_ring/rte_ring_peek.h
index d5e6ea1cf3..45f707dc7e 100644
--- a/lib/librte_ring/rte_ring_peek.h
+++ b/lib/librte_ring/rte_ring_peek.h
@@ -74,6 +74,7 @@ __rte_ring_do_enqueue_start(struct rte_ring *r, uint32_t n,
/* unsupported mode, shouldn't be here */
RTE_ASSERT(0);
n = 0;
+ free = 0;
}
if (free_space != NULL)
@@ -273,6 +274,7 @@ __rte_ring_do_dequeue_start(struct rte_ring *r, void *obj_table,
/* unsupported mode, shouldn't be here */
RTE_ASSERT(0);
n = 0;
+ avail = 0;
}
if (n != 0)
--
2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH v2 2/4] mempool/octeontx2: fix build for gcc O1 optimization
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 1/4] " Ferruh Yigit
@ 2020-05-08 16:45 ` Ferruh Yigit
2020-05-10 11:51 ` Jerin Jacob
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 3/4] net/ena: fix build for " Ferruh Yigit
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 4/4] event/octeontx2: " Ferruh Yigit
2 siblings, 1 reply; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-08 16:45 UTC (permalink / raw)
To: Jerin Jacob, Nithin Dabilpuram
Cc: dev, Ferruh Yigit, Thomas Monjalon, David Marchand
Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
Build error:
In file included from .../drivers/mempool/octeontx2/otx2_mempool.h:13,
from .../drivers/mempool/octeontx2/otx2_mempool_ops.c:8:
.../drivers/mempool/octeontx2/otx2_mempool_ops.c:
In function ‘otx2_npa_alloc’:
.../drivers/common/octeontx2/otx2_common.h:94:2:
error: ‘aura_handle’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
94 | rte_log(RTE_LOG_DEBUG, otx2_logtype_ ## subsystem, \
| ^~~~~~~
.../drivers/mempool/octeontx2/otx2_mempool_ops.c:643:11:
note: ‘aura_handle’ was declared here
643 | uint64_t aura_handle;
| ^~~~~~~~~~~
This looks like false positive, assigning an initial value to
'aura_handle' to fix the build error.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
This is assuming assigning initial value won't have a performance affect
if it does, we need to find another fix.
And overall not sure why this false positive warning is generated.
---
drivers/mempool/octeontx2/otx2_mempool_ops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mempool/octeontx2/otx2_mempool_ops.c b/drivers/mempool/octeontx2/otx2_mempool_ops.c
index 162b7f01da..078ffac3e2 100644
--- a/drivers/mempool/octeontx2/otx2_mempool_ops.c
+++ b/drivers/mempool/octeontx2/otx2_mempool_ops.c
@@ -640,7 +640,7 @@ otx2_npa_alloc(struct rte_mempool *mp)
struct otx2_npa_lf *lf;
struct npa_aura_s aura;
struct npa_pool_s pool;
- uint64_t aura_handle;
+ uint64_t aura_handle = 0;
size_t padding;
int rc;
--
2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH v2 3/4] net/ena: fix build for O1 optimization
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 1/4] " Ferruh Yigit
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 2/4] mempool/octeontx2: " Ferruh Yigit
@ 2020-05-08 16:45 ` Ferruh Yigit
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 4/4] event/octeontx2: " Ferruh Yigit
2 siblings, 0 replies; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-08 16:45 UTC (permalink / raw)
To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin,
Igor Chauskin
Cc: dev, Ferruh Yigit, Thomas Monjalon, David Marchand
Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
Build error:
.../drivers/net/ena/ena_ethdev.c: In function ‘eth_ena_dev_init’:
.../drivers/net/ena/ena_ethdev.c:1815:20:
error: ‘wd_state’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
1815 | adapter->wd_state = wd_state;
| ~~~~~~~~~~~~~~~~~~^~~~~~~~~~
This looks like false positive, fixing by assigning initial value to
'wd_state' variable.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
drivers/net/ena/ena_ethdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index c3fd3a4ac0..fbddc79f79 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1756,7 +1756,7 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
int rc;
static int adapters_found;
bool disable_meta_caching;
- bool wd_state;
+ bool wd_state = false;
eth_dev->dev_ops = &ena_dev_ops;
eth_dev->rx_pkt_burst = ð_ena_recv_pkts;
--
2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH v2 4/4] event/octeontx2: fix build for O1 optimization
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 1/4] " Ferruh Yigit
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 2/4] mempool/octeontx2: " Ferruh Yigit
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 3/4] net/ena: fix build for " Ferruh Yigit
@ 2020-05-08 16:45 ` Ferruh Yigit
2020-05-08 17:22 ` Ananyev, Konstantin
2 siblings, 1 reply; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-08 16:45 UTC (permalink / raw)
To: Pavan Nikhilesh, Jerin Jacob
Cc: dev, Ferruh Yigit, Thomas Monjalon, David Marchand, Konstantin Ananyev
Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
gcc 7.3.0
Build error
In file included from .../drivers/event/octeontx2/ot
x2_evdev.c:15:0:
.../drivers/event/octeontx2/otx2_evdev_stats.h:
In function ‘otx2_sso_xstats_get’:
.../drivers/event/octeontx2/otx2_evdev_stats.h:124:9:
error: ‘xstats’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
xstat = &xstats[ids[i] - start_offset];
~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This is false positive, 'xstats_mode_count' should be preventing taking
the loop and accessing 'xstats'.
Returning in that case to silence the compiler warning.
Reported-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
drivers/event/octeontx2/otx2_evdev_stats.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/event/octeontx2/otx2_evdev_stats.h b/drivers/event/octeontx2/otx2_evdev_stats.h
index 9d7c694ee6..74fcec8a07 100644
--- a/drivers/event/octeontx2/otx2_evdev_stats.h
+++ b/drivers/event/octeontx2/otx2_evdev_stats.h
@@ -67,7 +67,7 @@ otx2_sso_xstats_get(const struct rte_eventdev *event_dev,
switch (mode) {
case RTE_EVENT_DEV_XSTATS_DEVICE:
- break;
+ return 0;
case RTE_EVENT_DEV_XSTATS_PORT:
if (queue_port_id >= (signed int)dev->nb_event_ports)
goto invalid_value;
--
2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dpdk-dev] [PATCH v2 4/4] event/octeontx2: fix build for O1 optimization
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 4/4] event/octeontx2: " Ferruh Yigit
@ 2020-05-08 17:22 ` Ananyev, Konstantin
2020-05-10 11:53 ` Jerin Jacob
0 siblings, 1 reply; 20+ messages in thread
From: Ananyev, Konstantin @ 2020-05-08 17:22 UTC (permalink / raw)
To: Yigit, Ferruh, Pavan Nikhilesh, Jerin Jacob
Cc: dev, Thomas Monjalon, David Marchand
> Subject: [PATCH v2 4/4] event/octeontx2: fix build for O1 optimization
>
> Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
> gcc 7.3.0
>
> Build error
> In file included from .../drivers/event/octeontx2/ot
> x2_evdev.c:15:0:
> .../drivers/event/octeontx2/otx2_evdev_stats.h:
> In function ‘otx2_sso_xstats_get’:
> .../drivers/event/octeontx2/otx2_evdev_stats.h:124:9:
> error: ‘xstats’ may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
> xstat = &xstats[ids[i] - start_offset];
> ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This is false positive, 'xstats_mode_count' should be preventing taking
> the loop and accessing 'xstats'.
> Returning in that case to silence the compiler warning.
>
> Reported-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> drivers/event/octeontx2/otx2_evdev_stats.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/event/octeontx2/otx2_evdev_stats.h b/drivers/event/octeontx2/otx2_evdev_stats.h
> index 9d7c694ee6..74fcec8a07 100644
> --- a/drivers/event/octeontx2/otx2_evdev_stats.h
> +++ b/drivers/event/octeontx2/otx2_evdev_stats.h
> @@ -67,7 +67,7 @@ otx2_sso_xstats_get(const struct rte_eventdev *event_dev,
>
> switch (mode) {
> case RTE_EVENT_DEV_XSTATS_DEVICE:
> - break;
> + return 0;
> case RTE_EVENT_DEV_XSTATS_PORT:
> if (queue_port_id >= (signed int)dev->nb_event_ports)
> goto invalid_value;
> --
Tested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dpdk-dev] [PATCH v2 2/4] mempool/octeontx2: fix build for gcc O1 optimization
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 2/4] mempool/octeontx2: " Ferruh Yigit
@ 2020-05-10 11:51 ` Jerin Jacob
0 siblings, 0 replies; 20+ messages in thread
From: Jerin Jacob @ 2020-05-10 11:51 UTC (permalink / raw)
To: Ferruh Yigit
Cc: Jerin Jacob, Nithin Dabilpuram, dpdk-dev, Thomas Monjalon,
David Marchand
On Fri, May 8, 2020 at 10:16 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
> gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
>
> Build error:
> In file included from .../drivers/mempool/octeontx2/otx2_mempool.h:13,
> from .../drivers/mempool/octeontx2/otx2_mempool_ops.c:8:
> .../drivers/mempool/octeontx2/otx2_mempool_ops.c:
> In function ‘otx2_npa_alloc’:
> .../drivers/common/octeontx2/otx2_common.h:94:2:
> error: ‘aura_handle’ may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
> 94 | rte_log(RTE_LOG_DEBUG, otx2_logtype_ ## subsystem, \
> | ^~~~~~~
> .../drivers/mempool/octeontx2/otx2_mempool_ops.c:643:11:
> note: ‘aura_handle’ was declared here
> 643 | uint64_t aura_handle;
> | ^~~~~~~~~~~
>
> This looks like false positive, assigning an initial value to
> 'aura_handle' to fix the build error.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> This is assuming assigning initial value won't have a performance affect
> if it does, we need to find another fix.
> And overall not sure why this false positive warning is generated.
> ---
> drivers/mempool/octeontx2/otx2_mempool_ops.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mempool/octeontx2/otx2_mempool_ops.c b/drivers/mempool/octeontx2/otx2_mempool_ops.c
> index 162b7f01da..078ffac3e2 100644
> --- a/drivers/mempool/octeontx2/otx2_mempool_ops.c
> +++ b/drivers/mempool/octeontx2/otx2_mempool_ops.c
> @@ -640,7 +640,7 @@ otx2_npa_alloc(struct rte_mempool *mp)
> struct otx2_npa_lf *lf;
> struct npa_aura_s aura;
> struct npa_pool_s pool;
> - uint64_t aura_handle;
> + uint64_t aura_handle = 0;
It is a false positive. Since it is on a slow path, This workaround is OK.
While merging t this patch, Could you move "uint64_t aura_handle = 0;"
just before "struct otx2_npa_lf *lf" as
the variables are ordered in reverse xmas tree format in this file.
With the above change:
Acked-by: Jerin Jacob <jerinj@marvell.com>
> size_t padding;
> int rc;
>
> --
> 2.25.4
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dpdk-dev] [PATCH v2 4/4] event/octeontx2: fix build for O1 optimization
2020-05-08 17:22 ` Ananyev, Konstantin
@ 2020-05-10 11:53 ` Jerin Jacob
0 siblings, 0 replies; 20+ messages in thread
From: Jerin Jacob @ 2020-05-10 11:53 UTC (permalink / raw)
To: Ananyev, Konstantin
Cc: Yigit, Ferruh, Pavan Nikhilesh, Jerin Jacob, dev,
Thomas Monjalon, David Marchand
On Fri, May 8, 2020 at 10:52 PM Ananyev, Konstantin
<konstantin.ananyev@intel.com> wrote:
>
>
>
> > Subject: [PATCH v2 4/4] event/octeontx2: fix build for O1 optimization
> >
> > Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
> > gcc 7.3.0
> >
> > Build error
> > In file included from .../drivers/event/octeontx2/ot
> > x2_evdev.c:15:0:
> > .../drivers/event/octeontx2/otx2_evdev_stats.h:
> > In function ‘otx2_sso_xstats_get’:
> > .../drivers/event/octeontx2/otx2_evdev_stats.h:124:9:
> > error: ‘xstats’ may be used uninitialized in this function
> > [-Werror=maybe-uninitialized]
> > xstat = &xstats[ids[i] - start_offset];
> > ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > This is false positive, 'xstats_mode_count' should be preventing taking
> > the loop and accessing 'xstats'.
> > Returning in that case to silence the compiler warning.
> >
> > Reported-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> > drivers/event/octeontx2/otx2_evdev_stats.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/event/octeontx2/otx2_evdev_stats.h b/drivers/event/octeontx2/otx2_evdev_stats.h
> > index 9d7c694ee6..74fcec8a07 100644
> > --- a/drivers/event/octeontx2/otx2_evdev_stats.h
> > +++ b/drivers/event/octeontx2/otx2_evdev_stats.h
> > @@ -67,7 +67,7 @@ otx2_sso_xstats_get(const struct rte_eventdev *event_dev,
> >
> > switch (mode) {
> > case RTE_EVENT_DEV_XSTATS_DEVICE:
> > - break;
> > + return 0;
> > case RTE_EVENT_DEV_XSTATS_PORT:
> > if (queue_port_id >= (signed int)dev->nb_event_ports)
> > goto invalid_value;
> > --
>
> Tested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
>
> > 2.25.4
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc O1 optimization
2020-05-07 12:02 [dpdk-dev] [PATCH 1/3] ring: fix build for gcc O1 optimization Ferruh Yigit
` (3 preceding siblings ...)
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 1/4] " Ferruh Yigit
@ 2020-05-11 16:07 ` Ferruh Yigit
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 2/4] mempool/octeontx2: " Ferruh Yigit
` (3 more replies)
4 siblings, 4 replies; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-11 16:07 UTC (permalink / raw)
To: Honnappa Nagarahalli, Konstantin Ananyev
Cc: dev, Ferruh Yigit, Thomas Monjalon, David Marchand
Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
Two build errors:
1)
In file included from .../build/include/rte_ring_elem.h:1093,
from .../lib/librte_rcu/rte_rcu_qsbr.c:21:
../lib/librte_rcu/rte_rcu_qsbr.c: In function ‘rte_rcu_qsbr_dq_reclaim’:
.../build/include/rte_ring_peek.h:282:22:
error: ‘avail’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
282 | *available = avail - n;
| ~~~~~~^~~
./build/include/rte_ring_peek.h:259:11: note: ‘avail’ was declared here
259 | uint32_t avail, head, next;
| ^~~~~
2)
In file included from .../build/include/rte_ring_elem.h:1093,
from .../build/include/rte_ring.h:405,
from .../app/test/test_ring_stress.h:13,
from .../app/test/test_ring_stress_impl.h:5,
from .../app/test/test_ring_peek_stress.c:5:
.../app/test/test_ring_peek_stress.c: In function ‘_st_ring_enqueue_bulk’:
.../build/include/rte_ring_peek.h:80:22:
error: ‘free’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
80 | *free_space = free - n;
| ~~~~~^~~
.../build/include/rte_ring_peek.h:60:11: note: ‘free’ was declared here
60 | uint32_t free, head, next;
| ^~~~
The cases shouldn't be hit, and it looks like there is already logic
error if it has been hit, but assigning 'avail' & 'free' to '0' to fix
the build error.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
lib/librte_ring/rte_ring_peek.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_ring/rte_ring_peek.h b/lib/librte_ring/rte_ring_peek.h
index d5e6ea1cf3..45f707dc7e 100644
--- a/lib/librte_ring/rte_ring_peek.h
+++ b/lib/librte_ring/rte_ring_peek.h
@@ -74,6 +74,7 @@ __rte_ring_do_enqueue_start(struct rte_ring *r, uint32_t n,
/* unsupported mode, shouldn't be here */
RTE_ASSERT(0);
n = 0;
+ free = 0;
}
if (free_space != NULL)
@@ -273,6 +274,7 @@ __rte_ring_do_dequeue_start(struct rte_ring *r, void *obj_table,
/* unsupported mode, shouldn't be here */
RTE_ASSERT(0);
n = 0;
+ avail = 0;
}
if (n != 0)
--
2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH v3 2/4] mempool/octeontx2: fix build for gcc O1 optimization
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc " Ferruh Yigit
@ 2020-05-11 16:07 ` Ferruh Yigit
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 3/4] net/ena: fix build for " Ferruh Yigit
` (2 subsequent siblings)
3 siblings, 0 replies; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-11 16:07 UTC (permalink / raw)
To: Jerin Jacob, Nithin Dabilpuram
Cc: dev, Ferruh Yigit, Thomas Monjalon, David Marchand
Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
Build error:
In file included from .../drivers/mempool/octeontx2/otx2_mempool.h:13,
from .../drivers/mempool/octeontx2/otx2_mempool_ops.c:8:
.../drivers/mempool/octeontx2/otx2_mempool_ops.c:
In function ‘otx2_npa_alloc’:
.../drivers/common/octeontx2/otx2_common.h:94:2:
error: ‘aura_handle’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
94 | rte_log(RTE_LOG_DEBUG, otx2_logtype_ ## subsystem, \
| ^~~~~~~
.../drivers/mempool/octeontx2/otx2_mempool_ops.c:643:11:
note: ‘aura_handle’ was declared here
643 | uint64_t aura_handle;
| ^~~~~~~~~~~
This looks like false positive, assigning an initial value to
'aura_handle' to fix the build error.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
This is assuming assigning initial value won't have a performance affect
if it does, we need to find another fix.
And overall not sure why this false positive warning is generated.
---
drivers/mempool/octeontx2/otx2_mempool_ops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mempool/octeontx2/otx2_mempool_ops.c b/drivers/mempool/octeontx2/otx2_mempool_ops.c
index 162b7f01da..a14c9c5e94 100644
--- a/drivers/mempool/octeontx2/otx2_mempool_ops.c
+++ b/drivers/mempool/octeontx2/otx2_mempool_ops.c
@@ -637,10 +637,10 @@ static int
otx2_npa_alloc(struct rte_mempool *mp)
{
uint32_t block_size, block_count;
+ uint64_t aura_handle = 0;
struct otx2_npa_lf *lf;
struct npa_aura_s aura;
struct npa_pool_s pool;
- uint64_t aura_handle;
size_t padding;
int rc;
--
2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH v3 3/4] net/ena: fix build for O1 optimization
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc " Ferruh Yigit
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 2/4] mempool/octeontx2: " Ferruh Yigit
@ 2020-05-11 16:07 ` Ferruh Yigit
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 4/4] event/octeontx2: " Ferruh Yigit
2020-05-11 19:13 ` [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc " Thomas Monjalon
3 siblings, 0 replies; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-11 16:07 UTC (permalink / raw)
To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin,
Igor Chauskin
Cc: dev, Ferruh Yigit, Thomas Monjalon, David Marchand
Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
Build error:
.../drivers/net/ena/ena_ethdev.c: In function ‘eth_ena_dev_init’:
.../drivers/net/ena/ena_ethdev.c:1815:20:
error: ‘wd_state’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
1815 | adapter->wd_state = wd_state;
| ~~~~~~~~~~~~~~~~~~^~~~~~~~~~
This looks like false positive, fixing by assigning initial value to
'wd_state' variable.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
drivers/net/ena/ena_ethdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index c3fd3a4ac0..fbddc79f79 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1756,7 +1756,7 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
int rc;
static int adapters_found;
bool disable_meta_caching;
- bool wd_state;
+ bool wd_state = false;
eth_dev->dev_ops = &ena_dev_ops;
eth_dev->rx_pkt_burst = ð_ena_recv_pkts;
--
2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [dpdk-dev] [PATCH v3 4/4] event/octeontx2: fix build for O1 optimization
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc " Ferruh Yigit
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 2/4] mempool/octeontx2: " Ferruh Yigit
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 3/4] net/ena: fix build for " Ferruh Yigit
@ 2020-05-11 16:07 ` Ferruh Yigit
2020-05-11 19:13 ` [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc " Thomas Monjalon
3 siblings, 0 replies; 20+ messages in thread
From: Ferruh Yigit @ 2020-05-11 16:07 UTC (permalink / raw)
To: Pavan Nikhilesh, Jerin Jacob
Cc: dev, Ferruh Yigit, Thomas Monjalon, David Marchand, Konstantin Ananyev
Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
gcc 7.3.0
Build error
In file included from .../drivers/event/octeontx2/ot
x2_evdev.c:15:0:
.../drivers/event/octeontx2/otx2_evdev_stats.h:
In function ‘otx2_sso_xstats_get’:
.../drivers/event/octeontx2/otx2_evdev_stats.h:124:9:
error: ‘xstats’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
xstat = &xstats[ids[i] - start_offset];
~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This is false positive, 'xstats_mode_count' should be preventing taking
the loop and accessing 'xstats'.
Returning in that case to silence the compiler warning.
Reported-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Tested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
drivers/event/octeontx2/otx2_evdev_stats.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/event/octeontx2/otx2_evdev_stats.h b/drivers/event/octeontx2/otx2_evdev_stats.h
index 9d7c694ee6..74fcec8a07 100644
--- a/drivers/event/octeontx2/otx2_evdev_stats.h
+++ b/drivers/event/octeontx2/otx2_evdev_stats.h
@@ -67,7 +67,7 @@ otx2_sso_xstats_get(const struct rte_eventdev *event_dev,
switch (mode) {
case RTE_EVENT_DEV_XSTATS_DEVICE:
- break;
+ return 0;
case RTE_EVENT_DEV_XSTATS_PORT:
if (queue_port_id >= (signed int)dev->nb_event_ports)
goto invalid_value;
--
2.25.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc O1 optimization
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc " Ferruh Yigit
` (2 preceding siblings ...)
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 4/4] event/octeontx2: " Ferruh Yigit
@ 2020-05-11 19:13 ` Thomas Monjalon
3 siblings, 0 replies; 20+ messages in thread
From: Thomas Monjalon @ 2020-05-11 19:13 UTC (permalink / raw)
To: Ferruh Yigit
Cc: Honnappa Nagarahalli, Konstantin Ananyev, dev, David Marchand
11/05/2020 18:07, Ferruh Yigit:
> Can be reproduced with "make EXTRA_CFLAGS='-O1'" command using
> gcc (GCC) 9.3.1 20200408 (Red Hat 9.3.1-2)
Series applied, thanks
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2020-05-11 19:13 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 12:02 [dpdk-dev] [PATCH 1/3] ring: fix build for gcc O1 optimization Ferruh Yigit
2020-05-07 12:02 ` [dpdk-dev] [PATCH 2/3] mempool/octeontx2: " Ferruh Yigit
2020-05-07 14:05 ` Ananyev, Konstantin
2020-05-08 16:20 ` Ferruh Yigit
2020-05-08 16:39 ` Ferruh Yigit
2020-05-07 12:02 ` [dpdk-dev] [PATCH 3/3] net/ena: fix build for " Ferruh Yigit
2020-05-08 13:59 ` Michał Krawczyk
2020-05-07 13:47 ` [dpdk-dev] [PATCH 1/3] ring: fix build for gcc " Ananyev, Konstantin
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 1/4] " Ferruh Yigit
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 2/4] mempool/octeontx2: " Ferruh Yigit
2020-05-10 11:51 ` Jerin Jacob
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 3/4] net/ena: fix build for " Ferruh Yigit
2020-05-08 16:45 ` [dpdk-dev] [PATCH v2 4/4] event/octeontx2: " Ferruh Yigit
2020-05-08 17:22 ` Ananyev, Konstantin
2020-05-10 11:53 ` Jerin Jacob
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc " Ferruh Yigit
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 2/4] mempool/octeontx2: " Ferruh Yigit
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 3/4] net/ena: fix build for " Ferruh Yigit
2020-05-11 16:07 ` [dpdk-dev] [PATCH v3 4/4] event/octeontx2: " Ferruh Yigit
2020-05-11 19:13 ` [dpdk-dev] [PATCH v3 1/4] ring: fix build for gcc " Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).