Re: [dpdk-dev] [EXT] Re: [PATCH v2 2/2] eal: roundup tsc frequency when estimating it

2019-03-16 Thread Pavan Nikhilesh Bhagavatula
On Sat, 2019-03-16 at 18:22 +, Wiles, Keith wrote:
> > On Mar 16, 2019, at 12:56 PM, Pavan Nikhilesh Bhagavatula <
> > pbhagavat...@marvell.com> wrote:
> > 
> > On Sat, 2019-03-16 at 17:18 +, Wiles, Keith wrote:
> > > > On Mar 16, 2019, at 10:06 AM, Pavan Nikhilesh Bhagavatula <
> > > > pbhagavat...@marvell.com> wrote:
> > > > 
> > > > On Sat, 2019-03-16 at 14:42 +, Wiles, Keith wrote:
> > > > > > On Mar 16, 2019, at 2:03 AM, Pavan Nikhilesh Bhagavatula <
> > > > > > pbhagavat...@marvell.com> wrote:
> > > > > > 
> > > > > > From: Pavan Nikhilesh 
> > > > > > 
> > > > > > When estimating tsc frequency using sleep/gettime round it
> > > > > > up
> > > > > > to
> > > > > > the
> > > > > > nearest multiple of 10Mhz for more accuracy.
> > > 
> > > How does rounding up the TSC value become more accurate, If the
> > > value
> > > is 1 cycles more then it should be then your macro would round
> > > down
> > > and if it is 1 cycle greater than 1E7 it would round up.
> > 
> > Example in case of RTE_ARM_EAL_RDTSC_USE_PMU enabled 
> > 
> > Before roundup : 140979
> > After roundup : 14
> > EAL: TSC frequency is ~14 Hz
> > 
> > 
> > Before roundup : 139060
> > After roundup : 14
> > EAL: TSC frequency is ~14 Hz
> > 
> > > > > > Signed-off-by: Pavan Nikhilesh 
> > > > > > ---
> > > > > > Useful in case of ARM64 if we enable
> > > > > > RTE_ARM_EAL_RDTSC_USE_PMU,
> > > > > > get_tsc_freq_arch() will return 0 as there is no
> > > > > > instruction to
> > > > > > determine
> > > > > > the clk of PMU and eal falls back to sleep(1).
> > > > > > 
> > > > > > lib/librte_eal/common/eal_common_timer.c | 4 ++--
> > > > > > lib/librte_eal/linuxapp/eal/eal_timer.c  | 2 +-
> > > > > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > It appears you did not use the head of the master as linuxapp is
> > > now
> > > just linux and freebsdapp is freebsd. You need to rebase to the
> > > head
> > > of master and send a new version.
> > > > > > diff --git a/lib/librte_eal/common/eal_common_timer.c
> > > > > > b/lib/librte_eal/common/eal_common_timer.c
> > > > > > index dcf26bfea..1358bbed0 100644
> > > > > > --- a/lib/librte_eal/common/eal_common_timer.c
> > > > > > +++ b/lib/librte_eal/common/eal_common_timer.c
> > > > > > @@ -69,7 +69,7 @@ estimate_tsc_freq(void)
> > > > > > /* assume that the sleep(1) will sleep for 1 second */
> > > > > > uint64_t start = rte_rdtsc();
> > > > > > sleep(1);
> > > > > > -   return rte_rdtsc() - start;
> > > > > > +   return RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, 1E7);
> > > 
> > > The 1E7 is a magic number convert this to a meaningful define.
> > 
> > 1E7 ~ 10Mhz will convert to a macro.
> > 
> > > > > > }
> > > > > > 
> > > > > > void
> > > > > > @@ -83,7 +83,7 @@ set_tsc_freq(void)
> > > > > > if (!freq)
> > > > > > freq = estimate_tsc_freq();
> > > > > > 
> > > > > > -   RTE_LOG(DEBUG, EAL, "TSC frequency is ~%" PRIu64 "
> > > > > > KHz\n", freq
> > > > > > / 1000);
> > > > > > +   RTE_LOG(INFO, EAL, "TSC frequency is ~%" PRIu64 "
> > > > > > Hz\n", freq);
> > > > > > eal_tsc_resolution_hz = freq;
> > > > 
> > > > I missed this log will remove it in the next version.
> > > > 
> > > > > > }
> > > > > > 
> > > > > > diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c
> > > > > > b/lib/librte_eal/linuxapp/eal/eal_timer.c
> > > > > > index bc8f05199..864d6ef29 100644
> > > > > > --- a/lib/librte_eal/linuxapp/eal/eal_timer.c
> > > > > > +++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
> > > > > > @@ -248,7 +248,7 @@ get_tsc_freq(void)
> > > > > > 
> > > > > > double secs = (double)ns/NS_PER_SEC;
> > > > > > tsc_hz = (uint64_t)((end - start)/secs);
> > > > > > -   return tsc_hz;
> > > > > > +   return RTE_ALIGN_MUL_NEAR(tsc_hz, 1E7);
> > > > > 
> > > > > Maybe I missed an email about this, but why would I want the
> > > > > TSC
> > > > > hz
> > > > > rounded here? I do not mind the macro just the fact that we
> > > > > are
> > > > > changing TSC hz value. If the TSC value is wrong then we need
> > > > > to
> > > > > fix
> > > > > the value, but I do not see it being wrong here.
> > > > 
> > > > Since in this function nanosleep might not be cycle accurate we
> > > > need to
> > > > round it up.
> > > > 
> > > > Please note that estimation only applies
> > > > when  get_tsc_freq_arch()
> > > > fails. i.e there is no CPU instruction that specifies the
> > > > cyc/sec.
> > > > 
> > > > As I mentioned in the patch notes
> > > > "Useful in case of ARM64 if we enable
> > > > RTE_ARM_EAL_RDTSC_USE_PMU,
> > > > get_tsc_freq_arch() will return 0 as there is no instruction to
> > > > determine the clock of PMU and eal falls back to
> > > > sleep(1)/nanosleep.” 
> > > 
> > > OK, I looked at the changes and the code for setting the TSC
> > > again. I
> > > would have not handled the setting of TSC in the way it was done,
> > > but
> > > that is not your problem. I agree the changes do lo

