lkml.org 
[lkml]   [2017]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: 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;
\
 
 \ /
  Last update: 2017-09-01 22:33    [W:0.057 / U:0.824 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site