| Message ID | 512207A6.4000402@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 18 February 2013 11:51, Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> wrote: > On 02/18/2013 04:04 PM, Srivatsa S. Bhat wrote: >> On 02/18/2013 03:54 PM, Vincent Guittot wrote: >>> On 15 February 2013 20:40, Srivatsa S. Bhat >>> <srivatsa.bhat@linux.vnet.ibm.com> wrote: >>>> Hi Vincent, >>>> >>>> On 02/15/2013 06:58 PM, Vincent Guittot wrote: >>>>> Hi Srivatsa, >>>>> >>>>> I have run some tests with you branch (thanks Paul for the git tree) >>>>> and you will find results below. >>>>> >>>> >>>> Thank you very much for testing this patchset! >>>> >>>>> The tests condition are: >>>>> - 5 CPUs system in 2 clusters >>>>> - The test plugs/unplugs CPU2 and it increases the system load each 20 >>>>> plug/unplug sequence with either more cyclictests threads >>>>> - The test is done with all CPUs online and with only CPU0 and CPU2 >>>>> >>>>> The main conclusion is that there is no differences with and without >>>>> your patches with my stress tests. I'm not sure that it was the >>>>> expected results but the cpu_down is already quite low : 4-5ms in >>>>> average >>>>> >>>> >>>> Atleast my patchset doesn't perform _worse_ than mainline, with respect >>>> to cpu_down duration :-) >>> >>> yes exactly and it has pass more than 400 consecutive plug/unplug on >>> an ARM platform >>> >> >> Great! However, did you turn on CPU_IDLE during your tests? >> >> In my tests, I had turned off cpu idle in the .config, like I had mentioned in >> the cover letter. I'm struggling to get it working with CPU_IDLE/INTEL_IDLE >> turned on, because it gets into a lockup almost immediately. It appears that >> the lock-holder of clockevents_lock never releases it, for some reason.. >> See below for the full log. Lockdep has not been useful in debugging this, >> unfortunately :-( >> > > Ah, nevermind, the following diff fixes it :-) I had applied this fix on v5 > and tested but it still had races where I used to hit the lockups. Now after > I fixed all the memory barrier issues that Paul and Oleg pointed out in v5, > I applied this fix again and tested it just now - it works beautifully! :-) My tests have been done without cpuidle because i have some issues with function tracer and cpuidle But the cpu hotplug and cpuidle work well when I run the tests without enabling the function tracer Vincent > > I'll include this fix and post a v6 soon. > > Regards, > Srivatsa S. Bhat > > ---------------------------------------------------------------------------> > > > diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c > index 30b6de0..ca340fd 100644 > --- a/kernel/time/clockevents.c > +++ b/kernel/time/clockevents.c > @@ -17,6 +17,7 @@ > #include <linux/module.h> > #include <linux/notifier.h> > #include <linux/smp.h> > +#include <linux/cpu.h> > > #include "tick-internal.h" > > @@ -431,6 +432,7 @@ void clockevents_notify(unsigned long reason, void *arg) > unsigned long flags; > int cpu; > > + get_online_cpus_atomic(); > raw_spin_lock_irqsave(&clockevents_lock, flags); > clockevents_do_notify(reason, arg); > > @@ -459,6 +461,7 @@ void clockevents_notify(unsigned long reason, void *arg) > break; > } > raw_spin_unlock_irqrestore(&clockevents_lock, flags); > + put_online_cpus_atomic(); > } > EXPORT_SYMBOL_GPL(clockevents_notify); > #endif >
On Mon, 2013-02-18 at 11:58 +0100, Vincent Guittot wrote: > My tests have been done without cpuidle because i have some issues > with function tracer and cpuidle > > But the cpu hotplug and cpuidle work well when I run the tests without > enabling the function tracer > I know suspend and resume has issues with function tracing (because it makes things like calling smp_processor_id() crash the system), but I'm unaware of issues with hotplug itself. Could be some of the same issues. Can you give me more details, I'll try to investigate it. Thanks, -- Steve
On 18 February 2013 16:30, Steven Rostedt <rostedt@goodmis.org> wrote: > On Mon, 2013-02-18 at 11:58 +0100, Vincent Guittot wrote: > >> My tests have been done without cpuidle because i have some issues >> with function tracer and cpuidle >> >> But the cpu hotplug and cpuidle work well when I run the tests without >> enabling the function tracer >> > > I know suspend and resume has issues with function tracing (because it > makes things like calling smp_processor_id() crash the system), but I'm > unaware of issues with hotplug itself. Could be some of the same issues. > > Can you give me more details, I'll try to investigate it. yes for sure. The problem is more linked to cpuidle and function tracer. cpu hotplug and function tracer work when cpuidle is disable. cpu hotplug and cpuidle works if i don't enable function tracer. my platform is dead as soon as I enable function tracer if cpuidle is enabled. It looks like some notrace are missing in my platform driver but we haven't completely fix the issue yet Vincent > > Thanks, > > -- Steve > >
On Mon, 2013-02-18 at 17:50 +0100, Vincent Guittot wrote: > yes for sure. > The problem is more linked to cpuidle and function tracer. > > cpu hotplug and function tracer work when cpuidle is disable. > cpu hotplug and cpuidle works if i don't enable function tracer. > my platform is dead as soon as I enable function tracer if cpuidle is > enabled. It looks like some notrace are missing in my platform driver > but we haven't completely fix the issue yet > You can bisect to find out exactly what function is the problem: cat /debug/tracing/available_filter_functions > t f(t) { num=`wc -l t` sed -ne "1,${num}p" t > t1 let num=num+1 sed -ne "${num},$p" t > t2 cat t1 > /debug/tracing/set_ftrace_filter # note this may take a long time to finish echo function > /debug/tracing/current_tracer <failed? bisect f(t1), if not bisect f(t2)> }
On Mon, 2013-02-18 at 17:50 +0100, Vincent Guittot wrote: > yes for sure. > The problem is more linked to cpuidle and function tracer. > > cpu hotplug and function tracer work when cpuidle is disable. > cpu hotplug and cpuidle works if i don't enable function tracer. > my platform is dead as soon as I enable function tracer if cpuidle is > enabled. It looks like some notrace are missing in my platform driver > but we haven't completely fix the issue yet > You can bisect to find out exactly what function is the problem: cat /debug/tracing/available_filter_functions > t f(t) { num=`wc -l t` sed -ne "1,${num}p" t > t1 let num=num+1 sed -ne "${num},$p" t > t2 cat t1 > /debug/tracing/set_ftrace_filter # note this may take a long time to finish echo function > /debug/tracing/current_tracer <failed? bisect f(t1), if not bisect f(t2)> } -- Steve
On 18 February 2013 20:53, Steven Rostedt <rostedt@goodmis.org> wrote: > On Mon, 2013-02-18 at 17:50 +0100, Vincent Guittot wrote: > >> yes for sure. >> The problem is more linked to cpuidle and function tracer. >> >> cpu hotplug and function tracer work when cpuidle is disable. >> cpu hotplug and cpuidle works if i don't enable function tracer. >> my platform is dead as soon as I enable function tracer if cpuidle is >> enabled. It looks like some notrace are missing in my platform driver >> but we haven't completely fix the issue yet >> > > You can bisect to find out exactly what function is the problem: > > cat /debug/tracing/available_filter_functions > t > > f(t) { > num=`wc -l t` > sed -ne "1,${num}p" t > t1 > let num=num+1 > sed -ne "${num},$p" t > t2 > > cat t1 > /debug/tracing/set_ftrace_filter > # note this may take a long time to finish > > echo function > /debug/tracing/current_tracer > > <failed? bisect f(t1), if not bisect f(t2)> > } > Thanks, i'm going to have a look Vincent > -- Steve > > >
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c index 30b6de0..ca340fd 100644 --- a/kernel/time/clockevents.c +++ b/kernel/time/clockevents.c @@ -17,6 +17,7 @@ #include <linux/module.h> #include <linux/notifier.h> #include <linux/smp.h> +#include <linux/cpu.h> #include "tick-internal.h" @@ -431,6 +432,7 @@ void clockevents_notify(unsigned long reason, void *arg) unsigned long flags; int cpu; + get_online_cpus_atomic(); raw_spin_lock_irqsave(&clockevents_lock, flags); clockevents_do_notify(reason, arg); @@ -459,6 +461,7 @@ void clockevents_notify(unsigned long reason, void *arg) break; } raw_spin_unlock_irqrestore(&clockevents_lock, flags); + put_online_cpus_atomic(); } EXPORT_SYMBOL_GPL(clockevents_notify); #endif