Messages in this thread | | | Date | Thu, 11 Dec 2014 13:31:21 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH] tracing/sched: Check preempt_count() for current when reading task->state |
| |
* Steven Rostedt <rostedt@goodmis.org> wrote:
> On Thu, 11 Dec 2014 07:38:11 +0100 > Ingo Molnar <mingo@kernel.org> wrote: > > > > Cc: stable@vger.kernel.org # 3.13+ > > > Fixes: 01028747559a "sched: Create more preempt_count accessors" > > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > > --- > > > include/trace/events/sched.h | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h > > > index 0a68d5ae584e..13fbadcc172b 100644 > > > --- a/include/trace/events/sched.h > > > +++ b/include/trace/events/sched.h > > > @@ -97,10 +97,14 @@ static inline long __trace_sched_switch_state(struct task_struct *p) > > > long state = p->state; > > > > > > #ifdef CONFIG_PREEMPT > > > + unsigned long pc; > > > + > > > + pc = (p == current) ? preempt_count() : task_preempt_count(p); > > > + > > > /* > > > * For all intents and purposes a preempted task is a running task. > > > */ > > > - if (task_preempt_count(p) & PREEMPT_ACTIVE) > > > + if (pc & PREEMPT_ACTIVE) > > > state = TASK_RUNNING | TASK_STATE_MAX; > > > > I really don't like the overhead around here. > > Hi Ingo! > > What overhead are you worried about? Note, this is in the > schedule tracepoint and does not affect the scheduler itself > (as long as the tracepoint is not enabled).
Scheduler tracepoints are pretty popular, so I'm worried about their complexity when they are activated.
> I'm also thinking that as long as "prev" is always guaranteed > to be "current" we can remove the check and just use > preempt_count() always. But I'm worried that we can't > guaranteed that.
You could add a WARN_ON_ONCE() or so to double check that assumption?
> What other ideas do you have? Because wrong data is worse than > the overhead of the above code. If Thomas taught me anything, > it's that!
My idea is to have simpler, yet correct code. And ponies!
Thanks,
Ingo
| |