Re: [Xen-devel] [PATCH v5 08/12] x86/ioapic: Refactor the delay logic in timer_irq_works()
Hi Thomas, At 07/03/2017 03:15 AM, Thomas Gleixner wrote: On Fri, 30 Jun 2017, Dou Liyang wrote: +static void __init delay_with_tsc(void) +{ + unsigned long long start, now; + unsigned long ticks = jiffies; Please make that unsigned long end = jiffies + 4; ticks really means: number of ticks. But that variable is doing something different. um, I see. Will use 'end' instead. + start = rdtsc(); + + /* +* We don't know the TSC frequency yet, but waiting for +* 400/HZ TSC cycles is safe: +* 4 GHz == 10 jiffies +* 1 GHz == 40 jiffies +*/ + do { + rep_nop(); + now = rdtsc(); + } while ((now - start) < 400UL / HZ && + time_before_eq(jiffies, ticks + 4)); +} + +static void __init delay_without_tsc(void) +{ + int band = 1; + unsigned long ticks = jiffies; Please sort variables in reverse fir tree order unsigned long end = jiffies + 4; int band = 1; OK, I will. + + /* +* We don't know any frequency yet, but waiting for +* 4094000/HZ cycles is safe: +* 4 GHz == 10 jiffies +* 1 GHz == 40 jiffies +* 1 << 1 + 1 << 2 +...+ 1 << 11 = 4094 +*/ + do { + __delay(((1 << band++) * 1000UL) / HZ); s/1/1U/ Got it! Thanks, dou. + } while (band < 12 && time_before_eq(jiffies, ticks + 4)); +} Thanks, tglx ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 08/12] x86/ioapic: Refactor the delay logic in timer_irq_works()
On Fri, 30 Jun 2017, Dou Liyang wrote: > +static void __init delay_with_tsc(void) > +{ > + unsigned long long start, now; > + unsigned long ticks = jiffies; Please make that unsigned long end = jiffies + 4; ticks really means: number of ticks. But that variable is doing something different. > + start = rdtsc(); > + > + /* > + * We don't know the TSC frequency yet, but waiting for > + * 400/HZ TSC cycles is safe: > + * 4 GHz == 10 jiffies > + * 1 GHz == 40 jiffies > + */ > + do { > + rep_nop(); > + now = rdtsc(); > + } while ((now - start) < 400UL / HZ && > + time_before_eq(jiffies, ticks + 4)); > +} > + > +static void __init delay_without_tsc(void) > +{ > + int band = 1; > + unsigned long ticks = jiffies; Please sort variables in reverse fir tree order unsigned long end = jiffies + 4; int band = 1; > + > + /* > + * We don't know any frequency yet, but waiting for > + * 4094000/HZ cycles is safe: > + * 4 GHz == 10 jiffies > + * 1 GHz == 40 jiffies > + * 1 << 1 + 1 << 2 +...+ 1 << 11 = 4094 > + */ > + do { > + __delay(((1 << band++) * 1000UL) / HZ); s/1/1U/ > + } while (band < 12 && time_before_eq(jiffies, ticks + 4)); > +} Thanks, tglx ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 08/12] x86/ioapic: Refactor the delay logic in timer_irq_works()
Kernel use timer_irq_works() to detects the timer IRQs. It calls mdelay(10) to delay ten ticks and check whether the timer IRQ work or not. The mdelay() depends on the loops_per_jiffy which is set up in calibrate_delay(). Current kernel defaults the IRQ 0 is available when it calibrates delay. But it is wrong in the dump-capture kernel with 'notsc' option inherited from 1st kernel option. dump-capture kernel can't make sure the timer IRQ works well. The correct design is making the interrupt mode setup and checking timer IRQ works in advance of calibrate_delay(). That results in the mdelay() being unusable in timer_irq_works(). Preparatory patch to make the setup in advance. Refactor the delay logic by waiting for some cycles. In the system with X86_FEATURE_TSC feature, Use rdtsc(), others will call __delay() directly. Note: regard 4G as the max CPU frequence of current single CPU. Signed-off-by: Dou Liyang--- arch/x86/kernel/apic/io_apic.c | 45 -- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 347bb9f..f710077 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -1607,6 +1607,43 @@ static int __init notimercheck(char *s) } __setup("no_timer_check", notimercheck); +static void __init delay_with_tsc(void) +{ + unsigned long long start, now; + unsigned long ticks = jiffies; + + start = rdtsc(); + + /* +* We don't know the TSC frequency yet, but waiting for +* 400/HZ TSC cycles is safe: +* 4 GHz == 10 jiffies +* 1 GHz == 40 jiffies +*/ + do { + rep_nop(); + now = rdtsc(); + } while ((now - start) < 400UL / HZ && + time_before_eq(jiffies, ticks + 4)); +} + +static void __init delay_without_tsc(void) +{ + int band = 1; + unsigned long ticks = jiffies; + + /* +* We don't know any frequency yet, but waiting for +* 4094000/HZ cycles is safe: +* 4 GHz == 10 jiffies +* 1 GHz == 40 jiffies +* 1 << 1 + 1 << 2 +...+ 1 << 11 = 4094 +*/ + do { + __delay(((1 << band++) * 1000UL) / HZ); + } while (band < 12 && time_before_eq(jiffies, ticks + 4)); +} + /* * There is a nasty bug in some older SMP boards, their mptable lies * about the timer IRQ. We do the following to work around the situation: @@ -1625,8 +1662,12 @@ static int __init timer_irq_works(void) local_save_flags(flags); local_irq_enable(); - /* Let ten ticks pass... */ - mdelay((10 * 1000) / HZ); + + if (boot_cpu_has(X86_FEATURE_TSC)) + delay_with_tsc(); + else + delay_without_tsc(); + local_irq_restore(flags); /* -- 2.5.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel