* [dpdk-dev] [PATCH v2] timer: remove check_tsc_flags()
@ 2019-08-21 10:18 Jim Harris
2019-10-07 15:40 ` [dpdk-dev] [PATCH v2 RESEND] " Jim Harris
0 siblings, 1 reply; 6+ messages in thread
From: Jim Harris @ 2019-08-21 10:18 UTC (permalink / raw)
To: dev, anatoly.burakov
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(-)
diff --git a/lib/librte_eal/linux/eal/eal_timer.c b/lib/librte_eal/linux/eal/eal_timer.c
index 76ec17034..a904a8297 100644
--- a/lib/librte_eal/linux/eal/eal_timer.c
+++ b/lib/librte_eal/linux/eal/eal_timer.c
@@ -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;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* [dpdk-dev] [PATCH v2 RESEND] timer: remove check_tsc_flags()
2019-08-21 10:18 [dpdk-dev] [PATCH v2] timer: remove check_tsc_flags() Jim Harris
@ 2019-10-07 15:40 ` Jim Harris
2019-10-07 23:18 ` Stephen Hemminger
0 siblings, 1 reply; 6+ messages in thread
From: Jim Harris @ 2019-10-07 15:40 UTC (permalink / raw)
To: dev, bruce.richardson, anatoly.burakov, stephen
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(-)
diff --git a/lib/librte_eal/linux/eal/eal_timer.c b/lib/librte_eal/linux/eal/eal_timer.c
index 76ec17034..a904a8297 100644
--- a/lib/librte_eal/linux/eal/eal_timer.c
+++ b/lib/librte_eal/linux/eal/eal_timer.c
@@ -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;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH v2 RESEND] timer: remove check_tsc_flags()
2019-10-07 15:40 ` [dpdk-dev] [PATCH v2 RESEND] " Jim Harris
@ 2019-10-07 23:18 ` Stephen Hemminger
2019-10-08 8:36 ` Bruce Richardson
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2019-10-07 23:18 UTC (permalink / raw)
To: Jim Harris; +Cc: dev, bruce.richardson, anatoly.burakov
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>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH v2 RESEND] timer: remove check_tsc_flags()
2019-10-07 23:18 ` Stephen Hemminger
@ 2019-10-08 8:36 ` Bruce Richardson
2019-10-08 15:15 ` Stephen Hemminger
0 siblings, 1 reply; 6+ messages in thread
From: Bruce Richardson @ 2019-10-08 8:36 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Jim Harris, dev, anatoly.burakov
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>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH v2 RESEND] timer: remove check_tsc_flags()
2019-10-08 8:36 ` Bruce Richardson
@ 2019-10-08 15:15 ` Stephen Hemminger
2019-10-18 4:10 ` David Marchand
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2019-10-08 15:15 UTC (permalink / raw)
To: Bruce Richardson; +Cc: Jim Harris, dev, anatoly.burakov
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH v2 RESEND] timer: remove check_tsc_flags()
2019-10-08 15:15 ` Stephen Hemminger
@ 2019-10-18 4:10 ` David Marchand
0 siblings, 0 replies; 6+ messages in thread
From: David Marchand @ 2019-10-18 4:10 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Bruce Richardson, Jim Harris, dev, Burakov, Anatoly
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-18 4:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-21 10:18 [dpdk-dev] [PATCH v2] timer: remove check_tsc_flags() Jim Harris
2019-10-07 15:40 ` [dpdk-dev] [PATCH v2 RESEND] " Jim Harris
2019-10-07 23:18 ` Stephen Hemminger
2019-10-08 8:36 ` Bruce Richardson
2019-10-08 15:15 ` Stephen Hemminger
2019-10-18 4:10 ` David Marchand
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).