Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752013AbdHaHiB (ORCPT <rfc822;w@1wt.eu>);
        Thu, 31 Aug 2017 03:38:01 -0400
Received: from bombadil.infradead.org ([65.50.211.133]:45012 "EHLO
        bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751800AbdHaHhq (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 31 Aug 2017 03:37:46 -0400
Date: Thu, 31 Aug 2017 09:37:39 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        lkml <linux-kernel@vger.kernel.org>
Subject: Re: WARNING: possible circular locking dependency detected
Message-ID: <20170831073739.ytexc7omldyb5lgy@hirez.programming.kicks-ass.net>
References: <20170825100304.5cwrlrfwi7f3zcld@pd.tnic>
 <20170828145808.btuqpe2bvxymljyg@hirez.programming.kicks-ass.net>
 <alpine.DEB.2.20.1708291921350.2408@nanos>
 <20170829194948.GD32112@worktop.programming.kicks-ass.net>
 <alpine.DEB.2.20.1708292155360.2696@nanos>
 <20170830054730.GF32112@worktop.programming.kicks-ass.net>
 <alpine.DEB.2.20.1708310757430.1874@nanos>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.20.1708310757430.1874@nanos>
User-Agent: NeoMutt/20170609 (1.8.3)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1434
Lines: 41

On Thu, Aug 31, 2017 at 09:08:05AM +0200, Thomas Gleixner wrote:
> On Wed, 30 Aug 2017, Peter Zijlstra wrote:
> > On offline it basically does perf_event_disable() for all CPU context
> > events, and then adds HOTPLUG_OFFSET (-32) to arrive at: OFF +
> > HOTPLUG_OFFSET = -33.
> > 
> > That's smaller than ERROR and thus perf_event_enable() no-ops on events
> > for offline CPUs (maybe we should try and plumb an error return for
> > IOC_ENABLE).
> > 
> > On online we subtract the HOTPLUG_OFFSET again and the event becomes a
> > regular OFF, after which perf_event_enable() should work again.
> 
> I haven't come around to test that as I was busy cleaning up the unholy
> mess in the watchdog code.
> 
> One other thing I stumbled over is:
> 
>     perf_event_create()
>       ....
>       x86_hw_reserve(event)
> 
>       if (__x86_pmu_event_init(event) < 0)
>       	    event->destroy(event);
> 	    	x86_hw_release()
> 		    ....
> 		    cpus_read_lock();
> 
> If that happens from a hotplug function, we are doomed.
> 
> I mean, that particular watchdog event won't fail if the watchdog code
> would verify that already at init time (which it does soon), but in general
> event creation during hotplug is dangerous.

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.

I'll try and think...