Re: [dpdk-dev] [EXT] Re: [PATCH v2 2/2] eal: roundup tsc frequency when estimating it

2019-03-16 Thread Wiles, Keith


> On Mar 16, 2019, at 12:56 PM, Pavan Nikhilesh Bhagavatula 
>  wrote:
> 
> On Sat, 2019-03-16 at 17:18 +, Wiles, Keith wrote:
>>> On Mar 16, 2019, at 10:06 AM, Pavan Nikhilesh Bhagavatula <
>>> pbhagavat...@marvell.com> wrote:
>>> 
>>> On Sat, 2019-03-16 at 14:42 +, Wiles, Keith wrote:
> On Mar 16, 2019, at 2:03 AM, Pavan Nikhilesh Bhagavatula <
> pbhagavat...@marvell.com> wrote:
> 
> From: Pavan Nikhilesh 
> 
> When estimating tsc frequency using sleep/gettime round it up
> to
> the
> nearest multiple of 10Mhz for more accuracy.
>> 
>> How does rounding up the TSC value become more accurate, If the value
>> is 1 cycles more then it should be then your macro would round down
>> and if it is 1 cycle greater than 1E7 it would round up.
> 
> Example in case of RTE_ARM_EAL_RDTSC_USE_PMU enabled 
> 
> Before roundup : 140979
> After roundup : 14
> EAL: TSC frequency is ~14 Hz
> 
> 
> Before roundup : 139060
> After roundup : 14
> EAL: TSC frequency is ~14 Hz
> 
> Signed-off-by: Pavan Nikhilesh 
> ---
> Useful in case of ARM64 if we enable RTE_ARM_EAL_RDTSC_USE_PMU,
> get_tsc_freq_arch() will return 0 as there is no instruction to
> determine
> the clk of PMU and eal falls back to sleep(1).
> 
> lib/librte_eal/common/eal_common_timer.c | 4 ++--
> lib/librte_eal/linuxapp/eal/eal_timer.c  | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>> 
>> It appears you did not use the head of the master as linuxapp is now
>> just linux and freebsdapp is freebsd. You need to rebase to the head
>> of master and send a new version.
> diff --git a/lib/librte_eal/common/eal_common_timer.c
> b/lib/librte_eal/common/eal_common_timer.c
> index dcf26bfea..1358bbed0 100644
> --- a/lib/librte_eal/common/eal_common_timer.c
> +++ b/lib/librte_eal/common/eal_common_timer.c
> @@ -69,7 +69,7 @@ estimate_tsc_freq(void)
>   /* assume that the sleep(1) will sleep for 1 second */
>   uint64_t start = rte_rdtsc();
>   sleep(1);
> - return rte_rdtsc() - start;
> + return RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, 1E7);
>> 
>> The 1E7 is a magic number convert this to a meaningful define.
> 
> 1E7 ~ 10Mhz will convert to a macro.
> 
> }
> 
> void
> @@ -83,7 +83,7 @@ set_tsc_freq(void)
>   if (!freq)
>   freq = estimate_tsc_freq();
> 
> - RTE_LOG(DEBUG, EAL, "TSC frequency is ~%" PRIu64 "
> KHz\n", freq
> / 1000);
> + RTE_LOG(INFO, EAL, "TSC frequency is ~%" PRIu64 "
> Hz\n", freq);
>   eal_tsc_resolution_hz = freq;
>>> 
>>> I missed this log will remove it in the next version.
>>> 
> }
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c
> b/lib/librte_eal/linuxapp/eal/eal_timer.c
> index bc8f05199..864d6ef29 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_timer.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
> @@ -248,7 +248,7 @@ get_tsc_freq(void)
> 
>   double secs = (double)ns/NS_PER_SEC;
>   tsc_hz = (uint64_t)((end - start)/secs);
> - return tsc_hz;
> + return RTE_ALIGN_MUL_NEAR(tsc_hz, 1E7);
 
 Maybe I missed an email about this, but why would I want the TSC
 hz
 rounded here? I do not mind the macro just the fact that we are
 changing TSC hz value. If the TSC value is wrong then we need to
 fix
 the value, but I do not see it being wrong here.
