timer/linux: lower rounding of tsc estimation to 10KHz
PATCH v2 1/2 timer/linux: lower rounding of tsc estimation to 10KHz
In practice, the estimation result is just a couple of KHz away from the kernel's tsc_khz value, so it should suffice.
Roundin to 10MHz can cause a significant drift from real time, up to a second per 10 minutes.
See also bugzilla: 959
--- lib/eal/linux/eal_timer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
lib/eal/linux/eal_timer.c +3 -3
@@ -192,7 +192,7 @@ get_tsc_freq(void) {#ifdef CLOCK_MONOTONIC_RAW #define NS_PER_SEC 1E9 -#define CYC_PER_10MHZ 1E7 +#define CYC_PER_10KHZ 1E4struct timespec sleeptime = {.tv_nsec =NS_PER_SEC /10 };/* 1/10 second */
@@ -209,8 +209,8 @@ get_tsc_freq(void)double secs = (double )ns/NS_PER_SEC ; tsc_hz = (uint64_t )((end - start)/secs); -/* Round up to 10Mhz. 1E7 ~ 10Mhz */ - return RTE_ALIGN_MUL_NEAR(tsc_hz, CYC_PER_10MHZ); +/* Round up to 10Khz. 1E4 ~ 10Khz */ + return RTE_ALIGN_MUL_NEAR(tsc_hz, CYC_PER_10KHZ); }#endif return 0 ;
Signature · 1 line
2.45.0
PATCH v2 2/2 timer/linux: override TSC freq if no tsc_known_freq
If the tsc_known_freq cpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.
--- lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 5 +++- lib/eal/linux/eal_timer.c | 48 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 +++- 5 files changed, 56 insertions(+), 7 deletions(-)
lib/eal/common/eal_common_timer.c +1 -2
@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();
lib/eal/common/eal_private.h +1 -1
@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports.
lib/eal/freebsd/eal_timer.c +4 -1
@@ -26,12 +26,15 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {size_t sz;int tmp;uint64_t tsc_hz; +if (arch_hz) +return arch_hz; + sz =sizeof (tmp); tmp =0 ;
lib/eal/linux/eal_timer.c +46 -2
75 lines not on this page · raw message
lib/eal/windows/eal_timer.c +4 -1
@@ -49,13 +49,16 @@ rte_delay_us_sleep(unsigned int us) }uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {LARGE_INTEGER t_start, t_end, elapsed_us;LARGE_INTEGER frequency;uint64_t tsc_hz;uint64_t end, start; +if (arch_hz) +return arch_hz; +QueryPerformanceFrequency (&frequency);QueryPerformanceCounter (&t_start);
Signature · 1 line
2.45.0
Re: [PATCH 2/2] timer/linux: override TSC freq if no tsc_known_freq
Perhaps this was a bit hastily, I got an automated failure report on "arm64-native-linuxapp-gcc":
https://lab.dpdk.org/results/dashboard/patchsets/31106/
Not sure which patch caused the problem, looking at the kernel code it looks like 'tsc_known_freq' is only set for x86 arch, but it could be the other patch and maybe lowering the rounding to 10KHz is too aggressive, maybe better 100KHz or 1MHz.
As an alternative to 'tsc_known_freq' detection, maybe just provide an init parameter to set the frequency manually, along with some known-issue documentation.
Maybe we should just allow to specify the frequency as a parameter at init
Re: [PATCH 2/2] timer/linux: override TSC freq if no tsc_known_freq
Quoted 5 lines · Isaac Boukris
Perhaps this was a bit hastily, I got an automated failure report on "arm64-native-linuxapp-gcc":
That might be a false positive failure. I don't see how this patch could cause issues for a single ip reassembly test case.
Quoted 4 lines
Not sure which patch caused the problem, looking at the kernel code it looks like '
tsc_known_freq' is only set for x86 arch, but it could be the other patch and maybe lowering the rounding to 10KHz is too aggressive, maybe better 100KHz or 1MHz.
I don't see why we can't put an #ifdef in the code to always return true on non-x86 platforms. [Or if we don't want an #ifdef, we can always use 'if (strcmp(RTE_ARCH,"x86_64") && strcmp(RTE_ARCH,"i686"))' :-)]
Don't really have a strong opinion on the rounding, maybe others do. [Since you give 3 options, I'm going to suggest going for the middle one - 100KHz!]
Quoted 6 lines
As an alternative to '
tsc_known_freq' detection, maybe just provide an init parameter to set the frequency manually, along with some known-issue documentation.Maybe we should just allow to specify the frequency as a parameter at init
I don't think that is a great solution, better to detect the frequency if we can.
An interesting future enhancement would be to update DPDK to work with [1], where we read the tsc value directly from the kernel. According to that project, this is already available on Google production kernels, though not generally elsewhere. For those who care about having accurate TSC values, or faster startup times, having the extra kernel module to read the TSC value directly might be worthwhile.
Regards,
/Bruce
v2 cover Improve TSC frequency accuracy on Linux
In reply to Isaac BoukrisChanges in V2:
- changed rounding to 100KHz which should be pretty safe.
- limited the tsc_known_freq flag check to x86 arch.
Isaac Boukris (2):
timer/linux: lower rounding of tsc estimation to 100KHz timer/linux/x86: override TSC freq if no tsc_known_freq
lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 5 ++- lib/eal/linux/eal_timer.c | 56 ++++++++++++++++++++++++++++--- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 61 insertions(+), 10 deletions(-)
Signature · 1 line
2.45.0
PATCH v2 1/2 timer/linux: lower rounding of tsc estimation to 100KHz
In practice, the estimation result is just a couple of KHz away from the kernel's tsc_khz value, so it should suffice.
Rounding to 10MHz can cause a significant drift from real time, up to a second per 10 minutes.
See also bugzilla: 959
--- lib/eal/linux/eal_timer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
lib/eal/linux/eal_timer.c +3 -3
@@ -192,7 +192,7 @@ get_tsc_freq(void) {#ifdef CLOCK_MONOTONIC_RAW #define NS_PER_SEC 1E9 -#define CYC_PER_10MHZ 1E7 +#define CYC_PER_100KHZ 1E5struct timespec sleeptime = {.tv_nsec =NS_PER_SEC /10 };/* 1/10 second */
@@ -209,8 +209,8 @@ get_tsc_freq(void)double secs = (double )ns/NS_PER_SEC ; tsc_hz = (uint64_t )((end - start)/secs); -/* Round up to 10Mhz. 1E7 ~ 10Mhz */ - return RTE_ALIGN_MUL_NEAR(tsc_hz, CYC_PER_10MHZ); +/* Round up to 100Khz. 1E5 ~ 100Khz */ + return RTE_ALIGN_MUL_NEAR(tsc_hz, CYC_PER_100KHZ); }#endif return 0 ;
Signature · 1 line
2.45.0
PATCH v2 2/2 timer/linux/x86: override TSC freq if no tsc_known_freq
In reply to Isaac BoukrisIf the tsc_known_freq cpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.
--- lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 5 +++- lib/eal/linux/eal_timer.c | 50 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 +++- 5 files changed, 58 insertions(+), 7 deletions(-)
lib/eal/common/eal_common_timer.c +1 -2
@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();
lib/eal/common/eal_private.h +1 -1
@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports.
lib/eal/freebsd/eal_timer.c +4 -1
@@ -26,12 +26,15 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {size_t sz;int tmp;uint64_t tsc_hz; +if (arch_hz) +return arch_hz; + sz =sizeof (tmp); tmp =0 ;
lib/eal/linux/eal_timer.c +48 -2
77 lines not on this page · raw message
lib/eal/windows/eal_timer.c +4 -1
@@ -49,13 +49,16 @@ rte_delay_us_sleep(unsigned int us) }uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {LARGE_INTEGER t_start, t_end, elapsed_us;LARGE_INTEGER frequency;uint64_t tsc_hz;uint64_t end, start; +if (arch_hz) +return arch_hz; +QueryPerformanceFrequency (&frequency);QueryPerformanceCounter (&t_start);
Signature · 1 line
2.45.0
Re: [PATCH v2 2/2] timer/linux/x86: override TSC freq if no tsc_known_freq
lib/eal/linux/eal_timer.c get_tsc_freq(void) · line 256
Quoted 5 lines · On Tue, 1 Oct 2024 01:08:32 +0300 Isaac Boukris <[email protected]> wrote:
lib/eal/linux/eal_timer.c +5
-0+ +EAL_LOG (DEBUG ,"Refined architecture frequency %" PRIu64 +" to measured frequency %" PRIu64"." , + arch_hz, tsc_hz); + }
Best not to break messages, it make's it harder for users to search. Don't need period either.
EAL_LOG(DEBUG,
"Refined architecture frequency %"PRIu64" to measured frequency %"PRIu64,
v3 cover Improve TSC frequency accuracy on Linux
In reply to Isaac BoukrisChanges in V3:
- fixed ifdef logic.
- avoid breaking a debug line.
Isaac Boukris (2):
timer/linux: lower rounding of tsc estimation to 100KHz timer/linux/x86: override TSC freq if no tsc_known_freq
lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 5 ++- lib/eal/linux/eal_timer.c | 59 +++++++++++++++++++++++++++---- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 63 insertions(+), 11 deletions(-)
Signature · 1 line
2.45.0
PATCH v3 1/2 timer/linux: lower rounding of tsc estimation to 100KHz
In practice, the estimation result is just a couple of KHz away from the kernel's tsc_khz value, so it should suffice.
Rounding to 10MHz can cause a significant drift from real time, up to a second per 10 minutes.
See also bugzilla: 959
--- lib/eal/linux/eal_timer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
lib/eal/linux/eal_timer.c +3 -3
@@ -192,7 +192,7 @@ get_tsc_freq(void) {#ifdef CLOCK_MONOTONIC_RAW #define NS_PER_SEC 1E9 -#define CYC_PER_10MHZ 1E7 +#define CYC_PER_100KHZ 1E5struct timespec sleeptime = {.tv_nsec =NS_PER_SEC /10 };/* 1/10 second */
@@ -209,8 +209,8 @@ get_tsc_freq(void)double secs = (double )ns/NS_PER_SEC ; tsc_hz = (uint64_t )((end - start)/secs); -/* Round up to 10Mhz. 1E7 ~ 10Mhz */ - return RTE_ALIGN_MUL_NEAR(tsc_hz, CYC_PER_10MHZ); +/* Round up to 100Khz. 1E5 ~ 100Khz */ + return RTE_ALIGN_MUL_NEAR(tsc_hz, CYC_PER_100KHZ); }#endif return 0 ;
Signature · 1 line
2.45.0
Re: [PATCH v3 1/2] timer/linux: lower rounding of tsc estimation to 100KHz
Quoted 10 lines · On Tue, 1 Oct 2024 03:22:50 +0300 Isaac Boukris <[email protected]> wrote:
In practice, the estimation result is just a couple of KHz away from the kernel's
tsc_khzvalue, so it should suffice.Rounding to 10MHz can cause a significant drift from real time, up to a second per 10 minutes.
See also bugzilla: 959
Signed-off-by Isaac Boukris <[email protected]>---
PATCH v3 2/2 timer/linux/x86: override TSC freq if no tsc_known_freq
In reply to Isaac BoukrisIf the tsc_known_freq cpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.
--- lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 5 ++- lib/eal/linux/eal_timer.c | 53 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 60 insertions(+), 8 deletions(-)
lib/eal/common/eal_common_timer.c +1 -2
@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();
lib/eal/common/eal_private.h +1 -1
@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports.
lib/eal/freebsd/eal_timer.c +4 -1
@@ -26,12 +26,15 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {size_t sz;int tmp;uint64_t tsc_hz; +if (arch_hz) +return arch_hz; + sz =sizeof (tmp); tmp =0 ;
lib/eal/linux/eal_timer.c +50 -3
84 lines not on this page · raw message
lib/eal/windows/eal_timer.c +4 -1
@@ -49,13 +49,16 @@ rte_delay_us_sleep(unsigned int us) }uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {LARGE_INTEGER t_start, t_end, elapsed_us;LARGE_INTEGER frequency;uint64_t tsc_hz;uint64_t end, start; +if (arch_hz) +return arch_hz; +QueryPerformanceFrequency (&frequency);QueryPerformanceCounter (&t_start);
Signature · 1 line
2.45.0
Re: [PATCH v3 2/2] timer/linux/x86: override TSC freq if no tsc_known_freq
Quoted 13 lines · On Tue, 1 Oct 2024 03:22:51 +0300 Isaac Boukris <[email protected]> wrote:
lib/eal/windows/eal_timer.c +1
-1@@ -49,13 +49,16 @@ rte_delay_us_sleep(unsigned int us) }uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {LARGE_INTEGER t_start, t_end, elapsed_us;
lib/eal/windows/eal_timer.c rte_delay_us_sleep(unsigned int us) · line 64
Quoted 10 lines
lib/eal/windows/eal_timer.c +3
-0LARGE_INTEGER frequency;uint64_t tsc_hz;uint64_t end, start; +if (arch_hz) +return arch_hz; +QueryPerformanceFrequency (&frequency);QueryPerformanceCounter (&t_start);
On Windows, I would not use arch_hz at all, since it is opaque how the Windows kernel determines the frequency, and best not to get skew.
Re: [PATCH v3 2/2] timer/linux/x86: override TSC freq if no tsc_known_freq
Quoted 25 lines · Stephen Hemminger
Quoted 21 lines · On Tue, 1 Oct 2024 03:22:51 +0300 Isaac Boukris <[email protected]> wrote:
lib/eal/windows/eal_timer.c +4
-1@@ -49,13 +49,16 @@ rte_delay_us_sleep(unsigned int us) }uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {LARGE_INTEGER t_start, t_end, elapsed_us;LARGE_INTEGER frequency;uint64_t tsc_hz;uint64_t end, start; +if (arch_hz) +return arch_hz; +QueryPerformanceFrequency (&frequency);
Quoted 6 lines
lib/eal/windows/eal_timer.c rte_delay_us_sleep(unsigned int us)
Quoted 2 lines
lib/eal/windows/eal_timer.c +0
-1QueryPerformanceCounter (&t_start); --On Windows, I would not use
arch_hzat all, since it is opaque how the Windows kernel determines the frequency, and best not to get skew.
Not sure I follow, currently the patch doesn't change the behavior for Windows, and the Windows code is quite similar to the Linux one, should we always prefer the measured value in Windows?
On the other hand, I think I should update the rounding commit to also change the Windows code, there is no sense in 10MHz rounding there either.
Re: [PATCH v3 2/2] timer/linux/x86: override TSC freq if no tsc_known_freq
In reply to Isaac BoukrisQuoted 50 lines · Isaac Boukris
If the
tsc_known_freqcpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.Signed-off-by Isaac Boukris <[email protected]>--- lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 5 ++- lib/eal/linux/eal_timer.c | 53 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 60 insertions(+), 8 deletions(-)lib/eal/common/eal_common_timer.c +1
-2@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();lib/eal/common/eal_private.h +1
-1@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports. lib/eal/freebsd/eal_timer.c +1
-1@@ -26,12 +26,15 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {
lib/eal/freebsd/eal_timer.c line 40
Quoted 10 lines
lib/eal/freebsd/eal_timer.c +3
-0size_t sz;int tmp;uint64_t tsc_hz; +if (arch_hz) +return arch_hz; + sz =sizeof (tmp); tmp =0 ;
On FreeBSD I'm not sure this is the best behaviour. On BSD we read the TSC value from the kernel, which, one assumes, has measured it accurately. Therefore I'd tend toward just using the kernel value in all cases, maybe check the arch value (if non-zero) against that and warning if they have significant divergence. WDYT?
/Bruce
Re: [PATCH v3 2/2] timer/linux/x86: override TSC freq if no tsc_known_freq
Quoted 56 lines · Bruce Richardson
Quoted 54 lines · Isaac Boukris
If the
tsc_known_freqcpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.Signed-off-by Isaac Boukris <[email protected]>--- lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 5 ++- lib/eal/linux/eal_timer.c | 53 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 60 insertions(+), 8 deletions(-)lib/eal/common/eal_common_timer.c +1
-2@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();lib/eal/common/eal_private.h +1
-1@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports. lib/eal/freebsd/eal_timer.c +1
-1@@ -26,12 +26,15 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {size_t sz;int tmp;uint64_t tsc_hz;
Quoted 12 lines
lib/eal/freebsd/eal_timer.c line 40
Quoted 6 lines
lib/eal/freebsd/eal_timer.c +3
-0+if (arch_hz) +return arch_hz; + sz =sizeof (tmp); tmp =0 ;On FreeBSD I'm not sure this is the best behaviour. On BSD we read the TSC value from the kernel, which, one assumes, has measured it accurately. Therefore I'd tend toward just using the kernel value in all cases, maybe check the arch value (if non-zero) against that and warning if they have significant divergence. WDYT?
Makes sense, I'll add a patch for that. We could also use the arch value if for some reason the sysctlbyname() failed.
Re: [PATCH v3 2/2] timer/linux/x86: override TSC freq if no tsc_known_freq
Quoted 71 lines · Isaac Boukris
Quoted 56 lines · Bruce Richardson
Quoted 54 lines · Isaac Boukris
If the
tsc_known_freqcpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.Signed-off-by Isaac Boukris <[email protected]>--- lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 5 ++- lib/eal/linux/eal_timer.c | 53 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 60 insertions(+), 8 deletions(-)lib/eal/common/eal_common_timer.c +1
-2@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();lib/eal/common/eal_private.h +1
-1@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports. lib/eal/freebsd/eal_timer.c +1
-1@@ -26,12 +26,15 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {size_t sz;int tmp;uint64_t tsc_hz;Quoted 12 lines
lib/eal/freebsd/eal_timer.c line 40
Quoted 6 lines
lib/eal/freebsd/eal_timer.c +3
-0+if (arch_hz) +return arch_hz; + sz =sizeof (tmp); tmp =0 ;On FreeBSD I'm not sure this is the best behaviour. On BSD we read the TSC value from the kernel, which, one assumes, has measured it accurately. Therefore I'd tend toward just using the kernel value in all cases, maybe check the arch value (if non-zero) against that and warning if they have significant divergence. WDYT?
Quoted 2 lines
Makes sense, I'll add a patch for that. We could also use the arch value if for some reason the
sysctlbyname()failed.
Yep, +1 to that.
/Bruce
v4 cover Improve TSC frequency accuracy
In reply to Isaac BoukrisChanges in V4:
- lower ronding on Windows too
- on freebsd prefer system tsc_hz
Isaac Boukris (2):
timer: lower rounding of TSC estimation to 100KHz
timer: allow platform to override cpu TSC frequency
lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 8 +++-- lib/eal/linux/eal_timer.c | 59 +++++++++++++++++++++++++++---- lib/eal/windows/eal_timer.c | 11 +++--- 5 files changed, 68 insertions(+), 15 deletions(-)
Signature · 1 line
2.45.0
PATCH v4 1/2 timer: lower rounding of TSC estimation to 100KHz
In practice, the frequency is often not a nice round number, while the estimation results are rather accurate, just a couple of KHz away from the kernel's tsc_khz value, so it should suffice.
Rounding to 10MHz can cause a significant drift from real time, up to a second per 10 minutes.
See also bugzilla: 959
--- lib/eal/linux/eal_timer.c | 6 +++--- lib/eal/windows/eal_timer.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)
lib/eal/linux/eal_timer.c +3 -3
@@ -192,7 +192,7 @@ get_tsc_freq(void) {#ifdef CLOCK_MONOTONIC_RAW #define NS_PER_SEC 1E9 -#define CYC_PER_10MHZ 1E7 +#define CYC_PER_100KHZ 1E5struct timespec sleeptime = {.tv_nsec =NS_PER_SEC /10 };/* 1/10 second */
@@ -209,8 +209,8 @@ get_tsc_freq(void)double secs = (double )ns/NS_PER_SEC ; tsc_hz = (uint64_t )((end - start)/secs); -/* Round up to 10Mhz. 1E7 ~ 10Mhz */ - return RTE_ALIGN_MUL_NEAR(tsc_hz, CYC_PER_10MHZ); +/* Round up to 100Khz. 1E5 ~ 100Khz */ + return RTE_ALIGN_MUL_NEAR(tsc_hz, CYC_PER_100KHZ); }#endif return 0 ;
lib/eal/windows/eal_timer.c +3 -3
@@ -12,7 +12,7 @@#include "eal_private.h" #define US_PER_SEC 1E6 -#define CYC_PER_10MHZ 1E7 +#define CYC_PER_100KHZ 1E5void rte_delay_us_sleep (unsigned int us)
@@ -81,8 +81,8 @@ get_tsc_freq(void)double secs = ((double )elapsed_us.QuadPart)/US_PER_SEC ; tsc_hz = (uint64_t )((end - start)/secs); -/* Round up to 10Mhz. 1E7 ~ 10Mhz */ -return RTE_ALIGN_MUL_NEAR (tsc_hz,CYC_PER_10MHZ ); +/* Round up to 100Khz. 1E5 ~ 100Khz */ +return RTE_ALIGN_MUL_NEAR (tsc_hz,CYC_PER_100KHZ ); }
Signature · 1 line
2.45.0
PATCH v4 2/2 timer: allow platform to override cpu TSC frequency
In reply to Isaac BoukrisThe CPU value is often not accurate, allow overriding it based on info from the host OS.
On Linux X86, if the tsc_known_freq cpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.
On freebsd we have access to the kernel tsc_hz value, just use it.
--- lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 8 +++-- lib/eal/linux/eal_timer.c | 53 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 62 insertions(+), 9 deletions(-)
lib/eal/common/eal_common_timer.c +1 -2
@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();
lib/eal/common/eal_private.h +1 -1
@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports.
lib/eal/freebsd/eal_timer.c +6 -2
@@ -26,7 +26,7 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {size_t sz;int tmp;
@@ -50,9 +50,13 @@ get_tsc_freq(void) sz =sizeof (tsc_hz);if (sysctlbyname ("machdep.tsc_freq" , &tsc_hz, &sz,NULL ,0 )) {EAL_LOG (WARNING ,"%s" ,strerror (errno)); -return 0 ; +return arch_hz; } +if (arch_hz && arch_hz - tsc_hz > arch_hz /100 ) +EAL_LOG (WARNING ,"Host tsc_freq %" PRIu64" at odds with cpu value %" PRIu64, + tsc_hz, arch_hz); +return tsc_hz; }
lib/eal/linux/eal_timer.c +50 -3
84 lines not on this page · raw message
lib/eal/windows/eal_timer.c +4 -1
@@ -49,13 +49,16 @@ rte_delay_us_sleep(unsigned int us) }uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {LARGE_INTEGER t_start, t_end, elapsed_us;LARGE_INTEGER frequency;uint64_t tsc_hz;uint64_t end, start; +if (arch_hz) +return arch_hz; +QueryPerformanceFrequency (&frequency);QueryPerformanceCounter (&t_start);
Signature · 1 line
2.45.0
Re: [PATCH v4 2/2] timer: allow platform to override cpu TSC frequency
Quoted 11 lines · Isaac Boukris
The CPU value is often not accurate, allow overriding it based on info from the host OS.
On Linux X86, if the
tsc_known_freqcpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.On freebsd we have access to the kernel
tsc_hzvalue, just use it.Signed-off-by Isaac Boukris <[email protected]>---
Looks good to me. Hope to test it out soon on my systems to see how it goes.
One comment inline below.
/Bruce
Quoted 48 lines
lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 8 +++-- lib/eal/linux/eal_timer.c | 53 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 62 insertions(+), 9 deletions(-)lib/eal/common/eal_common_timer.c +1
-2@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();lib/eal/common/eal_private.h +1
-1@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports. lib/eal/freebsd/eal_timer.c +1
-1@@ -26,7 +26,7 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {size_t sz;int tmp;
lib/eal/freebsd/eal_timer.c get_tsc_freq(void) · line 56
Quoted 8 lines
lib/eal/freebsd/eal_timer.c +2
-1@@ -50,9 +50,13 @@ get_tsc_freq(void) sz =sizeof (tsc_hz);if (sysctlbyname ("machdep.tsc_freq" , &tsc_hz, &sz,NULL ,0 )) {EAL_LOG (WARNING ,"%s" ,strerror (errno)); -return 0 ; +return arch_hz; } +if (arch_hz && arch_hz - tsc_hz > arch_hz /100 )
Do we not need an "abs()" call on the "arch_hz - tsc_hz" in case tsc_hz is the bigger value?
Re: [PATCH v4 2/2] timer: allow platform to override cpu TSC frequency
Quoted 76 lines · Bruce Richardson
Quoted 11 lines · Isaac Boukris
The CPU value is often not accurate, allow overriding it based on info from the host OS.
On Linux X86, if the
tsc_known_freqcpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.On freebsd we have access to the kernel
tsc_hzvalue, just use it.Signed-off-by Isaac Boukris <[email protected]>---
Looks good to me. Hope to test it out soon on my systems to see how it goes.
One comment inline below.
/Bruce
Quoted 55 lines
lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 8 +++-- lib/eal/linux/eal_timer.c | 53 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 62 insertions(+), 9 deletions(-)lib/eal/common/eal_common_timer.c +1
-2@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();lib/eal/common/eal_private.h +1
-1@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports. lib/eal/freebsd/eal_timer.c +2
-2@@ -26,7 +26,7 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {size_t sz;int tmp;@@ -50,9 +50,13 @@ get_tsc_freq(void) sz =sizeof (tsc_hz);if (sysctlbyname ("machdep.tsc_freq" , &tsc_hz, &sz,NULL ,0 )) {EAL_LOG (WARNING ,"%s" ,strerror (errno)); -return 0 ; +return arch_hz; }
Quoted 4 lines
lib/eal/freebsd/eal_timer.c get_tsc_freq(void) · line 56
Quoted 1 line
lib/eal/freebsd/eal_timer.c +1
-0+if (arch_hz && arch_hz - tsc_hz > arch_hz /100 )Do we not need an "
abs()" call on the "arch_hz-tsc_hz" in casetsc_hzis the bigger value?
As they are unsigned it will overflow and result the absolute value, I actually did use abs() at first, and the compiler yelled at me:
warning: taking the absolute value of unsigned type ‘uint64_t’ {aka ‘long unsigned int’} has no effect [-Wabsolute-value]
Re: [PATCH v4 2/2] timer: allow platform to override cpu TSC frequency
Quoted 83 lines · Isaac Boukris
Quoted 76 lines · Bruce Richardson
Quoted 11 lines · Isaac Boukris
The CPU value is often not accurate, allow overriding it based on info from the host OS.
On Linux X86, if the
tsc_known_freqcpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.On freebsd we have access to the kernel
tsc_hzvalue, just use it.Signed-off-by Isaac Boukris <[email protected]>---
Looks good to me. Hope to test it out soon on my systems to see how it goes.
One comment inline below.
/Bruce
Quoted 55 lines
lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 8 +++-- lib/eal/linux/eal_timer.c | 53 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 62 insertions(+), 9 deletions(-)lib/eal/common/eal_common_timer.c +1
-2@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();lib/eal/common/eal_private.h +1
-1@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports. lib/eal/freebsd/eal_timer.c +2
-2@@ -26,7 +26,7 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {size_t sz;int tmp;@@ -50,9 +50,13 @@ get_tsc_freq(void) sz =sizeof (tsc_hz);if (sysctlbyname ("machdep.tsc_freq" , &tsc_hz, &sz,NULL ,0 )) {EAL_LOG (WARNING ,"%s" ,strerror (errno)); -return 0 ; +return arch_hz; }Quoted 4 lines
lib/eal/freebsd/eal_timer.c get_tsc_freq(void) · line 56
Quoted 1 line
lib/eal/freebsd/eal_timer.c +1
-0+if (arch_hz && arch_hz - tsc_hz > arch_hz /100 )Do we not need an "
abs()" call on the "arch_hz-tsc_hz" in casetsc_hzis the bigger value?
Quoted 4 lines
As they are unsigned it will overflow and result the absolute value, I actually did use
abs()at first, and the compiler yelled at me:
warning: taking the absolute value of unsigned type ‘uint64_t’ {aka ‘long unsigned int’} has no effect [-Wabsolute-value]
Yes, the compiler is right about them being unsigned. However, without using signed arithmetic and using abs, tsc_hz being even 1Hz greater than arch_hz will lead to the condition failing, so it is actually checking for tsc_hz being within 1% only on the low side.
/Bruce
Re: [PATCH v4 2/2] timer: allow platform to override cpu TSC frequency
Quoted 90 lines · Bruce Richardson
Quoted 83 lines · Isaac Boukris
Quoted 76 lines · Bruce Richardson
Quoted 11 lines · Isaac Boukris
The CPU value is often not accurate, allow overriding it based on info from the host OS.
On Linux X86, if the
tsc_known_freqcpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.On freebsd we have access to the kernel
tsc_hzvalue, just use it.Signed-off-by Isaac Boukris <[email protected]>---
Looks good to me. Hope to test it out soon on my systems to see how it goes.
One comment inline below.
/Bruce
Quoted 55 lines
lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 8 +++-- lib/eal/linux/eal_timer.c | 53 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 62 insertions(+), 9 deletions(-)lib/eal/common/eal_common_timer.c +1
-2@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();lib/eal/common/eal_private.h +1
-1@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports. lib/eal/freebsd/eal_timer.c +2
-2@@ -26,7 +26,7 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {size_t sz;int tmp;@@ -50,9 +50,13 @@ get_tsc_freq(void) sz =sizeof (tsc_hz);if (sysctlbyname ("machdep.tsc_freq" , &tsc_hz, &sz,NULL ,0 )) {EAL_LOG (WARNING ,"%s" ,strerror (errno)); -return 0 ; +return arch_hz; }Quoted 4 lines
lib/eal/freebsd/eal_timer.c get_tsc_freq(void) · line 56
Quoted 1 line
lib/eal/freebsd/eal_timer.c +1
-0+if (arch_hz && arch_hz - tsc_hz > arch_hz /100 )Do we not need an "
abs()" call on the "arch_hz-tsc_hz" in casetsc_hzis the bigger value?Quoted 4 lines
As they are unsigned it will overflow and result the absolute value, I actually did use
abs()at first, and the compiler yelled at me:
warning: taking the absolute value of unsigned type ‘uint64_t’ {aka ‘long unsigned int’} has no effect [-Wabsolute-value]
Quoted 4 lines
Yes, the compiler is right about them being unsigned. However, without using signed arithmetic and using abs,
tsc_hzbeing even 1Hz greater thanarch_hzwill lead to the condition failing, so it is actually checking fortsc_hzbeing within 1% only on the low side.
Ah right, I guess the 'has no effect' warning confused me.. V5 patch on its way.
Re: [PATCH v4 0/2] Improve TSC frequency accuracy
In reply to Isaac BoukrisQuoted 8 lines · Isaac Boukris
Changes in V4:
- lower ronding on Windows too
- on freebsd prefer systemtsc_hzIsaac Boukris (2):
timer: lower rounding of TSC estimation to 100KHz
timer: allow platform to override cpu TSC frequency
Quoted 8 lines
lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 8 +++-- lib/eal/linux/eal_timer.c | 59 +++++++++++++++++++++++++++---- lib/eal/windows/eal_timer.c | 11 +++--- 5 files changed, 68 insertions(+), 15 deletions(-)
I like this cleanup. One open on patch 2 still on my end. With that resolved:
v5 cover Improve TSC frequency accuracy
In reply to Isaac BoukrisChanges if V5:
- fix absolute delta calculation per Bruce's comment.
Isaac Boukris (2):
timer: lower rounding of TSC estimation to 100KHz
timer: allow platform to override cpu TSC frequency
lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 8 +++-- lib/eal/linux/eal_timer.c | 59 +++++++++++++++++++++++++++---- lib/eal/windows/eal_timer.c | 11 +++--- 5 files changed, 68 insertions(+), 15 deletions(-)
Signature · 1 line
2.45.0
PATCH v5 1/2 timer: lower rounding of TSC estimation to 100KHz
In practice, the frequency is often not a nice round number, while the estimation results are rather accurate, just a couple of KHz away from the kernel's tsc_khz value, so it should suffice.
Rounding to 10MHz can cause a significant drift from real time, up to a second per 10 minutes.
See also bugzilla: 959
--- lib/eal/linux/eal_timer.c | 6 +++--- lib/eal/windows/eal_timer.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)
lib/eal/linux/eal_timer.c +3 -3
@@ -192,7 +192,7 @@ get_tsc_freq(void) {#ifdef CLOCK_MONOTONIC_RAW #define NS_PER_SEC 1E9 -#define CYC_PER_10MHZ 1E7 +#define CYC_PER_100KHZ 1E5struct timespec sleeptime = {.tv_nsec =NS_PER_SEC /10 };/* 1/10 second */
@@ -209,8 +209,8 @@ get_tsc_freq(void)double secs = (double )ns/NS_PER_SEC ; tsc_hz = (uint64_t )((end - start)/secs); -/* Round up to 10Mhz. 1E7 ~ 10Mhz */ - return RTE_ALIGN_MUL_NEAR(tsc_hz, CYC_PER_10MHZ); +/* Round up to 100Khz. 1E5 ~ 100Khz */ + return RTE_ALIGN_MUL_NEAR(tsc_hz, CYC_PER_100KHZ); }#endif return 0 ;
lib/eal/windows/eal_timer.c +3 -3
@@ -12,7 +12,7 @@#include "eal_private.h" #define US_PER_SEC 1E6 -#define CYC_PER_10MHZ 1E7 +#define CYC_PER_100KHZ 1E5void rte_delay_us_sleep (unsigned int us)
@@ -81,8 +81,8 @@ get_tsc_freq(void)double secs = ((double )elapsed_us.QuadPart)/US_PER_SEC ; tsc_hz = (uint64_t )((end - start)/secs); -/* Round up to 10Mhz. 1E7 ~ 10Mhz */ -return RTE_ALIGN_MUL_NEAR (tsc_hz,CYC_PER_10MHZ ); +/* Round up to 100Khz. 1E5 ~ 100Khz */ +return RTE_ALIGN_MUL_NEAR (tsc_hz,CYC_PER_100KHZ ); }
Signature · 1 line
2.45.0
Re: [PATCH v5 1/2] timer: lower rounding of TSC estimation to 100KHz
Quoted 10 lines · Isaac Boukris
In practice, the frequency is often not a nice round number, while the estimation results are rather accurate, just a couple of KHz away from the kernel's
tsc_khzvalue, so it should suffice.Rounding to 10MHz can cause a significant drift from real time, up to a second per 10 minutes.
See also bugzilla: 959
Signed-off-by Isaac Boukris <[email protected]>
Stephen Hemminger
Acked-byRe: [PATCH v5 1/2] timer: lower rounding of TSC estimation to 100KHz
Quoted 5 lines · On Thu, 3 Oct 2024 15:05:02 +0100 Bruce Richardson…
Quoted 4 lines · Isaac Boukris
In practice, the frequency is often not a nice round number, while the estimation results are rather accurate, just a couple of KHz away from the kernel's
tsc_khzvalue, so it should suffice.
Quoted 8 lines
Quoted 6 lines
Rounding to 10MHz can cause a significant drift from real time, up to a second per 10 minutes.
See also bugzilla: 959
Signed-off-by Isaac Boukris <[email protected]>Acked-by Bruce Richardson <[email protected]>
Re: [PATCH v5 1/2] timer: lower rounding of TSC estimation to 100KHz
Quoted 8 lines · Stephen Hemminger
Quoted 5 lines · On Thu, 3 Oct 2024 15:05:02 +0100 Bruce Richardson…
Quoted 4 lines · Isaac Boukris
In practice, the frequency is often not a nice round number, while the estimation results are rather accurate, just a couple of KHz away from the kernel's
tsc_khzvalue, so it should suffice.
Quoted 8 lines
Quoted 7 lines
Quoted 6 lines
Rounding to 10MHz can cause a significant drift from real time, up to a second per 10 minutes.
See also bugzilla: 959
Signed-off-by Isaac Boukris <[email protected]>Acked-by Bruce Richardson <[email protected]>
Series applied.
Thanks Issac.
Signature · 1 line
David Marchand
PATCH v5 2/2 timer: allow platform to override cpu TSC frequency
In reply to Isaac BoukrisThe CPU provided value is often not accurate, allow overriding it based on info from the host OS.
On Linux X86, if the tsc_known_freq cpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.
On freebsd we have access to the kernel tsc_hz value, just use it.
--- lib/eal/common/eal_common_timer.c | 3 +- lib/eal/common/eal_private.h | 2 +- lib/eal/freebsd/eal_timer.c | 8 +++-- lib/eal/linux/eal_timer.c | 53 +++++++++++++++++++++++++++++-- lib/eal/windows/eal_timer.c | 5 ++- 5 files changed, 62 insertions(+), 9 deletions(-)
lib/eal/common/eal_common_timer.c +1 -2
@@ -66,8 +66,7 @@ set_tsc_freq(void) } freq =get_tsc_freq_arch (); -if (!freq) - freq =get_tsc_freq (); + freq =get_tsc_freq (freq);if (!freq) freq =estimate_tsc_freq ();
lib/eal/common/eal_private.h +1 -1
@@ -374,7 +374,7 @@ void set_tsc_freq(void);* * This function is private to the EAL. */ -uint64_t get_tsc_freq (void ); +uint64_t get_tsc_freq (uint64_t arch_hz);/** * Get TSC frequency if the architecture supports.
lib/eal/freebsd/eal_timer.c +6 -2
@@ -26,7 +26,7 @@enum timer_source eal_timer_source =EAL_TIMER_TSC ;uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {size_t sz;int tmp;
@@ -50,9 +50,13 @@ get_tsc_freq(void) sz =sizeof (tsc_hz);if (sysctlbyname ("machdep.tsc_freq" , &tsc_hz, &sz,NULL ,0 )) {EAL_LOG (WARNING ,"%s" ,strerror (errno)); -return 0 ; +return arch_hz; } +if (arch_hz &&RTE_MAX (arch_hz, tsc_hz) -RTE_MIN (arch_hz, tsc_hz) > arch_hz /100 ) +EAL_LOG (WARNING ,"Host tsc_freq %" PRIu64" at odds with cpu value %" PRIu64, + tsc_hz, arch_hz); +return tsc_hz; }
lib/eal/linux/eal_timer.c +50 -3
84 lines not on this page · raw message
lib/eal/windows/eal_timer.c +4 -1
@@ -49,13 +49,16 @@ rte_delay_us_sleep(unsigned int us) }uint64_t -get_tsc_freq (void ) +get_tsc_freq (uint64_t arch_hz) {LARGE_INTEGER t_start, t_end, elapsed_us;LARGE_INTEGER frequency;uint64_t tsc_hz;uint64_t end, start; +if (arch_hz) +return arch_hz; +QueryPerformanceFrequency (&frequency);QueryPerformanceCounter (&t_start);
Signature · 1 line
2.45.0
Re: [PATCH v5 2/2] timer: allow platform to override cpu TSC frequency
Quoted 11 lines · Isaac Boukris
The CPU provided value is often not accurate, allow overriding it based on info from the host OS.
On Linux X86, if the
tsc_known_freqcpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.On freebsd we have access to the kernel
tsc_hzvalue, just use it.Signed-off-by Isaac Boukris <[email protected]>---
Stephen Hemminger
Acked-byRe: [PATCH v5 2/2] timer: allow platform to override cpu TSC frequency
Quoted 8 lines · On Thu, 3 Oct 2024 15:06:06 +0100 Bruce Richardson…
Quoted 7 lines · Isaac Boukris
The CPU provided value is often not accurate, allow overriding it based on info from the host OS.
On Linux X86, if the
tsc_known_freqcpu flag is missing, it means the kernel doesn't trust it and calculates its own. We should do the same to avoid drift.
Quoted 5 lines
Quoted 4 lines
On freebsd we have access to the kernel
tsc_hzvalue, just use it.Signed-off-by Isaac Boukris <[email protected]>---
Acked-by Bruce Richardson <[email protected]>