Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH] x86: enable preemption in delay

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


rostedt at goodmis

May 24, 2008, 8:11 PM

Post #1 of 14 (1962 views)
Permalink
[PATCH] x86: enable preemption in delay

The RT team has been searching for a nasty latency. This latency shows
up out of the blue and has been seen to be as big as 5ms!

Using ftrace I found the cause of the latency.

pcscd-2995 3dNh1 52360300us : irq_exit (smp_apic_timer_interrupt)
pcscd-2995 3dN.2 52360301us : idle_cpu (irq_exit)
pcscd-2995 3dN.2 52360301us : rcu_irq_exit (irq_exit)
pcscd-2995 3dN.1 52360771us : smp_apic_timer_interrupt (apic_timer_interrupt
)
pcscd-2995 3dN.1 52360771us : exit_idle (smp_apic_timer_interrupt)

Here's an example of a 400 us latency. pcscd took a timer interrupt and
returned with "need resched" enabled, but did not reschedule until after
the next interrupt came in at 52360771us 400us later!

At first I thought we somehow missed a preemption check in entry.S. But
I also noticed that this always seemed to happen during a __delay call.

pcscd-2995 3dN.2 52360836us : rcu_irq_exit (irq_exit)
pcscd-2995 3.N.. 52361265us : preempt_schedule (__delay)

Looking at the x86 delay, I found my problem.

In git commit 35d5d08a085c56f153458c3f5d8ce24123617faf, Andrew Morton
placed preempt_disable around the entire delay due to TSC's not working
nicely on SMP. Unfortunately for those that care about latencies this
is devastating! Especially when we have callers to mdelay(8).

Here I enable preemption during the loop and account for anytime the task
migrates to a new CPU. The delay asked for may be extended a bit by
the migration, but delay only guarantees that it will delay for that minimum
time. Delaying longer should not be an issue.

Signed-off-by: Steven Rostedt <srostedt [at] redhat>
---
arch/x86/lib/delay_32.c | 23 ++++++++++++++++++++++-
arch/x86/lib/delay_64.c | 26 +++++++++++++++++++++++---
2 files changed, 45 insertions(+), 4 deletions(-)