>>> 
>>> Since in this function nanosleep might not be cycle accurate we
>>> need to
>>> round it up.
>>> 
>>> Please note that estimation only applies when  get_tsc_freq_arch()
>>> fails. i.e there is no CPU instruction that specifies the cyc/sec.
>>> 
>>> As I mentioned in the patch notes
>>> "Useful in case of ARM64 if we enable RTE_ARM_EAL_RDTSC_USE_PMU,
>>> get_tsc_freq_arch() will return 0 as there is no instruction to
>>> determine the clock of PMU and eal falls back to
>>> sleep(1)/nanosleep.” 
>> 
>> OK, I looked at the changes and the code for setting the TSC again. I
>> would have not handled the setting of TSC in the way it was done, but
>> that is not your problem. I agree the changes do look ok, the only
>> problem I have is the new macro will roundup or down depending on the
>> value. In your statement you are wanting to roundup the values.
>> 
>> If the sleep/nanosleep is less than a second for some reason, then
>> your macro would round it down is that what we wanted? I guess my
>> point is you are assuming the TSC calculation will always be less
>> than a second (with sleep) and the macro would round up depending on
>> the value calculated using the sleep/nanosleep.
>> 
>> I was playing with these MUL macros and I am not sure they do what we
>> expect in the case of the multiple value is much closer to the value
>> passed.
>> 
>> If we have a v = 10001 and multiple to 1000 we have the following:
>> 
>> RTE_ALIGN_MUL_CEIL(10001, 1000)

