Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1442426 > unrolled thread

[PATCH v3 1/3] arch, x86, tsc deadline clockevent dev: reduce frequency roundoff error

Started byNicolai Stange <nicstange@gmail.com>
First post2016-07-13 15:10 +0200
Last post2016-07-13 16:40 +0200
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v3 1/3] arch, x86, tsc deadline clockevent dev: reduce frequency roundoff error Nicolai Stange <nicstange@gmail.com> - 2016-07-13 15:10 +0200
    Re: [PATCH v3 1/3] arch, x86, tsc deadline clockevent dev: reduce  frequency roundoff error Peter Zijlstra <peterz@infradead.org> - 2016-07-13 16:00 +0200
      Re: [PATCH v3 1/3] arch, x86, tsc deadline clockevent dev: reduce  frequency roundoff error Paolo Bonzini <pbonzini@redhat.com> - 2016-07-13 16:00 +0200
        Re: [PATCH v3 1/3] arch, x86, tsc deadline clockevent dev: reduce frequency roundoff error Nicolai Stange <nicstange@gmail.com> - 2016-07-13 16:40 +0200

#1442426 — [PATCH v3 1/3] arch, x86, tsc deadline clockevent dev: reduce frequency roundoff error

FromNicolai Stange <nicstange@gmail.com>
Date2016-07-13 15:10 +0200
Subject[PATCH v3 1/3] arch, x86, tsc deadline clockevent dev: reduce frequency roundoff error
Message-ID<rUs5k-3uO-1@gated-at.bofh.it>
In setup_APIC_timer(), the registered clockevent device's frequency
is calculated by first dividing tsc_khz by TSC_DIVISOR and multiplying
it with 1000 afterwards.

The multiplication with 1000 is done for converting from kHz to Hz and the
division by TSC_DIVISOR is carried out in order to make sure that the final
result fits into an u32.

However, with the order given in this calculation, the roundoff error
introduced by the division gets magnified by a factor of 1000 by the
following multiplication.

Increase the accuracy by reversing the order of the division and
multiplication. In order not to overflow during this calculation, cast
temporarily to u64.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 arch/x86/kernel/apic/apic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 89a5bce..dce654c 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -563,7 +563,8 @@ static void setup_APIC_timer(void)
 				    CLOCK_EVT_FEAT_DUMMY);
 		levt->set_next_event = lapic_next_deadline;
 		clockevents_config_and_register(levt,
-						(tsc_khz / TSC_DIVISOR) * 1000,
+						(u32)(((u64)tsc_khz * 1000) /
+							TSC_DIVISOR),
 						0xF, ~0UL);
 	} else
 		clockevents_register_device(levt);
-- 
2.9.0

[toc] | [next] | [standalone]


#1442478 — Re: [PATCH v3 1/3] arch, x86, tsc deadline clockevent dev: reduce frequency roundoff error

FromPeter Zijlstra <peterz@infradead.org>
Date2016-07-13 16:00 +0200
SubjectRe: [PATCH v3 1/3] arch, x86, tsc deadline clockevent dev: reduce frequency roundoff error
Message-ID<rUsRH-3OH-13@gated-at.bofh.it>
In reply to#1442426
On Wed, Jul 13, 2016 at 03:03:42PM +0200, Nicolai Stange wrote:

>  		clockevents_config_and_register(levt,
> +						(u32)(((u64)tsc_khz * 1000) /
> +							TSC_DIVISOR),
>  						0xF, ~0UL);

div_u64() perhaps ?

[toc] | [prev] | [next] | [standalone]


#1442479 — Re: [PATCH v3 1/3] arch, x86, tsc deadline clockevent dev: reduce frequency roundoff error

FromPaolo Bonzini <pbonzini@redhat.com>
Date2016-07-13 16:00 +0200
SubjectRe: [PATCH v3 1/3] arch, x86, tsc deadline clockevent dev: reduce frequency roundoff error
Message-ID<rUsRH-3OH-21@gated-at.bofh.it>
In reply to#1442478

On 13/07/2016 15:49, Peter Zijlstra wrote:
> On Wed, Jul 13, 2016 at 03:03:42PM +0200, Nicolai Stange wrote:
> 
>>  		clockevents_config_and_register(levt,
>> +						(u32)(((u64)tsc_khz * 1000) /
>> +							TSC_DIVISOR),
>>  						0xF, ~0UL);
> 
> div_u64() perhaps ?

Or just squash together the two patches and do

	tsc_khz * (1000 / TSC_DIVISOR)

because with TSC_DIVISOR equal to 2/4/8 there is no error from
reassociating the operation.

Paolo

[toc] | [prev] | [next] | [standalone]


#1442502

FromNicolai Stange <nicstange@gmail.com>
Date2016-07-13 16:40 +0200
Message-ID<rUtuq-4iQ-11@gated-at.bofh.it>
In reply to#1442479
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 13/07/2016 15:49, Peter Zijlstra wrote:
>> On Wed, Jul 13, 2016 at 03:03:42PM +0200, Nicolai Stange wrote:
>> 
>>>  		clockevents_config_and_register(levt,
>>> +						(u32)(((u64)tsc_khz * 1000) /
>>> +							TSC_DIVISOR),
>>>  						0xF, ~0UL);
>> 
>> div_u64() perhaps ?
>
> Or just squash together the two patches and do
>
> 	tsc_khz * (1000 / TSC_DIVISOR)
>
> because with TSC_DIVISOR equal to 2/4/8 there is no error from
> reassociating the operation.

Oh great, I didn't see this. Will resend.

Thanks,

Nicolai

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web