* [dpdk-dev] [PATCH v2] power: fix unused-but-set variable error
@ 2021-11-05 15:53 Jim Harris
2021-11-08 20:40 ` David Marchand
2021-11-10 10:55 ` [dpdk-dev] " Jiang, YuX
0 siblings, 2 replies; 6+ messages in thread
From: Jim Harris @ 2021-11-05 15:53 UTC (permalink / raw)
To: dev, david.hunt, david.marchand; +Cc: Jim Harris
clang-13 rightfully complains that the tot_ppi
variable in update_stats is set but not used, since
the final accumulated tot_ppi results isn't used
anywhere.
Original idea was to just remove the tot_ppi variable,
but feedback from David Marchand on mailing list was
that the related ppi_av array in struct priority_worker
wasn't useful and should be removed. This allows us
to also remove num_dequeue_pkts_prev and pc from
struct priority_worker since they are only used in
conjunction with the ppi_av array.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
---
lib/power/rte_power_empty_poll.c | 12 +-----------
lib/power/rte_power_empty_poll.h | 5 -----
2 files changed, 1 insertion(+), 16 deletions(-)
diff --git a/lib/power/rte_power_empty_poll.c b/lib/power/rte_power_empty_poll.c
index 975aa92997..c4b5de9601 100644
--- a/lib/power/rte_power_empty_poll.c
+++ b/lib/power/rte_power_empty_poll.c
@@ -72,8 +72,6 @@ enter_normal_state(struct priority_worker *poll_stats)
/* Clear the averages arrays and strs */
memset(poll_stats->edpi_av, 0, sizeof(poll_stats->edpi_av));
poll_stats->ec = 0;
- memset(poll_stats->ppi_av, 0, sizeof(poll_stats->ppi_av));
- poll_stats->pc = 0;
poll_stats->cur_freq = MED;
poll_stats->iter_counter = 0;
@@ -91,8 +89,6 @@ enter_busy_state(struct priority_worker *poll_stats)
{
memset(poll_stats->edpi_av, 0, sizeof(poll_stats->edpi_av));
poll_stats->ec = 0;
- memset(poll_stats->ppi_av, 0, sizeof(poll_stats->ppi_av));
- poll_stats->pc = 0;
poll_stats->cur_freq = HGH;
poll_stats->iter_counter = 0;
@@ -207,7 +203,7 @@ update_training_stats(struct priority_worker *poll_stats,
static __rte_always_inline uint32_t
update_stats(struct priority_worker *poll_stats)
{
- uint64_t tot_edpi = 0, tot_ppi = 0;
+ uint64_t tot_edpi = 0;
uint32_t j, percent;
struct priority_worker *s = poll_stats;
@@ -216,10 +212,6 @@ update_stats(struct priority_worker *poll_stats)
s->empty_dequeues_prev = s->empty_dequeues;
- uint64_t ppi = s->num_dequeue_pkts - s->num_dequeue_pkts_prev;
-
- s->num_dequeue_pkts_prev = s->num_dequeue_pkts;
-
if (s->thresh[s->cur_freq].base_edpi < cur_edpi) {
/* edpi mean empty poll counter difference per interval */
@@ -233,11 +225,9 @@ update_stats(struct priority_worker *poll_stats)
}
s->edpi_av[s->ec++ % BINS_AV] = cur_edpi;
- s->ppi_av[s->pc++ % BINS_AV] = ppi;
for (j = 0; j < BINS_AV; j++) {
tot_edpi += s->edpi_av[j];
- tot_ppi += s->ppi_av[j];
}
tot_edpi = tot_edpi / BINS_AV;
diff --git a/lib/power/rte_power_empty_poll.h b/lib/power/rte_power_empty_poll.h
index 6ba0a37074..43fb276077 100644
--- a/lib/power/rte_power_empty_poll.h
+++ b/lib/power/rte_power_empty_poll.h
@@ -71,7 +71,6 @@ struct priority_worker {
enum queue_state queue_state;
uint64_t empty_dequeues_prev;
- uint64_t num_dequeue_pkts_prev;
/* Used for training only */
struct freq_threshold thresh[NUM_FREQ];
@@ -82,10 +81,6 @@ struct priority_worker {
uint64_t edpi_av[BINS_AV];
/* empty poll counter */
uint32_t ec;
- /* ppi mean valid poll counter per interval */
- uint64_t ppi_av[BINS_AV];
- /* valid poll counter */
- uint32_t pc;
uint32_t lcore_id;
uint32_t iter_counter;
--
2.32.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v2] power: fix unused-but-set variable error
2021-11-05 15:53 [dpdk-dev] [PATCH v2] power: fix unused-but-set variable error Jim Harris
@ 2021-11-08 20:40 ` David Marchand
2021-11-12 14:46 ` David Marchand
2021-11-10 10:55 ` [dpdk-dev] " Jiang, YuX
1 sibling, 1 reply; 6+ messages in thread
From: David Marchand @ 2021-11-08 20:40 UTC (permalink / raw)
To: Jim Harris; +Cc: dev, David Hunt
On Fri, Nov 5, 2021 at 4:57 PM Jim Harris <james.r.harris@intel.com> wrote:
>
> clang-13 rightfully complains that the tot_ppi
> variable in update_stats is set but not used, since
> the final accumulated tot_ppi results isn't used
> anywhere.
>
> Original idea was to just remove the tot_ppi variable,
> but feedback from David Marchand on mailing list was
> that the related ppi_av array in struct priority_worker
> wasn't useful and should be removed. This allows us
> to also remove num_dequeue_pkts_prev and pc from
> struct priority_worker since they are only used in
> conjunction with the ppi_av array.
This library has its internals exposed to the world for no good reason.
I am pretty sure all those structures could be hidden in the .c.
It is still an experimental API: we can hope it gets cleaned up at some point.
This patch that drops dead code is a first step and lgtm.
>
> Signed-off-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v2] power: fix unused-but-set variable error
2021-11-05 15:53 [dpdk-dev] [PATCH v2] power: fix unused-but-set variable error Jim Harris
2021-11-08 20:40 ` David Marchand
@ 2021-11-10 10:55 ` Jiang, YuX
2021-11-10 11:07 ` David Marchand
1 sibling, 1 reply; 6+ messages in thread
From: Jiang, YuX @ 2021-11-10 10:55 UTC (permalink / raw)
To: Harris, James R, dev, Hunt, David, david.marchand; +Cc: Harris, James R
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Jim Harris
> Sent: Friday, November 5, 2021 11:54 PM
> To: dev@dpdk.org; Hunt, David <david.hunt@intel.com>;
> david.marchand@redhat.com
> Cc: Harris, James R <james.r.harris@intel.com>
> Subject: [dpdk-dev] [PATCH v2] power: fix unused-but-set variable error
>
> clang-13 rightfully complains that the tot_ppi variable in update_stats is set
> but not used, since the final accumulated tot_ppi results isn't used anywhere.
>
> Original idea was to just remove the tot_ppi variable, but feedback from
> David Marchand on mailing list was that the related ppi_av array in struct
> priority_worker wasn't useful and should be removed. This allows us to also
> remove num_dequeue_pkts_prev and pc from struct priority_worker since
> they are only used in conjunction with the ppi_av array.
>
> Signed-off-by: Jim Harris <james.r.harris@intel.com>
> ---
Hi Jim and David,
We verify this patch http://patches.dpdk.org/project/dpdk/patch/20211105155351.350403-1-james.r.harris@intel.com/ based on dpdk21.11-rc2 main branch(f4801fdb7828f45ddc4ada148b1b0a230a226570), still verify failed.
Could you pls have a look again?
Verified OS details:
Tested Failed on OS Fedora35/Kernel 5.14.16-301.fc35.x86_64/gcc version11.2.1/clang version 13.0.0/ninja version 1.10.2
Failed log details:
[root@dpdk-Fedora34-64-gcc dpdk]# ninja -C x86_64-native-linuxapp-clang/
ninja: Entering directory `x86_64-native-linuxapp-clang/'
[686/3119] Compiling C object drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o
FAILED: drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o
clang -Idrivers/libtmp_rte_bus_fslmc.a.p -Idrivers -I../drivers -Idrivers/bus/fslmc -I../drivers/bus/fslmc -I../drivers/bus/fslmc/mc -I../drivers/bus/fslmc/qbman/include -I../drivers/bus/fslmc/portal -I. -I.. -Iconfig -I../config -Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include -Ilib/eal/x86/include -I../lib/eal/x86/include -Ilib/eal/common -I../lib/eal/common -Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs -Ilib/metrics -I../lib/metrics -Ilib/telemetry -I../lib/telemetry -Idrivers/common/dpaax -I../drivers/common/dpaax -I../drivers/common/dpaax/caamflib -Ilib/eventdev -I../lib/eventdev -Ilib/ring -I../lib/ring -Ilib/ethdev -I../lib/ethdev -Ilib/net -I../lib/net -Ilib/mbuf -I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/meter -I../lib/meter -Ilib/hash -I../lib/hash -Ilib/rcu -I../lib/rcu -Ilib/timer -I../lib/timer -Ilib/cryptodev -I../lib/cryptodev -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Werror -O3 -include rte_config.h -Wextra -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-packed-member -Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=native -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -DRTE_LOG_DEFAULT_LOGTYPE=bus.fslmc -MD -MQ drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o -MF drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o.d -o drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o -c ../drivers/bus/fslmc/fslmc_bus.c
../drivers/bus/fslmc/fslmc_bus.c:318:6: error: variable 'device_count' set but not used [-Werror,-Wunused-but-set-variable]
int device_count = 0;
^
1 error generated.
[691/3119] Generating rte_bus_dpaa.sym_chk with a custom command (wrapped by meson to capture output)
ninja: build stopped: subcommand failed.bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v2] power: fix unused-but-set variable error
2021-11-10 10:55 ` [dpdk-dev] " Jiang, YuX
@ 2021-11-10 11:07 ` David Marchand
2021-11-10 11:34 ` Jiang, YuX
0 siblings, 1 reply; 6+ messages in thread
From: David Marchand @ 2021-11-10 11:07 UTC (permalink / raw)
To: Jiang, YuX; +Cc: Harris, James R, dev, Hunt, David
On Wed, Nov 10, 2021 at 11:56 AM Jiang, YuX <yux.jiang@intel.com> wrote:
> We verify this patch http://patches.dpdk.org/project/dpdk/patch/20211105155351.350403-1-james.r.harris@intel.com/ based on dpdk21.11-rc2 main branch(f4801fdb7828f45ddc4ada148b1b0a230a226570), still verify failed.
> Could you pls have a look again?
>
> Verified OS details:
> Tested Failed on OS Fedora35/Kernel 5.14.16-301.fc35.x86_64/gcc version11.2.1/clang version 13.0.0/ninja version 1.10.2
>
> Failed log details:
> [root@dpdk-Fedora34-64-gcc dpdk]# ninja -C x86_64-native-linuxapp-clang/
Is it fc34 or fc35?
> ninja: Entering directory `x86_64-native-linuxapp-clang/'
> [686/3119] Compiling C object drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o
> FAILED: drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o
> clang -Idrivers/libtmp_rte_bus_fslmc.a.p -Idrivers -I../drivers -Idrivers/bus/fslmc -I../drivers/bus/fslmc -I../drivers/bus/fslmc/mc -I../drivers/bus/fslmc/qbman/include -I../drivers/bus/fslmc/portal -I. -I.. -Iconfig -I../config -Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include -Ilib/eal/x86/include -I../lib/eal/x86/include -Ilib/eal/common -I../lib/eal/common -Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs -Ilib/metrics -I../lib/metrics -Ilib/telemetry -I../lib/telemetry -Idrivers/common/dpaax -I../drivers/common/dpaax -I../drivers/common/dpaax/caamflib -Ilib/eventdev -I../lib/eventdev -Ilib/ring -I../lib/ring -Ilib/ethdev -I../lib/ethdev -Ilib/net -I../lib/net -Ilib/mbuf -I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/meter -I../lib/meter -Ilib/hash -I../lib/hash -Ilib/rcu -I../lib/rcu -Ilib/timer -I../lib/timer -Ilib/cryptodev -I../lib/cryptodev -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Werror -O3 -include rte_config.h -Wextra -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-packed-member -Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=native -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -DRTE_LOG_DEFAULT_LOGTYPE=bus.fslmc -MD -MQ drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o -MF drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o.d -o drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o -c ../drivers/bus/fslmc/fslmc_bus.c
> ../drivers/bus/fslmc/fslmc_bus.c:318:6: error: variable 'device_count' set but not used [-Werror,-Wunused-but-set-variable]
> int device_count = 0;
> ^
> 1 error generated.
> [691/3119] Generating rte_bus_dpaa.sym_chk with a custom command (wrapped by meson to capture output)
> ninja: build stopped: subcommand failed.bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o
Thanks for reporting.
The issue on fslmc is not related to Jim patch.
If there is no bugzilla opened for this issue, please open a new one.
Thanks.
--
David Marchand
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v2] power: fix unused-but-set variable error
2021-11-10 11:07 ` David Marchand
@ 2021-11-10 11:34 ` Jiang, YuX
0 siblings, 0 replies; 6+ messages in thread
From: Jiang, YuX @ 2021-11-10 11:34 UTC (permalink / raw)
To: David Marchand; +Cc: Harris, James R, dev, Hunt, David
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, November 10, 2021 7:08 PM
> To: Jiang, YuX <yux.jiang@intel.com>
> Cc: Harris, James R <james.r.harris@intel.com>; dev@dpdk.org; Hunt, David
> <david.hunt@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2] power: fix unused-but-set variable error
>
> On Wed, Nov 10, 2021 at 11:56 AM Jiang, YuX <yux.jiang@intel.com> wrote:
> > We verify this patch
> http://patches.dpdk.org/project/dpdk/patch/20211105155351.350403-1-
> james.r.harris@intel.com/ based on dpdk21.11-rc2 main
> branch(f4801fdb7828f45ddc4ada148b1b0a230a226570), still verify failed.
> > Could you pls have a look again?
> >
> > Verified OS details:
> > Tested Failed on OS Fedora35/Kernel 5.14.16-301.fc35.x86_64/gcc
> version11.2.1/clang version 13.0.0/ninja version 1.10.2
> >
> > Failed log details:
> > [root@dpdk-Fedora34-64-gcc dpdk]# ninja -C x86_64-native-linuxapp-
> clang/
>
> Is it fc34 or fc35?
>
> > ninja: Entering directory `x86_64-native-linuxapp-clang/'
> > [686/3119] Compiling C object
> drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o
> > FAILED: drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o
> > clang -Idrivers/libtmp_rte_bus_fslmc.a.p -Idrivers -I../drivers -
> Idrivers/bus/fslmc -I../drivers/bus/fslmc -I../drivers/bus/fslmc/mc -
> I../drivers/bus/fslmc/qbman/include -I../drivers/bus/fslmc/portal -I. -I.. -
> Iconfig -I../config -Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -
> I../lib/eal/linux/include -Ilib/eal/x86/include -I../lib/eal/x86/include -
> Ilib/eal/common -I../lib/eal/common -Ilib/eal -I../lib/eal -Ilib/kvargs -
> I../lib/kvargs -Ilib/metrics -I../lib/metrics -Ilib/telemetry -I../lib/telemetry -
> Idrivers/common/dpaax -I../drivers/common/dpaax -
> I../drivers/common/dpaax/caamflib -Ilib/eventdev -I../lib/eventdev -
> Ilib/ring -I../lib/ring -Ilib/ethdev -I../lib/ethdev -Ilib/net -I../lib/net -Ilib/mbuf
> -I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/meter -I../lib/meter -
> Ilib/hash -I../lib/hash -Ilib/rcu -I../lib/rcu -Ilib/timer -I../lib/timer -
> Ilib/cryptodev -I../lib/cryptodev -fcolor-diagnostics -D_FILE_OFFSET_BITS=64
> -Wall -Winvalid-pch -Werror -O3 -include rte_config.h -Wextra -Wcast-qual -
> Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-
> declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition
> -Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-
> strings -Wno-address-of-packed-member -Wno-missing-field-initializers -
> D_GNU_SOURCE -fPIC -march=native -DALLOW_EXPERIMENTAL_API -
> DALLOW_INTERNAL_API -DRTE_LOG_DEFAULT_LOGTYPE=bus.fslmc -MD -
> MQ drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o -MF
> drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o.d -o
> drivers/libtmp_rte_bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o -
> c ../drivers/bus/fslmc/fslmc_bus.c
> > ../drivers/bus/fslmc/fslmc_bus.c:318:6: error: variable 'device_count' set
> but not used [-Werror,-Wunused-but-set-variable]
> > int device_count = 0;
> > ^
> > 1 error generated.
> > [691/3119] Generating rte_bus_dpaa.sym_chk with a custom command
> (wrapped by meson to capture output)
> > ninja: build stopped: subcommand
> failed.bus_fslmc.a.p/bus_fslmc_fslmc_bus.c.o
>
> Thanks for reporting.
>
> The issue on fslmc is not related to Jim patch.
> If there is no bugzilla opened for this issue, please open a new one.
>
> Thanks.
>
>
> --
> David Marchand
Got it, thanks David. We will report new bug on bugzilla.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] power: fix unused-but-set variable error
2021-11-08 20:40 ` David Marchand
@ 2021-11-12 14:46 ` David Marchand
0 siblings, 0 replies; 6+ messages in thread
From: David Marchand @ 2021-11-12 14:46 UTC (permalink / raw)
To: Jim Harris; +Cc: dev, David Hunt
On Mon, Nov 8, 2021 at 9:40 PM David Marchand <david.marchand@redhat.com> wrote:
> > clang-13 rightfully complains that the tot_ppi
> > variable in update_stats is set but not used, since
> > the final accumulated tot_ppi results isn't used
> > anywhere.
> >
> > Signed-off-by: Jim Harris <james.r.harris@intel.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
I split the fix and marked it for backport.
The cleanup on unused metadata is merged as a second patch.
Applied, thanks Jim.
--
David Marchand
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-12 14:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-05 15:53 [dpdk-dev] [PATCH v2] power: fix unused-but-set variable error Jim Harris
2021-11-08 20:40 ` David Marchand
2021-11-12 14:46 ` David Marchand
2021-11-10 10:55 ` [dpdk-dev] " Jiang, YuX
2021-11-10 11:07 ` David Marchand
2021-11-10 11:34 ` Jiang, YuX
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).