Re: [dpdk-dev] [EXT] Re: [PATCH v2 2/2] eal: roundup tsc frequency when estimating it

2019-03-16 Thread Pavan Nikhilesh Bhagavatula
On Sat, 2019-03-16 at 17:18 +, Wiles, Keith wrote:
> > On Mar 16, 2019, at 10:06 AM, Pavan Nikhilesh Bhagavatula <
> > pbhagavat...@marvell.com> wrote:
> > 
> > On Sat, 2019-03-16 at 14:42 +, Wiles, Keith wrote:
> > > > On Mar 16, 2019, at 2:03 AM, Pavan Nikhilesh Bhagavatula <
> > > > pbhagavat...@marvell.com> wrote:
> > > > 
> > > > From: Pavan Nikhilesh 
> > > > 
> > > > When estimating tsc frequency using sleep/gettime round it up
> > > > to
> > > > the
> > > > nearest multiple of 10Mhz for more accuracy.
> 
> How does rounding up the TSC value become more accurate, If the value
> is 1 cycles more then it should be then your macro would round down
> and if it is 1 cycle greater than 1E7 it would round up.

Example in case of RTE_ARM_EAL_RDTSC_USE_PMU enabled 

Before roundup : 140979
After roundup : 14
EAL: TSC frequency is ~14 Hz


Before roundup : 139060
After roundup : 14
EAL: TSC frequency is ~14 Hz

> > > > Signed-off-by: Pavan Nikhilesh 
> > > > ---
> > > > Useful in case of ARM64 if we enable RTE_ARM_EAL_RDTSC_USE_PMU,
> > > > get_tsc_freq_arch() will return 0 as there is no instruction to
> > > > determine
> > > > the clk of PMU and eal falls back to sleep(1).
> > > > 
> > > > lib/librte_eal/common/eal_common_timer.c | 4 ++--
> > > > lib/librte_eal/linuxapp/eal/eal_timer.c  | 2 +-
> > > > 2 files changed, 3 insertions(+), 3 deletions(-)
> 
> It appears you did not use the head of the master as linuxapp is now
> just linux and freebsdapp is freebsd. You need to rebase to the head
> of master and send a new version.
> > > > diff --git a/lib/librte_eal/common/eal_common_timer.c
> > > > b/lib/librte_eal/common/eal_common_timer.c
> > > > index dcf26bfea..1358bbed0 100644
> > > > --- a/lib/librte_eal/common/eal_common_timer.c
> > > > +++ b/lib/librte_eal/common/eal_common_timer.c
> > > > @@ -69,7 +69,7 @@ estimate_tsc_freq(void)
> > > > /* assume that the sleep(1) will sleep for 1 second */
> > > > uint64_t start = rte_rdtsc();
> > > > sleep(1);
> > > > -   return rte_rdtsc() - start;
> > > > +   return RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, 1E7);
> 
> The 1E7 is a magic number convert this to a meaningful define.

1E7 ~ 10Mhz will convert to a macro.

