Re: [PATCH RFC v5 1/6] softirq: reorder trace_softirqs_on to prevent lockdep splat

[Joel Fernandes]

On Tue, May 1, 2018 at 7:02 AM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> On Mon, 30 Apr 2018 18:41:59 -0700
> Joel Fernandes <joelaf@xxxxxxxxxx> wrote:

> > I'm able to reproduce a lockdep splat when CONFIG_PROVE_LOCKING=y and
> > CONFIG_PREEMPTIRQ_EVENTS=y.

> Needs more info in the change log. It also requires that
> CONFIG_DEBUG_LOCKED=y is set.

> Add this to the change log:

> The issue was this:

> Start with: preempt_count = 1 << SOFTIRQ_SHIFT

>          __local_bh_enable(cnt = 1 << SOFTIRQ_SHIFT) {
>                  if (softirq_count() == (cnt && SOFTIRQ_MASK)) {
>                          trace_softirqs_on() {
>                                  current->softirqs_enabled = 1;
>                          }
>                  }
>                  preempt_count_sub(cnt) {
>                          trace_preempt_on() {
>                                  tracepoint() {
>                                          rcu_read_lock_sched() {
>                                                  // jumps into lockdep

> Where preempt_count still has softirqs disabled, but
> current->softirqs_enabled is true, and we get a splat.

> This patch should be separate (as you had it before), and needs to be
> submitted now because it already causes issues. We can add:

> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: d59158162e032 ("tracing: Add support for preempt and irq
enable/disable events")

Ok, I'll split and resubmit with your reasoning in the changelog.

Just to clarify, my changelog was based on older patches, that's why the
reason appears different but the root cause is the same. In an earlier
series, I was doing lockdep_{off,on} around the rcu_read_lock_sched call so
that stage wasn't splatting, but then after the read_lock is held, the
tracepoint probe such as those registered by preemptoff tracer  was
acquiring locks and causing the same splat.

Anyway, this just for some justification of why changelog appears to
present a different scenario with the same fix. But I'll change it to yours
and resubmit with the tags, thanks a lot for looking into it,

thanks,

- Joel