[v2,RESEND] timer: remove check_tsc_flags()
Checks
Commit Message
This code was added 7+ years ago:
commit fb022b85bae4 ("timer: check TSC reliability")
presumably when variant TSCs were still somewhat
common? But this code doesn't do anything except print
a warning, and the warning doesn't give any kind of
advice to the user, so let's just remove it.
While the warning has no functional meaning, the
/proc/cpuinfo parsing consumes a non-trivial amount
of time which is especially noticeable in secondary
processes. On my test system, it consumes
21ms out of the 66ms total execution time for
rte_eal_init() in a secondary process.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
---
lib/librte_eal/linux/eal/eal_timer.c | 36 ----------------------------------
1 file changed, 36 deletions(-)
Comments
On Mon, 07 Oct 2019 08:40:05 -0700
Jim Harris <james.r.harris@intel.com> wrote:
> This code was added 7+ years ago:
>
> commit fb022b85bae4 ("timer: check TSC reliability")
>
> presumably when variant TSCs were still somewhat
> common? But this code doesn't do anything except print
> a warning, and the warning doesn't give any kind of
> advice to the user, so let's just remove it.
>
> While the warning has no functional meaning, the
> /proc/cpuinfo parsing consumes a non-trivial amount
> of time which is especially noticeable in secondary
> processes. On my test system, it consumes
> 21ms out of the 66ms total execution time for
> rte_eal_init() in a secondary process.
>
> Signed-off-by: Jim Harris <james.r.harris@intel.com>
Yes this code is dead.
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
On Mon, Oct 07, 2019 at 04:18:54PM -0700, Stephen Hemminger wrote:
> On Mon, 07 Oct 2019 08:40:05 -0700 Jim Harris <james.r.harris@intel.com>
> wrote:
>
> > This code was added 7+ years ago:
> >
> > commit fb022b85bae4 ("timer: check TSC reliability")
> >
> > presumably when variant TSCs were still somewhat common? But this code
> > doesn't do anything except print a warning, and the warning doesn't
> > give any kind of advice to the user, so let's just remove it.
> >
> > While the warning has no functional meaning, the /proc/cpuinfo parsing
> > consumes a non-trivial amount of time which is especially noticeable in
> > secondary processes. On my test system, it consumes 21ms out of the
> > 66ms total execution time for rte_eal_init() in a secondary process.
> >
> > Signed-off-by: Jim Harris <james.r.harris@intel.com>
>
> Yes this code is dead.
>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>
+1 for this. Even if it was needed, we should never parse /proc/cpuinfo,
since we have a DPDK function to query cpuid directly anyway.
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
On Tue, 8 Oct 2019 09:36:49 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:
> On Mon, Oct 07, 2019 at 04:18:54PM -0700, Stephen Hemminger wrote:
> > On Mon, 07 Oct 2019 08:40:05 -0700 Jim Harris <james.r.harris@intel.com>
> > wrote:
> >
> > > This code was added 7+ years ago:
> > >
> > > commit fb022b85bae4 ("timer: check TSC reliability")
> > >
> > > presumably when variant TSCs were still somewhat common? But this code
> > > doesn't do anything except print a warning, and the warning doesn't
> > > give any kind of advice to the user, so let's just remove it.
> > >
> > > While the warning has no functional meaning, the /proc/cpuinfo parsing
> > > consumes a non-trivial amount of time which is especially noticeable in
> > > secondary processes. On my test system, it consumes 21ms out of the
> > > 66ms total execution time for rte_eal_init() in a secondary process.
> > >
> > > Signed-off-by: Jim Harris <james.r.harris@intel.com>
> >
> > Yes this code is dead.
> >
> > Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> >
>
> +1 for this. Even if it was needed, we should never parse /proc/cpuinfo,
> since we have a DPDK function to query cpuid directly anyway.
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
It also turns out that Hyper-V/Azure report unstable TSC because
when VM is migrated there are blips in TSC. There upcoming changes to
handle that in Hypervisor and Linux drivers.
On Tue, Oct 8, 2019 at 5:16 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 8 Oct 2019 09:36:49 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
>
> > On Mon, Oct 07, 2019 at 04:18:54PM -0700, Stephen Hemminger wrote:
> > > On Mon, 07 Oct 2019 08:40:05 -0700 Jim Harris <james.r.harris@intel.com>
> > > wrote:
> > >
> > > > This code was added 7+ years ago:
> > > >
> > > > commit fb022b85bae4 ("timer: check TSC reliability")
> > > >
> > > > presumably when variant TSCs were still somewhat common? But this code
> > > > doesn't do anything except print a warning, and the warning doesn't
> > > > give any kind of advice to the user, so let's just remove it.
> > > >
> > > > While the warning has no functional meaning, the /proc/cpuinfo parsing
> > > > consumes a non-trivial amount of time which is especially noticeable in
> > > > secondary processes. On my test system, it consumes 21ms out of the
> > > > 66ms total execution time for rte_eal_init() in a secondary process.
> > > >
> > > > Signed-off-by: Jim Harris <james.r.harris@intel.com>
> > > Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Applied, thanks.
--
David Marchand
@@ -192,41 +192,6 @@ rte_eal_hpet_init(int make_default)
}
#endif
-static void
-check_tsc_flags(void)
-{
- char line[512];
- FILE *stream;
-
- stream = fopen("/proc/cpuinfo", "r");
- if (!stream) {
- RTE_LOG(WARNING, EAL, "WARNING: Unable to open /proc/cpuinfo\n");
- return;
- }
-
- while (fgets(line, sizeof line, stream)) {
- char *constant_tsc;
- char *nonstop_tsc;
-
- if (strncmp(line, "flags", 5) != 0)
- continue;
-
- constant_tsc = strstr(line, "constant_tsc");
- nonstop_tsc = strstr(line, "nonstop_tsc");
- if (!constant_tsc || !nonstop_tsc)
- RTE_LOG(WARNING, EAL,
- "WARNING: cpu flags "
- "constant_tsc=%s "
- "nonstop_tsc=%s "
- "-> using unreliable clock cycles !\n",
- constant_tsc ? "yes":"no",
- nonstop_tsc ? "yes":"no");
- break;
- }
-
- fclose(stream);
-}
-
uint64_t
get_tsc_freq(void)
{
@@ -263,6 +228,5 @@ rte_eal_timer_init(void)
eal_timer_source = EAL_TIMER_TSC;
set_tsc_freq();
- check_tsc_flags();
return 0;
}