lkml.org 
[lkml]   [2011]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH,RFC] perf: panic due to inclied cpu context task_ctx value
    On Wed, Mar 30, 2011 at 06:37:30PM +0200, Oleg Nesterov wrote:
    > On 03/30, Peter Zijlstra wrote:
    > >
    > > --- linux-2.6.orig/kernel/perf_event.c
    > > +++ linux-2.6/kernel/perf_event.c
    > > @@ -125,9 +125,25 @@ enum event_type_t {
    > > * perf_sched_events : >0 events exist
    > > * perf_cgroup_events: >0 per-cpu cgroup events exist on this cpu
    > > */
    > > -atomic_t perf_sched_events __read_mostly;
    > > +atomic_t perf_sched_events_in __read_mostly;
    > > +atomic_t perf_sched_events_out __read_mostly;
    > > static DEFINE_PER_CPU(atomic_t, perf_cgroup_events);
    > >
    > > +static void perf_sched_events_inc(void)
    > > +{
    > > + jump_label_inc(&perf_sched_events_out);
    > > + jump_label_inc(&perf_sched_events_in);
    > > +}
    > > +
    > > +static void perf_sched_events_dec(void)
    > > +{
    > > + jump_label_dec(&perf_sched_events_in);
    > > + JUMP_LABEL(&perf_sched_events_in, no_sync);
    > > + synchronize_sched();
    > > +no_sync:
    > > + jump_label_dec(&perf_sched_events_out);
    > > +}
    >
    > Nice! I didn't realize we can simply use JUMP_LABEL() directly and then
    > the code doesn't depend on HAVE_JUMP_LABEL.
    >
    > Now, the problem is, after I read the comments I am not sure I understand
    > what synchronize_sched() actually doe. Add Paul.
    >
    > So. synchronize_sched() above should ensure that all CPUs do context
    > switch at least once (ignoring idle). And I _thought_ that in practice
    > this should work.
    >
    > But, unles I misread the comment above synchronize_sched(), it seems that
    > it only guarantees the end of "everything" which disables preemption,
    > explicitly or not. IOW, say, in theory rcu_read_unlock_sched() could
    > trigger ->passed_quiesc == T without reschedule.

    For rcu_read_lock() in preemptible RCU, this is true. But for
    rcu_read_unlock_sched(), the only way rcu_note_context_switch() is called
    is if the code is preempted, which ends up calling schedule().

    > Oh, and this is not theoretical, afaics. run_ksoftirqd() does
    > rcu_note_context_switch().

    Interesting... Color me confused.

    Suppose the rcu_note_context_switch() in run_ksoftirqd() was replaced
    with schedule(). This has to be OK, right? But schedule() itself
    invokes rcu_note_context_switch(). So if it is OK to call schedule(),
    it should be OK to call rcu_note_context_switch() directly, right?

    So, what am I missing here?

    > So, I think we need something else :/

    The thing that I would be more concerned about is the idle loop.
    If a CPU is in the idle loop, then rcu_sched_qs() will be invoked
    (and which is invoked by rcu_note_context_switch()). So is it
    illegal to use the above in the idle loop?

    BTW, if it turns out that the idle loop is a problem, I could put
    an explicit call to rcu_sched_qs() in the affected idle loops.
    But currently anything in an idle thread is a quiescent state.

    Thanx, Paul


    \
     
     \ /
      Last update: 2011-03-30 20:33    [W:0.162 / U:0.324 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site