Messages in this thread Patch in this message | | | Date | Fri, 1 Sep 2017 22:32:50 +0200 | From | Peter Zijlstra <> | Subject | Re: WARNING: possible circular locking dependency detected |
| |
On Thu, Aug 31, 2017 at 11:24:13PM +0200, Thomas Gleixner wrote: > On Thu, 31 Aug 2017, Thomas Gleixner wrote: > > On Thu, 31 Aug 2017, Peter Zijlstra wrote: > > > > > On Thu, Aug 31, 2017 at 09:55:57AM +0200, Thomas Gleixner wrote: > > > > > Arghh!!! > > > > > > > > > > And allowing us to create events for offline CPUs (possible I think, but > > > > > maybe slightly tricky) won't solve that, because we're already holding > > > > > the hotplug_lock during PREPARE. > > > > > > > > There are two ways to cure that: > > > > > > > > 1) Have a pre cpus_write_lock() stage which is serialized via > > > > cpus_add_remove_lock, which is the outer lock for hotplug. > > > > > > > > There we can sanely create stuff and fail with all consequences. > > > > > > True, if you're willing to add more state to that hotplug thing I'll try > > > and make that perf patch that allows attaching to offline CPUs. > > > > Now that I think more about it. That's going to be an interesting exercise > > vs. the hotplug state registration which relies on cpus_read_lock() > > serialization..... > > We could have that for built-in stuff which is guaranteed to be never > unregistered. Pretty restricted, but for cases like that it could > work. Famous last work ...
I think something like this (on top of the previous patch that preserves the event<->cpu relation over hotplug) should allow perf_event_create_kernel_counter() to create events on offline CPUs.
It will create them as if perf_event_attr::disabled=1 and requires perf_event_enable() after the CPU comes online (mirroring how offline does an implicit perf_event_disable() as per the previous patch).
--- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -2427,6 +2427,24 @@ perf_install_in_context(struct perf_even smp_store_release(&event->ctx, ctx); if (!task) { + /* + * Check if the @cpu we're creating an event for is offline. + * + * We use the perf_cpu_context::ctx::mutex to serialize against + * the hotplug notifiers. See perf_event_{init,exit}_cpu(). + */ + struct perf_cpu_context *cpuctx = + container_of(ctx, struct perf_cpu_context, ctx); + + if (!cpuctx->online) { + raw_spin_lock_irq(&ctx->lock); + add_event_to_context(event, ctx); + event->state = PERF_EVENT_STATE_OFF + + PERF_EVENT_STATE_HOTPLUG_OFFSET; + raw_spin_unlock_irq(&ctx->lock); + return; + } + cpu_function_call(cpu, __perf_install_in_context, event); return; } @@ -10181,6 +10199,8 @@ SYSCALL_DEFINE5(perf_event_open, * * We use the perf_cpu_context::ctx::mutex to serialize against * the hotplug notifiers. See perf_event_{init,exit}_cpu(). + * + * XXX not strictly required, preserves existing behaviour. */ struct perf_cpu_context *cpuctx = container_of(ctx, struct perf_cpu_context, ctx); @@ -10368,21 +10388,6 @@ perf_event_create_kernel_counter(struct goto err_unlock; } - if (!task) { - /* - * Check if the @cpu we're creating an event for is online. - * - * We use the perf_cpu_context::ctx::mutex to serialize against - * the hotplug notifiers. See perf_event_{init,exit}_cpu(). - */ - struct perf_cpu_context *cpuctx = - container_of(ctx, struct perf_cpu_context, ctx); - if (!cpuctx->online) { - err = -ENODEV; - goto err_unlock; - } - } - if (!exclusive_event_installable(event, ctx)) { err = -EBUSY; goto err_unlock;
| |