Messages in this thread | | | Date | Fri, 31 May 2013 17:56:44 +0200 | From | Frederic Weisbecker <> | Subject | Re: [PATCH][RFC] tracing/context-tracking: Add preempt_schedule_context() for tracing |
| |
On Fri, May 31, 2013 at 11:18:48AM -0400, Steven Rostedt wrote: > On Fri, 2013-05-31 at 15:43 +0200, Frederic Weisbecker wrote: > > > > +void __sched notrace preempt_schedule_context(void) > > > +{ > > > + struct thread_info *ti = current_thread_info(); > > > + enum ctx_state prev_ctx; > > > + > > > + if (likely(ti->preempt_count || irqs_disabled())) > > > + return; > > > + > > > + /* > > > + * Need to disable preemption in case user_exit() is traced > > > + * and the tracer calls preempt_enable_notrace() causing > > > + * an infinite recursion. > > > + */ > > > + preempt_disable_notrace(); > > > + prev_ctx = this_cpu_read(context_tracking.state); > > > + user_exit(); > > > > You can reuse exception_enter() > > I originally did use that, but then noticed that everything else in > context_tracking.c used context_tracking.state directly. I have no > problems doing it this way again.
It's more about the fact that exception_*() APIs already implement part of what you're doing. And yeah as a bonus it's also better to keep context_tracking internals in context_tracking.c
> > > > > > + preempt_enable_no_resched_notrace(); > > > + > > > + preempt_schedule(); > > > + > > > + preempt_disable_notrace(); > > > + if (prev_ctx == IN_USER) > > > + user_enter(); > > > > And then exception_exit() here. > > > > I guess this replaces your fix with schedule_preempt_user(). I liked > > it because it seems that: > > > > if (need_resched()) { > > user_exit(); > > local_irq_enable(); > > schedule(); > > local_irq_enable(); > > user_enter(); > > } > > > > is a common pattern of arch user resume preemption that we can consolidate. > > > > But your new patch probably makes it more widely safe for the function tracer > > for any function that can be called and traced in IN_USER mode. Not only user preemption. > > Think about do_notify_resume() for example if it is called after syscall_trace_leave(). > > > > Independantly, schedule_preempt_user() is still interesting for consolidation. > > And I think that patch is still valid from just a clean up point of > view. It just didn't cover all the cases needed for tracing.
Right.
Thanks.
| |