> > > > }
> > > > 
> > > > void
> > > > @@ -83,7 +83,7 @@ set_tsc_freq(void)
> > > > if (!freq)
> > > > freq = estimate_tsc_freq();
> > > > 
> > > > -   RTE_LOG(DEBUG, EAL, "TSC frequency is ~%" PRIu64 "
> > > > KHz\n", freq
> > > > / 1000);
> > > > +   RTE_LOG(INFO, EAL, "TSC frequency is ~%" PRIu64 "
> > > > Hz\n", freq);
> > > > eal_tsc_resolution_hz = freq;
> > 
> > I missed this log will remove it in the next version.
> > 
> > > > }
> > > > 
> > > > diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c
> > > > b/lib/librte_eal/linuxapp/eal/eal_timer.c
> > > > index bc8f05199..864d6ef29 100644
> > > > --- a/lib/librte_eal/linuxapp/eal/eal_timer.c
> > > > +++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
> > > > @@ -248,7 +248,7 @@ get_tsc_freq(void)
> > > > 
> > > > double secs = (double)ns/NS_PER_SEC;
> > > > tsc_hz = (uint64_t)((end - start)/secs);
> > > > -   return tsc_hz;
> > > > +   return RTE_ALIGN_MUL_NEAR(tsc_hz, 1E7);
> > > 
> > > Maybe I missed an email about this, but why would I want the TSC
> > > hz
> > > rounded here? I do not mind the macro just the fact that we are
> > > changing TSC hz value. If the TSC value is wrong then we need to
> > > fix
> > > the value, but I do not see it being wrong here.
> > 
> > Since in this function nanosleep might not be cycle accurate we
> > need to
> > round it up.
> > 
> > Please note that estimation only applies when  get_tsc_freq_arch()
> > fails. i.e there is no CPU instruction that specifies the cyc/sec.
> > 
> > As I mentioned in the patch notes
> > "Useful in case of ARM64 if we enable RTE_ARM_EAL_RDTSC_USE_PMU,
> > get_tsc_freq_arch() will return 0 as there is no instruction to
> > determine the clock of PMU and eal falls back to
> > sleep(1)/nanosleep.” 
> 
> OK, I looked at the changes and the code for setting the TSC again. I
> would have not handled the setting of TSC in the way it was done, but
> that is not your problem. I agree the changes do look ok, the only
> problem I have is the new macro will roundup or down depending on the
> value. In your statement you are wanting to roundup the values.
> 
> If the sleep/nanosleep is less than a second for some reason, then
> your macro would round it down is that what we wanted? I guess my
> point is you are assuming the TSC calculation will always be less
> than a second (with sleep) and the macro would round up depending on
> the value calculated using the sleep/nanosleep.
> 
> I was playing with these MUL macros and I am not sure they do what we
> expect in the case of the multiple value is much closer to the value
> passed.
> 
> If we have a v = 10001 and multiple to 

Re: [dpdk-dev] [EXT] Re: [PATCH v2 2/2] eal: roundup tsc frequency when estimating it

2019-03-16 Thread Wiles, Keith


> On Mar 16, 2019, at 10:06 AM, Pavan Nikhilesh Bhagavatula 
>  wrote:
> 
> On Sat, 2019-03-16 at 14:42 +, Wiles, Keith wrote:
>>> On Mar 16, 2019, at 2:03 AM, Pavan Nikhilesh Bhagavatula <
>>> pbhagavat...@marvell.com> wrote:
>>> 
>>> From: Pavan Nikhilesh 
>>> 
>>> When estimating tsc frequency using sleep/gettime round it up to
>>> the
>>> nearest multiple of 10Mhz for more accuracy.

How does rounding up the TSC value become more accurate, If the value is 1 
cycles more then it should be then your macro would round down and if it is 1 
cycle greater than 1E7 it would round up.
>>> 
>>> Signed-off-by: Pavan Nikhilesh 
>>> ---
>>> Useful in case of ARM64 if we enable RTE_ARM_EAL_RDTSC_USE_PMU,
>>> get_tsc_freq_arch() will return 0 as there is no instruction to
>>> determine
>>> the clk of PMU and eal falls back to sleep(1).
>>> 
>>> lib/librte_eal/common/eal_common_timer.c | 4 ++--
>>> lib/librte_eal/linuxapp/eal/eal_timer.c  | 2 +-
>>> 2 files changed, 3 insertions(+), 3 deletions(-)

