From patchwork Wed Jul 18 19:43:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathieu Poirier X-Patchwork-Id: 142294 Delivered-To: patch@linaro.org Received: by 2002:a2e:9754:0:0:0:0:0 with SMTP id f20-v6csp922320ljj; Wed, 18 Jul 2018 12:43:23 -0700 (PDT) X-Google-Smtp-Source: AAOMgpfptAqk+WXRsO90S21AuTBZ7f/9XefiM8e0o2Nwm45cZd1pG7E1Y9vafA+t8MQ/XzssLDpL X-Received: by 2002:a62:8559:: with SMTP id u86-v6mr6542545pfd.32.1531943003373; Wed, 18 Jul 2018 12:43:23 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1531943003; cv=none; d=google.com; s=arc-20160816; b=Kwh0xxld44IoZl20W+ReZ3c20DzdVS8KDGo7fOJrJ+XOEja5Ll3EeOYvLSFaYJEQEe tMkXJ0gsuUGzr2ttor3inagQt1z2f8Y2R7wHr1RkX5q2aArx27uFEaczdWPNcu65NPgo oFhUBkeWGJTw2xvklrC8TudEVziV9Q4vrNwLKCz7tiDOPKGgcEYUNSOX3rNwmqhJI+ew p+rRFx4yAseG5tLk7npDlNWINpmai9DzCw9o2vJ4BtSpMBZv2UF7CK7dbz9u/ZuCdkcm dwe/Qkhp0tUnSsz4E0NlFfFE4+E4qNlgCyBXdLa1/e3e28zMOMvp2ZpYXIjwEk4BPe1p 6dyA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :dkim-signature:arc-authentication-results; bh=gs8VFzCKf8BLvqem/9OK8lkc0ZJ2fY7gqczTHQelo64=; b=akOOwzbf+eCo6bsAta15yicLOLg4/Tg8iMEvzxeKh0v7Nmfic6J1nGnMj+LvATSukC t5pfPR6ylpKUl/QV/eGaYBaKLdk5LtHfJ9qbcOnIJ7j5ezCProoX9c3jtwtXALO9qphe BpyP0nx9zyQ+tIBQJ1fs3dv61T0dMneZyUVP6uUAjmqiFk7m3vFb6K6opzYpbkue0nM/ wmdytLay1xg5ktt3DGKQhFWeR5uqPYBelq706VxuwnXrPihy5Tjz2W4Q+eYGMjYapBpf cY8Z9C8mWnL8kN3CEUwQEuialNBBTVCRZQBQJVdXWvvAJCU5xWTCh0HXbUyRcR8VuyTW bfbg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=VALrZvzz; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id q126-v6si4276575pfb.277.2018.07.18.12.43.23; Wed, 18 Jul 2018 12:43:23 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=VALrZvzz; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730252AbeGRUWd (ORCPT + 31 others); Wed, 18 Jul 2018 16:22:33 -0400 Received: from mail-it0-f65.google.com ([209.85.214.65]:36192 "EHLO mail-it0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730003AbeGRUWd (ORCPT ); Wed, 18 Jul 2018 16:22:33 -0400 Received: by mail-it0-f65.google.com with SMTP id j185-v6so6028548ite.1 for ; Wed, 18 Jul 2018 12:43:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=gs8VFzCKf8BLvqem/9OK8lkc0ZJ2fY7gqczTHQelo64=; b=VALrZvzzYqEL2YMu0gvCMr72fjYKu97W7ufICWHw4QrnTzSuNQNIGcsALo+DSAW+bC HpSlzc8/5ywg3hsCvdLq8ceY+EMDwFZwIdhRRGTYHlu/QBfaN7Zb34x3SkUUwC5KXE6d snzDYbC160IKkW2qqCmT+L/V3+4SQzOT/4zmw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=gs8VFzCKf8BLvqem/9OK8lkc0ZJ2fY7gqczTHQelo64=; b=rL47hG9RqKN1WpXeTe3eZyQXpYLHv80m/DIc16RBGNwrqwKgij3hw9InTISZNmZzLL RlKWNyKGPNszZFNMtE8iOwFeEDsgs+2+BZbjBGrdeESlXlyZdQGRanPER/udlC9/PHN9 9EQvGmdUW5JTdsQdT2ju+Iwd04J4QzB0+gwn7QxetFk1nWuaFX1vXAcYvJuWDIRgM0Lu 5hzLNkdib3ND8/Iofk/HJu91nKKZgHz5uyW+DNVY9iczsplefk8z7s3JwUeMrPqyL+O3 fRFLmRFEnTBcVOdjBhuNVKuz/CIWWD0m94TWhul2sdcTWv3XGAXWH6VIgv9HKWndKNhC Yiig== X-Gm-Message-State: AOUpUlGZorQ192j6TMjrWZW21tFBmsYs1Fbf0asEx/40G4FoyINVEm5l PdV6/OpIYp5nu5AwOHk776CVZOg6xNA= X-Received: by 2002:a02:879a:: with SMTP id t26-v6mr6746270jai.67.1531942990523; Wed, 18 Jul 2018 12:43:10 -0700 (PDT) Received: from xps15.cg.shawcable.net (S0106002369de4dac.cg.shawcable.net. [68.147.8.254]) by smtp.gmail.com with ESMTPSA id b125-v6sm1845636itc.37.2018.07.18.12.43.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 18 Jul 2018 12:43:09 -0700 (PDT) From: Mathieu Poirier To: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org, alexander.shishkin@linux.intel.com, Suzuki.Poulose@arm.com Subject: [PATCH] coresight: etm_perf: Rework alloc/free methods to avoid lockep warning Date: Wed, 18 Jul 2018 13:43:08 -0600 Message-Id: <1531942988-20901-1-git-send-email-mathieu.poirier@linaro.org> X-Mailer: git-send-email 2.7.4 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org When enabling the lockdep mechanic and working with CPU-wide scenarios we get the following console output: [ 54.632093] ====================================================== [ 54.638207] WARNING: possible circular locking dependency detected [ 54.644322] 4.18.0-rc3-00042-g2d39e6356bb7-dirty #309 Not tainted [ 54.650350] ------------------------------------------------------ [ 54.656464] perf/2862 is trying to acquire lock: [ 54.661031] 000000007e21d170 (&event->mmap_mutex){+.+.}, at: perf_event_set_output+0x98/0x138 [ 54.669486] [ 54.669486] but task is already holding lock: [ 54.675256] 000000001080eb1b (&cpuctx_mutex){+.+.}, at: perf_event_ctx_lock_nested+0xf8/0x1f0 [ 54.683704] [ 54.683704] which lock already depends on the new lock. [ 54.683704] [ 54.691797] [ 54.691797] the existing dependency chain (in reverse order) is: [ 54.699201] [ 54.699201] -> #3 (&cpuctx_mutex){+.+.}: [ 54.704556] __mutex_lock+0x70/0x808 [ 54.708608] mutex_lock_nested+0x1c/0x28 [ 54.713005] perf_event_init_cpu+0x8c/0xd8 [ 54.717574] perf_event_init+0x194/0x1d4 [ 54.721971] start_kernel+0x2b8/0x42c [ 54.726107] [ 54.726107] -> #2 (pmus_lock){+.+.}: [ 54.731114] __mutex_lock+0x70/0x808 [ 54.735165] mutex_lock_nested+0x1c/0x28 [ 54.739560] perf_event_init_cpu+0x30/0xd8 [ 54.744129] cpuhp_invoke_callback+0x84/0x248 [ 54.748954] _cpu_up+0xe8/0x1c8 [ 54.752576] do_cpu_up+0xa8/0xc8 [ 54.756283] cpu_up+0x10/0x18 [ 54.759731] smp_init+0xa0/0x114 [ 54.763438] kernel_init_freeable+0x120/0x288 [ 54.768264] kernel_init+0x10/0x108 [ 54.772230] ret_from_fork+0x10/0x18 [ 54.776279] [ 54.776279] -> #1 (cpu_hotplug_lock.rw_sem){++++}: [ 54.782492] cpus_read_lock+0x34/0xb0 [ 54.786631] etm_setup_aux+0x5c/0x308 [ 54.790769] rb_alloc_aux+0x1ec/0x300 [ 54.794906] perf_mmap+0x284/0x610 [ 54.798787] mmap_region+0x388/0x570 [ 54.802838] do_mmap+0x344/0x4f8 [ 54.806544] vm_mmap_pgoff+0xe4/0x110 [ 54.810682] ksys_mmap_pgoff+0xa8/0x240 [ 54.814992] sys_mmap+0x18/0x28 [ 54.818613] el0_svc_naked+0x30/0x34 [ 54.822661] [ 54.822661] -> #0 (&event->mmap_mutex){+.+.}: [ 54.828445] lock_acquire+0x48/0x68 [ 54.832409] __mutex_lock+0x70/0x808 [ 54.836459] mutex_lock_nested+0x1c/0x28 [ 54.840855] perf_event_set_output+0x98/0x138 [ 54.845680] _perf_ioctl+0x2a0/0x6a0 [ 54.849731] perf_ioctl+0x3c/0x68 [ 54.853526] do_vfs_ioctl+0xb8/0xa20 [ 54.857577] ksys_ioctl+0x80/0xb8 [ 54.861370] sys_ioctl+0xc/0x18 [ 54.864990] el0_svc_naked+0x30/0x34 [ 54.869039] [ 54.869039] other info that might help us debug this: [ 54.869039] [ 54.876960] Chain exists of: [ 54.876960] &event->mmap_mutex --> pmus_lock --> &cpuctx_mutex [ 54.876960] [ 54.887217] Possible unsafe locking scenario: [ 54.887217] [ 54.893073] CPU0 CPU1 [ 54.897552] ---- ---- [ 54.902030] lock(&cpuctx_mutex); [ 54.905396] lock(pmus_lock); [ 54.910911] lock(&cpuctx_mutex); [ 54.916770] lock(&event->mmap_mutex); [ 54.920566] [ 54.920566] *** DEADLOCK *** [ 54.920566] [ 54.926424] 1 lock held by perf/2862: [ 54.930042] #0: 000000001080eb1b (&cpuctx_mutex){+.+.}, at: perf_event_ctx_lock_nested+0xf8/0x1f0 This is fixed by working with the cpu_present_mask, avoinding at the same the need to use get/put_online_cpus() that triggers lockdep. Signed-off-by: Mathieu Poirier --- drivers/hwtracing/coresight/coresight-etm-perf.c | 39 +++++++++++++----------- 1 file changed, 21 insertions(+), 18 deletions(-) -- 2.7.4 diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 677695635211..16b9c87d9d00 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -108,26 +108,32 @@ static int etm_event_init(struct perf_event *event) static void free_event_data(struct work_struct *work) { int cpu; + void *snk_config; cpumask_t *mask; struct etm_event_data *event_data; struct coresight_device *sink; event_data = container_of(work, struct etm_event_data, work); mask = &event_data->mask; - /* - * First deal with the sink configuration. See comment in - * etm_setup_aux() about why we take the first available path. - */ - if (event_data->snk_config) { - cpu = cpumask_first(mask); - sink = coresight_get_sink(event_data->path[cpu]); - if (sink_ops(sink)->free_buffer) - sink_ops(sink)->free_buffer(event_data->snk_config); - } + snk_config = event_data->snk_config; for_each_cpu(cpu, mask) { - if (!(IS_ERR_OR_NULL(event_data->path[cpu]))) - coresight_release_path(event_data->path[cpu]); + if (IS_ERR_OR_NULL(event_data->path[cpu])) + continue; + + /* + * Free sink configuration - there can only be one sink + * per event. + */ + if (snk_config) { + sink = coresight_get_sink(event_data->path[cpu]); + if (sink_ops(sink)->free_buffer) { + sink_ops(sink)->free_buffer(snk_config); + snk_config = NULL; + } + } + + coresight_release_path(event_data->path[cpu]); } kfree(event_data->path); @@ -145,16 +151,13 @@ static void *alloc_event_data(int cpu) if (!event_data) return NULL; - /* Make sure nothing disappears under us */ - get_online_cpus(); - size = num_online_cpus(); - mask = &event_data->mask; + size = num_present_cpus(); + if (cpu != -1) cpumask_set_cpu(cpu, mask); else - cpumask_copy(mask, cpu_online_mask); - put_online_cpus(); + cpumask_copy(mask, cpu_present_mask); /* * Each CPU has a single path between source and destination. As such