Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1641982

Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order

Show all headers | View raw

On Fri, 12 May 2017 21:49:56 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> In general we avoid nested locking in the kernel. Nested locking makes
> an absolute mockery of locking rules and what all gets protected.

I'm not against the goal of having get_online_cpus() not be nested,
but I don't agree with the above comment.

If we look at locking in a more abstract view, lock(A) is just
something to protect critical section A. Lock(B) is just something to
protect critical section B. To prevent deadlocks, if one enters
critical section A and then enters critical section B before leaving
critical section A, then the system must never enter critical section B
and then enter critical section A.

But for nested locks, the nested lock is not a new critical section. If
we have lock(A); ...; lock(A); ...; unlock(A); ...; unlock(A); that
just shows a single entry into critical section A. Even if we have:

lock(A);...; lock(B); ...; lock(A); ...; unlock(A); ...;
unlock(B); ...; unlock(A);

As long as there never exits a lock(B); ...; lock(A); without first
taking lock A before taking lock(B).

Because the rules only matter when entering a critical section, and the
taking of the second lock(A) is basically just a nop.

We do this all the time in the kernel, but we just don't do it with
nesting locks. We usually do it with __func()s.


void func(void) {
	lock(A);
	__func();
	unlock(A);
}

and later we could even have:

void foo(void) {
	lock(A);
	...;
	lock(B);
	...;
	__func();
	...;
	unlock(B);
	...;
	unlock(A);
}

If lock(A) was able to nest, then __func() wouldn't be needed. We could
always just call func() which would take lock(A); lockdep will still
work just fine, as it would only care when the lock is first taken, not
its nesting.

One thing we need to do with the current approach is

void __func(void) {

	lockdep_assert_held(A);

	...;
}

to make sure that A is held when calling __func().

Again, I'm not against the lofty goal of having no nesting of
get_online_cpus(), but I just don't agree that nesting locks make a
mockery out of the locking rules.

-- Steve

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order Steven Rostedt <rostedt@goodmis.org> - 2017-05-12 19:30 +0200
  [RFC][PATCH 5/5] perf: Grab event_mutex before taking get_online_cpus() Steven Rostedt <rostedt@goodmis.org> - 2017-05-12 19:30 +0200
  [RFC][PATCH 3/5] kprobes: Take get_online_cpus() before taking jump_label_lock() Steven Rostedt <rostedt@goodmis.org> - 2017-05-12 19:30 +0200
    Re: [RFC][PATCH 3/5] kprobes: Take get_online_cpus() before taking  jump_label_lock() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-05-12 20:40 +0200
      Re: [RFC][PATCH 3/5] kprobes: Take get_online_cpus() before taking  jump_label_lock() Steven Rostedt <rostedt@goodmis.org> - 2017-05-12 20:50 +0200
    Re: [RFC][PATCH 3/5] kprobes: Take get_online_cpus() before taking  jump_label_lock() Masami Hiramatsu <mhiramat@kernel.org> - 2017-05-17 20:00 +0200
  Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-05-12 20:20 +0200
  Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order Peter Zijlstra <peterz@infradead.org> - 2017-05-12 22:00 +0200
    Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order Steven Rostedt <rostedt@goodmis.org> - 2017-05-12 22:20 +0200
    Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order Steven Rostedt <rostedt@goodmis.org> - 2017-05-12 23:40 +0200
      Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-05-13 15:50 +0200
        Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order Peter Zijlstra <peterz@infradead.org> - 2017-05-15 11:10 +0200
          Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-05-15 20:50 +0200
            Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order Peter Zijlstra <peterz@infradead.org> - 2017-05-16 10:20 +0200
              Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-05-16 14:50 +0200
                Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-05-16 16:30 +0200
                Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order Peter Zijlstra <peterz@infradead.org> - 2017-05-17 12:50 +0200
                Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-05-17 17:00 +0200
                Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-05-18 06:00 +0200
    Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order Steven Rostedt <rostedt@goodmis.org> - 2017-05-15 21:10 +0200

csiph-web