It appears you did not use the head of the master as linuxapp is now just linux 
and freebsdapp is freebsd. You need to rebase to the head of master and send a 
new version.
>>> 
>>> diff --git a/lib/librte_eal/common/eal_common_timer.c
>>> b/lib/librte_eal/common/eal_common_timer.c
>>> index dcf26bfea..1358bbed0 100644
>>> --- a/lib/librte_eal/common/eal_common_timer.c
>>> +++ b/lib/librte_eal/common/eal_common_timer.c
>>> @@ -69,7 +69,7 @@ estimate_tsc_freq(void)
>>> /* assume that the sleep(1) will sleep for 1 second */
>>> uint64_t start = rte_rdtsc();
>>> sleep(1);
>>> -   return rte_rdtsc() - start;
>>> +   return RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, 1E7);

The 1E7 is a magic number convert this to a meaningful define.
>>> }
>>> 
>>> void
>>> @@ -83,7 +83,7 @@ set_tsc_freq(void)
>>> if (!freq)
>>> freq = estimate_tsc_freq();
>>> 
>>> -   RTE_LOG(DEBUG, EAL, "TSC frequency is ~%" PRIu64 " KHz\n", freq
>>> / 1000);
>>> +   RTE_LOG(INFO, EAL, "TSC frequency is ~%" PRIu64 " Hz\n", freq);
>>> eal_tsc_resolution_hz = freq;
> 
> I missed this log will remove it in the next version.
> 
>>> }
>>> 
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c
>>> b/lib/librte_eal/linuxapp/eal/eal_timer.c
>>> index bc8f05199..864d6ef29 100644
>>> --- a/lib/librte_eal/linuxapp/eal/eal_timer.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
>>> @@ -248,7 +248,7 @@ get_tsc_freq(void)
>>> 
>>> double secs = (double)ns/NS_PER_SEC;
>>> tsc_hz = (uint64_t)((end - start)/secs);
>>> -   return tsc_hz;
>>> +   return RTE_ALIGN_MUL_NEAR(tsc_hz, 1E7);
>> 
>> Maybe I missed an email about this, but why would I want the TSC hz
>> rounded here? I do not mind the macro just the fact that we are
>> changing TSC hz value. If the TSC value is wrong then we need to fix
>> the value, but I do not see it being wrong here.
> 
> Since in this function nanosleep might not be cycle accurate we need to
> round it up.
> 
> Please note that estimation only applies when  get_tsc_freq_arch()
> fails. i.e there is no CPU instruction that specifies the cyc/sec.
> 
> As I mentioned in the patch notes
> "Useful in case of ARM64 if we enable RTE_ARM_EAL_RDTSC_USE_PMU,
> get_tsc_freq_arch() will return 0 as there is no instruction to
> determine the clock of PMU and eal falls back to sleep(1)/nanosleep.” 

OK, I looked at the changes and the code for setting the TSC again. I would 
have not handled the setting of TSC in the way it was done, but that is not 
your problem. I agree the changes do look ok, the only problem I have is the 
new macro will roundup or down depending on the value. In your statement you 
are wanting to roundup the values.

If the sleep/nanosleep is less than a second for some reason, then your macro 
would round it down is that what we wanted? I guess my point is you are 
assuming the TSC calculation will always be less than a second (with sleep) and 
the macro would round up depending on the value calculated using the 
sleep/nanosleep.

I was playing with these MUL macros and I am not sure they do what we expect in 
the case of the multiple value is much closer to the value passed.

If we have a v = 10001 and multiple to 1000 we have the following:

RTE_ALIGN_MUL_CEIL(10001, 1000)
(10001 + (1000 - 1)) / (1000 * 1000)
(10001 + 999)/ 100
2/ 100
Result: 0

RTE_ALIGN_MUL_FLOOR(10001, 1000)
(10001 / (1000 * 1000))
(10001 / 100)
Result: 0

Unless I am wrong here the value v must be over a 1,000,000 to make these 
macros work or the value v to be greater than (mul * mul) in all cases or zero 
is the result. It may work for the TSC values as we are using a small mul value 
compared to the TSC value. If DPDK was ported to a slower machine it could be 
also zero.

