Messages in this thread | | | From | Joel Fernandes <> | Date | Tue, 01 May 2018 15:00:52 +0000 | Subject | Re: [PATCH RFC v5 1/6] softirq: reorder trace_softirqs_on to prevent lockdep splat |
| |
On Tue, May 1, 2018 at 7:02 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 30 Apr 2018 18:41:59 -0700 > Joel Fernandes <joelaf@google.com> 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@vger.kernel.org > 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
| |