Hi Ilya,
Thanks for your comments. Please take time to see the inline comments.
Best regards,
Lance
> -----Original Message-----
> From: Ilya Maximets <i.maxim...@ovn.org>
> Sent: Friday, November 22, 2019 3:23 AM
> To: Lance Yang (Arm Technology China) <lance.y...@arm.com>; ovs-
> d...@openvswitch.org
> Cc: Jieqiang Wang (Arm Technology China) <jieqiang.w...@arm.com>; Gavin Hu
> (Arm
> Technology China) <gavin...@arm.com>; Jingzhao Ni (Arm Technology China)
> <jingzhao...@arm.com>; nd <n...@arm.com>; Ruifeng Wang (Arm Technology China)
> <ruifeng.w...@arm.com>
> Subject: Re: [ovs-dev] [PATCH v1 1/4] dpif-netdev: Fix the pmd_perf_stats
>
> On 20.11.2019 9:13, Lance Yang wrote:
> > When compiling Open vSwitch for aarch64, the compiler will warn about
> > an uninitialized variable in lib/dpif-netdev-perf.c. It is necessary
> > to initialize it to avoid build failure.
>
> Hi. Thanks for making OVS build on aarch64.
>
> Could you provide the error message?
[Lance]
For the error message you would like to see, I copied it below:
--- Begin of the error message ---
In file included from lib/dpif-netdev-perf.c:21:
lib/dpif-netdev-perf.c: In function 'pmd_perf_estimate_tsc_frequency':
lib/dpif-netdev-perf.h:198:16: error: 's.last_tsc' may be used uninitialized in
this function [-Werror=maybe-uninitialized]
return s->last_tsc;
~^~~~~~~~~~
libtool: compile: gcc -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib
-I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
-Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast
-Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes
-Wmissing-field-initializers -fno-strict-aliasing -Wswitch-bool
-Wlogical-not-parentheses -Wsizeof-array-argument -Wbool-compare
-Wshift-negative-value -Wduplicated-cond -Wshadow -Wmultistatement-macros
-Wcast-align=strict -Werror -Werror -g -O2 -MT lib/heap.lo -MD -MP -MF
lib/.deps/heap.Tpo -c lib/heap.c -o lib/heap.o
cc1: all warnings being treated as errors
--- end of the message---
191 static inline uint64_t
192 rdtsc_syscall(struct pmd_perf_stats *s)
193 {
194 struct timespec val;
195 uint64_t v;
196
197 if (clock_gettime(CLOCK_MONOTONIC_RAW, &val) != 0) {
198 return s->last_tsc;
199 }
200
201 v = val.tv_sec * UINT64_C(1000000000) + val.tv_nsec;
202 return s->last_tsc = v;
203 }
204 #endif
When the clock_gettime function failed, the code "return s->last_tsc" will be
reached.
> In fact this variable is never used (only assigned), so this should be a
> false-positive. So, I'd
> like to look at the error message.
>
> Also, it might be better to rename the patch to something more specific. e.g.
> 'dpif-netdev-perf: Avoid false-positive on uninitialized perf stats.'
I typed an "Enter key" in the patch. The original title should be:
Subject: [ovs-dev] [PATCH v1 1/4] dpif-netdev: Fix the pmd_perf_stats
uninitalized issue
If this title looks fine, I will modify the title as above in the patch set v2.
> >
> > Reviewed-by: Yanqin Wei <yanqin....@arm.com>
> > Signed-off-by: Lance Yang <lance.y...@arm.com>
> > ---
> > lib/dpif-netdev-perf.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/dpif-netdev-perf.c b/lib/dpif-netdev-perf.c index
> > baf90b0..f85bb0c 100644
> > --- a/lib/dpif-netdev-perf.c
> > +++ b/lib/dpif-netdev-perf.c
> > @@ -63,6 +63,7 @@ pmd_perf_estimate_tsc_frequency(void)
> > struct pmd_perf_stats s;
> > uint64_t start, stop;
> >
> > + memset(&s, 0, sizeof(s));
>
> Please, don't parenthesize the argument of a 'sizeof'.
>
> Also, it might be better to initialize the variable close to the first usage.
> It doesn't look
> good here.
>
[Lance]
I will remove the parenthesize for the sizeof operator and move the struct
initialization closer to where it is used.
> > /* DPDK is not available or returned unreliable value.
> > * Trying to estimate. */
> > affinity = ovs_numa_thread_getaffinity_dump();
> >
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