I think we need to fix the macros and rethink how TSC is set here.

> 
>>> }
>>> #endif
>>> return 0;
>>> --
>>> 2.21.0
>>> 

Re: [dpdk-dev] [EXT] Re: [PATCH v2 2/2] eal: roundup tsc frequency when estimating it

2019-03-16 Thread Pavan Nikhilesh Bhagavatula
On Sat, 2019-03-16 at 14:42 +, Wiles, Keith wrote:
> > On Mar 16, 2019, at 2:03 AM, Pavan Nikhilesh Bhagavatula <
> > pbhagavat...@marvell.com> wrote:
> > 
> > From: Pavan Nikhilesh 
> > 
> > When estimating tsc frequency using sleep/gettime round it up to
> > the
> > nearest multiple of 10Mhz for more accuracy.
> > 
> > Signed-off-by: Pavan Nikhilesh 
> > ---
> > Useful in case of ARM64 if we enable RTE_ARM_EAL_RDTSC_USE_PMU,
> > get_tsc_freq_arch() will return 0 as there is no instruction to
> > determine
> > the clk of PMU and eal falls back to sleep(1).
> > 
> > lib/librte_eal/common/eal_common_timer.c | 4 ++--
> > lib/librte_eal/linuxapp/eal/eal_timer.c  | 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/librte_eal/common/eal_common_timer.c
> > b/lib/librte_eal/common/eal_common_timer.c
> > index dcf26bfea..1358bbed0 100644
> > --- a/lib/librte_eal/common/eal_common_timer.c
> > +++ b/lib/librte_eal/common/eal_common_timer.c
> > @@ -69,7 +69,7 @@ estimate_tsc_freq(void)
> > /* assume that the sleep(1) will sleep for 1 second */
> > uint64_t start = rte_rdtsc();
> > sleep(1);
> > -   return rte_rdtsc() - start;
> > +   return RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, 1E7);
> > }
> > 
> > void
> > @@ -83,7 +83,7 @@ set_tsc_freq(void)
> > if (!freq)
> > freq = estimate_tsc_freq();
> > 
> > -   RTE_LOG(DEBUG, EAL, "TSC frequency is ~%" PRIu64 " KHz\n", freq
> > / 1000);
> > +   RTE_LOG(INFO, EAL, "TSC frequency is ~%" PRIu64 " Hz\n", freq);
> > eal_tsc_resolution_hz = freq;

I missed this log will remove it in the next version.

> > }
> > 
> > diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c
> > b/lib/librte_eal/linuxapp/eal/eal_timer.c
> > index bc8f05199..864d6ef29 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal_timer.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
> > @@ -248,7 +248,7 @@ get_tsc_freq(void)
> > 
> > double secs = (double)ns/NS_PER_SEC;
> > tsc_hz = (uint64_t)((end - start)/secs);
> > -   return tsc_hz;
> > +   return RTE_ALIGN_MUL_NEAR(tsc_hz, 1E7);
> 
> Maybe I missed an email about this, but why would I want the TSC hz
> rounded here? I do not mind the macro just the fact that we are
> changing TSC hz value. If the TSC value is wrong then we need to fix
> the value, but I do not see it being wrong here.

Since in this function nanosleep might not be cycle accurate we need to
round it up.

Please note that estimation only applies when  get_tsc_freq_arch()
fails. i.e there is no CPU instruction that specifies the cyc/sec.

As I mentioned in the patch notes
"Useful in case of ARM64 if we enable RTE_ARM_EAL_RDTSC_USE_PMU,
get_tsc_freq_arch() will return 0 as there is no instruction to
determine the clock of PMU and eal falls back to sleep(1)/nanosleep." 

> > }
> > #endif
> > return 0;
> > --
> > 2.21.0
> > 
> 
> Regards,
> Keith
>