Index: linux-compile.git/arch/x86/lib/delay_32.c
===================================================================
--- linux-compile.git.orig/arch/x86/lib/delay_32.c 2008-05-24 22:30:49.000000000 -0400
+++ linux-compile.git/arch/x86/lib/delay_32.c 2008-05-24 22:31:43.000000000 -0400
@@ -44,12 +44,33 @@ static void delay_loop(unsigned long loo
static void delay_tsc(unsigned long loops)
{
unsigned long bclock, now;
+ int cpu;

- preempt_disable(); /* TSC's are per-cpu */
+ preempt_disable();
+ cpu = smp_processor_id();
rdtscl(bclock);
do {
rep_nop();
rdtscl(now);
+ /* Allow RT tasks to run */
+ preempt_enable();
+ preempt_disable();
+ /*
+ * It is possible that we moved to another CPU,
+ * and since TSC's are per-cpu we need to
+ * calculate that. The delay must guarantee that
+ * we wait "at least" the amount of time. Being
+ * moved to another CPU could make the wait longer
+ * but we just need to make sure we waited long
+ * enough. Rebalance the counter for this CPU.
+ */
+ if (unlikely(cpu != smp_processor_id())) {
+ if ((now-bclock) >= loops)
+ break;
+ loops -= (now - bclock);
+ rdtscl(bclock);
+ rdtscl(now);
+ }
} while ((now-bclock) < loops);
preempt_enable();
}
Index: linux-compile.git/arch/x86/lib/delay_64.c
===================================================================
--- linux-compile.git.orig/arch/x86/lib/delay_64.c 2008-05-24 22:30:49.000000000 -0400
+++ linux-compile.git/arch/x86/lib/delay_64.c 2008-05-24 22:31:43.000000000 -0400
@@ -31,14 +31,34 @@ int __devinit read_current_timer(unsigne
void __delay(unsigned long loops)
{
unsigned bclock, now;
+ int cpu;

- preempt_disable(); /* TSC's are pre-cpu */
+ preempt_disable();
+ cpu = smp_processor_id();
rdtscl(bclock);
do {
rep_nop();
rdtscl(now);
- }
- while ((now-bclock) < loops);
+ /* Allow RT tasks to run */
+ preempt_enable();
+ preempt_disable();
+ /*
+ * It is possible that we moved to another CPU,
+ * and since TSC's are per-cpu we need to
+ * calculate that. The delay must guarantee that
+ * we wait "at least" the amount of time. Being
+ * moved to another CPU could make the wait longer
+ * but we just need to make sure we waited long
+ * enough. Rebalance the counter for this CPU.
+ */
+ if (unlikely(cpu != smp_processor_id())) {
+ if ((now-bclock) >= loops)
+ break;
+ loops -= (now - bclock);
+ rdtscl(bclock);
+ rdtscl(now);
+ }
+ } while ((now-bclock) < loops);
preempt_enable();
}
EXPORT_SYMBOL(__delay);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


tglx at linutronix

May 25, 2008, 5:44 AM

Post #2 of 14 (1872 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

On Sat, 24 May 2008, Steven Rostedt wrote:
> The RT team has been searching for a nasty latency. This latency shows
> up out of the blue and has been seen to be as big as 5ms!
>
> @@ -44,12 +44,33 @@ static void delay_loop(unsigned long loo
> static void delay_tsc(unsigned long loops)
> {
> unsigned long bclock, now;
> + int cpu;
>
> - preempt_disable(); /* TSC's are per-cpu */
> + preempt_disable();
> + cpu = smp_processor_id();
> rdtscl(bclock);
> do {
> rep_nop();
> rdtscl(now);
> + /* Allow RT tasks to run */
> + preempt_enable();
> + preempt_disable();
> + /*
> + * It is possible that we moved to another CPU,
> + * and since TSC's are per-cpu we need to
> + * calculate that. The delay must guarantee that
> + * we wait "at least" the amount of time. Being
> + * moved to another CPU could make the wait longer
> + * but we just need to make sure we waited long
> + * enough. Rebalance the counter for this CPU.
> + */
> + if (unlikely(cpu != smp_processor_id())) {

Eeek, once you migrated you do this all the time. you need to update
cpu here.

> + if ((now-bclock) >= loops)
> + break;

Also this is really dangerous with unsynchronized TSCs. You might get
migrated and return immediately because the TSC on the other CPU is
far ahead.

What you really want is something like the patch below, but we should
reuse the sched_clock_cpu() thingy to make that simpler. Looking into
that right now.

Thanks,
tglx

diff --git a/arch/x86/lib/delay_32.c b/arch/x86/lib/delay_32.c
index 4535e6d..66a3c32 100644
--- a/arch/x86/lib/delay_32.c
+++ b/arch/x86/lib/delay_32.c
@@ -40,17 +40,51 @@ static void delay_loop(unsigned long loops)
:"0" (loops));
}

+/*
+ * 5 usec on a 1GHZ machine. Not necessarily correct, but not too long
+ * either.
+ */
+#define TSC_MIGRATE_COUNT 5000
+
/* TSC based delay: */
static void delay_tsc(unsigned long loops)
{
unsigned long bclock, now;
+ int cpu;

- preempt_disable(); /* TSC's are per-cpu */
+ preempt_disable();
+ cpu = smp_processor_id();
rdtscl(bclock);
do {
rep_nop();
- rdtscl(now);
- } while ((now-bclock) < loops);
+
+ /* Allow RT tasks to run */
+ preempt_enable();
+ preempt_disable();
+
+ /*
+ * It is possible that we moved to another CPU, and
+ * since TSC's are per-cpu we need to calculate
+ * that. The delay must guarantee that we wait "at
+ * least" the amount of time. Being moved to another
+ * CPU could make the wait longer but we just need to
+ * make sure we waited long enough. Rebalance the
+ * counter for this CPU.
+ */
+ if (unlikely(cpu != smp_processor_id())) {
+ if (loops <= TSC_MIGRATE_COUNT)
+ break;
+ cpu = smp_processor_id();
+ rdtscl(bclock);
+ loops -= TSC_MIGRATE_COUNT;
+ } else {
+ rdtscl(now);
+ if ((now - bclock) >= loops)
+ break;
+ loops -= (now - bclock);
+ bclock = now;
+ }
+ } while (loops > 0);
preempt_enable();
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rostedt at goodmis

May 25, 2008, 6:07 AM

Post #3 of 14 (1888 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

On Sun, 25 May 2008, Thomas Gleixner wrote:
> > - preempt_disable(); /* TSC's are per-cpu */
> > + preempt_disable();
> > + cpu = smp_processor_id();
> > rdtscl(bclock);
> > do {
> > rep_nop();
> > rdtscl(now);
> > + /* Allow RT tasks to run */
> > + preempt_enable();
> > + preempt_disable();
> > + /*
> > + * It is possible that we moved to another CPU,
> > + * and since TSC's are per-cpu we need to
> > + * calculate that. The delay must guarantee that
> > + * we wait "at least" the amount of time. Being
> > + * moved to another CPU could make the wait longer
> > + * but we just need to make sure we waited long
> > + * enough. Rebalance the counter for this CPU.
> > + */
> > + if (unlikely(cpu != smp_processor_id())) {
>
> Eeek, once you migrated you do this all the time. you need to update
> cpu here.

Good catch! I'll update that.

>
> > + if ((now-bclock) >= loops)
> > + break;
>
> Also this is really dangerous with unsynchronized TSCs. You might get
> migrated and return immediately because the TSC on the other CPU is
> far ahead.

No it isn't ;-)

The now and bclock are both from before the migration. The cpus were the
same becaues we were under preempt disbled at the time. I recalculate
after the change has been noticed.

But you are right, I forgot to update cpu. :-/

>
> What you really want is something like the patch below, but we should
> reuse the sched_clock_cpu() thingy to make that simpler. Looking into
> that right now.
>

Sure, but this should be simple enough.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rostedt at goodmis

May 25, 2008, 6:17 AM

Post #4 of 14 (1873 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

On Sun, 25 May 2008, Thomas Gleixner wrote:
>
> +/*
> + * 5 usec on a 1GHZ machine. Not necessarily correct, but not too long
> + * either.

And what happens when we have 10GHz boxes that can do migration in 1us,
and the delay that is asked for is 2us. We can return early. I don't like
to place assumptions of this kind that can hurt with future hardware
enhancements.

-- Steve


> + */
> +#define TSC_MIGRATE_COUNT 5000
> +
> /* TSC based delay: */
> static void delay_tsc(unsigned long loops)
> {
> unsigned long bclock, now;
> + int cpu;
>
> - preempt_disable(); /* TSC's are per-cpu */
> + preempt_disable();
> + cpu = smp_processor_id();
> rdtscl(bclock);
> do {
> rep_nop();
> - rdtscl(now);
> - } while ((now-bclock) < loops);
> +
> + /* Allow RT tasks to run */
> + preempt_enable();
> + preempt_disable();
> +
> + /*
> + * It is possible that we moved to another CPU, and
> + * since TSC's are per-cpu we need to calculate
> + * that. The delay must guarantee that we wait "at
> + * least" the amount of time. Being moved to another
> + * CPU could make the wait longer but we just need to
> + * make sure we waited long enough. Rebalance the
> + * counter for this CPU.
> + */
> + if (unlikely(cpu != smp_processor_id())) {
> + if (loops <= TSC_MIGRATE_COUNT)
> + break;
> + cpu = smp_processor_id();
> + rdtscl(bclock);
> + loops -= TSC_MIGRATE_COUNT;
> + } else {
> + rdtscl(now);
> + if ((now - bclock) >= loops)
> + break;
> + loops -= (now - bclock);
> + bclock = now;
> + }
> + } while (loops > 0);
> preempt_enable();
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


alan at lxorguk

May 25, 2008, 6:19 AM

Post #5 of 14 (1886 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

> And what happens when we have 10GHz boxes that can do migration in 1us,
> and the delay that is asked for is 2us. We can return early. I don't like
> to place assumptions of this kind that can hurt with future hardware
> enhancements.

Then in the hypothetical future you fix it, and for now its clearly
documented. Th preempt botch in the current tree breaks all sorts of
existing real world setups so needs to go. (and we have people who do
stuff like mdelay(150);

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rostedt at goodmis

May 25, 2008, 6:40 AM

Post #6 of 14 (1887 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

On Sun, 25 May 2008, Alan Cox wrote:

> > And what happens when we have 10GHz boxes that can do migration in 1us,
> > and the delay that is asked for is 2us. We can return early. I don't like
> > to place assumptions of this kind that can hurt with future hardware
> > enhancements.
>
> Then in the hypothetical future you fix it, and for now its clearly

No, I'm saying that we don't need to account for the migrate. It will
only make the delay longer, and if the code was preemptible, that delay
is not guaranteed to be that long anyway.


> documented. Th preempt botch in the current tree breaks all sorts of
> existing real world setups so needs to go. (and we have people who do
> stuff like mdelay(150);

My updated patch takes Thomas's patch into consideration. There's no need
to put on limited timeouts due to migration.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


pavel at ucw

May 25, 2008, 11:01 AM

Post #7 of 14 (1827 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

Hi!

> > + if (unlikely(cpu != smp_processor_id())) {
>
> Eeek, once you migrated you do this all the time. you need to update
> cpu here.
>
> > + if ((now-bclock) >= loops)
> > + break;
>
> Also this is really dangerous with unsynchronized TSCs. You might get
> migrated and return immediately because the TSC on the other CPU is
> far ahead.
>
> What you really want is something like the patch below, but we should
> reuse the sched_clock_cpu() thingy to make that simpler. Looking into
> that right now.

> @@ -40,17 +40,51 @@ static void delay_loop(unsigned long loops)
> :"0" (loops));
> }
>
> +/*
> + * 5 usec on a 1GHZ machine. Not necessarily correct, but not too long
> + * either.
> + */
> +#define TSC_MIGRATE_COUNT 5000
> +
> /* TSC based delay: */
> static void delay_tsc(unsigned long loops)
> {
> unsigned long bclock, now;
> + int cpu;
>
> - preempt_disable(); /* TSC's are per-cpu */
> + preempt_disable();
> + cpu = smp_processor_id();
> rdtscl(bclock);
> do {
> rep_nop();
> - rdtscl(now);
> - } while ((now-bclock) < loops);
> +
> + /* Allow RT tasks to run */
> + preempt_enable();
> + preempt_disable();
> +
> + /*
> + * It is possible that we moved to another CPU, and
> + * since TSC's are per-cpu we need to calculate
> + * that. The delay must guarantee that we wait "at
> + * least" the amount of time. Being moved to another
> + * CPU could make the wait longer but we just need to
> + * make sure we waited long enough. Rebalance the
> + * counter for this CPU.
> + */
> + if (unlikely(cpu != smp_processor_id())) {
> + if (loops <= TSC_MIGRATE_COUNT)
> + break;
> + cpu = smp_processor_id();
> + rdtscl(bclock);
> + loops -= TSC_MIGRATE_COUNT;
> + } else {
> + rdtscl(now);
> + if ((now - bclock) >= loops)
> + break;
> + loops -= (now - bclock);
> + bclock = now;

What happens with different cpus running on different frequencies...?
Cpufreq?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


arjan at infradead

May 25, 2008, 11:53 AM

Post #8 of 14 (1860 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

On Sat, 24 May 2008 23:11:20 -0400 (EDT)
Steven Rostedt <rostedt [at] goodmis> wrote:
>
> In git commit 35d5d08a085c56f153458c3f5d8ce24123617faf, Andrew Morton
> placed preempt_disable around the entire delay due to TSC's not
> working nicely on SMP. Unfortunately for those that care about
> latencies this is devastating! Especially when we have callers to
> mdelay(8).

we used to have a WARN_ON if mdelay was called while preemptable..
maybe we should put that back in?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


tglx at linutronix

May 25, 2008, 12:01 PM

Post #9 of 14 (1860 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

On Sun, 25 May 2008, Arjan van de Ven wrote:

> On Sat, 24 May 2008 23:11:20 -0400 (EDT)
> Steven Rostedt <rostedt [at] goodmis> wrote:
> >
> > In git commit 35d5d08a085c56f153458c3f5d8ce24123617faf, Andrew Morton
> > placed preempt_disable around the entire delay due to TSC's not
> > working nicely on SMP. Unfortunately for those that care about
> > latencies this is devastating! Especially when we have callers to
> > mdelay(8).
>
> we used to have a WARN_ON if mdelay was called while preemptable..
> maybe we should put that back in?

We'd better have one which warns, when mdelay is called with
preemption disabled.

Thanks,
tglx

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


arjan at infradead

May 25, 2008, 12:28 PM

Post #10 of 14 (1870 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

On Sun, 25 May 2008 21:01:32 +0200 (CEST)
Thomas Gleixner <tglx [at] linutronix> wrote:

> On Sun, 25 May 2008, Arjan van de Ven wrote:
>
> > On Sat, 24 May 2008 23:11:20 -0400 (EDT)
> > Steven Rostedt <rostedt [at] goodmis> wrote:
> > >
> > > In git commit 35d5d08a085c56f153458c3f5d8ce24123617faf, Andrew
> > > Morton placed preempt_disable around the entire delay due to
> > > TSC's not working nicely on SMP. Unfortunately for those that
> > > care about latencies this is devastating! Especially when we have
> > > callers to mdelay(8).
> >
> > we used to have a WARN_ON if mdelay was called while preemptable..
> > maybe we should put that back in?
>
> We'd better have one which warns, when mdelay is called with
> preemption disabled.

argument for my variant was "should have used msleep instead"...

I don't disagree that mdelay() is harmful in general; at some point I
stuck a WARN_ON in mdelay when called from irq context...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rostedt at goodmis

May 25, 2008, 12:32 PM

Post #11 of 14 (1870 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

On Sun, 25 May 2008, Arjan van de Ven wrote:

> On Sat, 24 May 2008 23:11:20 -0400 (EDT)
> Steven Rostedt <rostedt [at] goodmis> wrote:
> >
> > In git commit 35d5d08a085c56f153458c3f5d8ce24123617faf, Andrew Morton
> > placed preempt_disable around the entire delay due to TSC's not
> > working nicely on SMP. Unfortunately for those that care about
> > latencies this is devastating! Especially when we have callers to
> > mdelay(8).
>
> we used to have a WARN_ON if mdelay was called while preemptable..
> maybe we should put that back in?
>

That would be a good idea, but that wouldn't solve this issue. The issue
here is that the mdelay itself disables preemption. The caller had
preemption enabled.

The problem was caused by a quick fix by Andrew to handle out of sync
TSC's for a delay that used TSC's as a counter.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


tglx at linutronix

May 25, 2008, 1:21 PM

Post #12 of 14 (1862 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

On Sun, 25 May 2008, Steven Rostedt wrote:
>
> On Sun, 25 May 2008, Thomas Gleixner wrote:
> >
> > +/*
> > + * 5 usec on a 1GHZ machine. Not necessarily correct, but not too long
> > + * either.
>
> And what happens when we have 10GHz boxes that can do migration in 1us,
> and the delay that is asked for is 2us. We can return early. I don't like
> to place assumptions of this kind that can hurt with future hardware
> enhancements.

Do the math. It's 500ns on a 10GHz machine then. So nothing to worry
about and future proof.

Also when those 10GHZ machines arrive the synchronized TSC should have
finally become reality.

Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rostedt at goodmis

May 28, 2008, 6:01 AM

Post #13 of 14 (1828 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

On Sun, 25 May 2008, Pavel Machek wrote:
> > + if (unlikely(cpu != smp_processor_id())) {
> > + if (loops <= TSC_MIGRATE_COUNT)
> > + break;
> > + cpu = smp_processor_id();
> > + rdtscl(bclock);
> > + loops -= TSC_MIGRATE_COUNT;
> > + } else {
> > + rdtscl(now);
> > + if ((now - bclock) >= loops)
> > + break;
> > + loops -= (now - bclock);
> > + bclock = now;
>
> What happens with different cpus running on different frequencies...?
> Cpufreq?

It's not even protected with the old code.

inline void __const_udelay(unsigned long xloops)
{
__delay(((xloops * HZ *
cpu_data(raw_smp_processor_id()).loops_per_jiffy) >> 32) + 1);
}

Here it calculates the loops_per_jiffy for the CPU and calls into __delay.
But it can easily be preempted here and the delay could run on another
CPU.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


pavel at ucw

May 31, 2008, 12:18 AM

Post #14 of 14 (1802 views)
Permalink
Re: [PATCH] x86: enable preemption in delay [In reply to]

On Wed 2008-05-28 09:01:06, Steven Rostedt wrote:
>
>
> On Sun, 25 May 2008, Pavel Machek wrote:
> > > + if (unlikely(cpu != smp_processor_id())) {
> > > + if (loops <= TSC_MIGRATE_COUNT)
> > > + break;
> > > + cpu = smp_processor_id();
> > > + rdtscl(bclock);
> > > + loops -= TSC_MIGRATE_COUNT;
> > > + } else {
> > > + rdtscl(now);
> > > + if ((now - bclock) >= loops)
> > > + break;
> > > + loops -= (now - bclock);
> > > + bclock = now;
> >
> > What happens with different cpus running on different frequencies...?
> > Cpufreq?
>
> It's not even protected with the old code.

Maybe, but it is simple to fix as long as preemption is disabled. When
you enable it, it becomes much harder.

Lets get that fixed.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.