From patchwork Mon Nov 4 18:12:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathieu Poirier X-Patchwork-Id: 178462 Delivered-To: patch@linaro.org Received: by 2002:ac9:3c86:0:0:0:0:0 with SMTP id w6csp4389435ocf; Mon, 4 Nov 2019 10:13:26 -0800 (PST) X-Google-Smtp-Source: APXvYqz1uOBh+/M+wAoFdrGkluzvhHRVZzxIjot8A7B0zN6jd3487s7mqWbSp22jrL6Yg1ZHxOCs X-Received: by 2002:a50:ac3c:: with SMTP id v57mr30365002edc.300.1572891206587; Mon, 04 Nov 2019 10:13:26 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1572891206; cv=none; d=google.com; s=arc-20160816; b=x9HifsXke46vjOXybIbdmyYxNOWDy/UUgNiXgbLgZnVazu93BxU5zhrlJd8wylDtwC A/3irCMZOAiBQznBUvA0tlUUo1GsrVRYSoLiOR7J3iq7vAKkYEjYJ9EnsSXUZOsraUyh vKZu8X3DCgKt9WLWjGXYOH4xFAL8V66uRl2/HejTX/yNnnYDqQAadVQx7ip5vaDlyBc4 mqeHBNitzmD4uGiKZaGWTj0cGpQ8cxJdYe3Lmo/iB2XK9hzalPH8xPr+Ww2KsK3xOVkS RAgGgWEUBpyFJ74vwVeQMVZ32QSXB+257VccsxzGA6J4Gnc40fuh6NjEougRJjNnJOQi uFqQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:references:in-reply-to:message-id:date :subject:cc:to:from:dkim-signature; bh=zNSLQ4yVOGXzKEZuz3+X88N/a9oM0tmHL+W7cfWAqG4=; b=Q0f9z+aylPMOfWpgxP1e5Pi8MIzK5/nK4Ph+Eabx87knvqjEMmLzUb9Sy9Ef+62aST u27rKKPHMIcL10PKnvimczuFleSCJ5e7y60ZeqXnQ3bOA5KTzhXAThWk4XLtlM0s+HAW rRXt334QhgpZvqb18Z9Y0BEuPsZ5sFJPhJYJ4muTegZ5SNPjUWxP3n3fO/N9eger6/qU tRarPhg8tECwv77K9g4oLZqKItuZGk4jLU+uZE60BbZP535Cry/Q6+z1G7WC5x+lAzSY gxmUnpEuzV6qX6lRcW4ChkP4l/DXytaai6m2FoG1s0iv3ZDgMjU+q18f9A5FIlISskrP wpag== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=U61y0vCE; 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 o14si1259472ejr.14.2019.11.04.10.13.26; Mon, 04 Nov 2019 10:13:26 -0800 (PST) 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=U61y0vCE; 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 S1729657AbfKDSNW (ORCPT + 26 others); Mon, 4 Nov 2019 13:13:22 -0500 Received: from mail-pg1-f195.google.com ([209.85.215.195]:34668 "EHLO mail-pg1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729617AbfKDSNK (ORCPT ); Mon, 4 Nov 2019 13:13:10 -0500 Received: by mail-pg1-f195.google.com with SMTP id e4so11881345pgs.1 for ; Mon, 04 Nov 2019 10:13:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=zNSLQ4yVOGXzKEZuz3+X88N/a9oM0tmHL+W7cfWAqG4=; b=U61y0vCEaJMrncMMWKTp6alsvzkXjC9SFPXjSpuTDW5wP2lJ5MWsrgFL0edDG6iB8A 16U73gfEg4PN6OKZfBs2IYB12wQk9ICH/0R/6rLVJhTvUE1wLuQUwtzL0htIZrbkCD/n qKbYEmA5lAbSze1N5pEKHVX2FPwCN+nKZfWN3ZMCWHjt5my8w9wWsjiXWnqoZV0hWCrB +fgtm569leiYQPgNkZTaA81ov8RlBc4jSc82Pr3q71AjyuaOMYlwS4SNEgQxIMHvqIMC 5jncbym3qbVGrIsHRj53O1kEzrwC4S2jnN1icF/c1wyvIKRTKeuoM09MJv+wI8zHWdEi leqA== 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:in-reply-to :references; bh=zNSLQ4yVOGXzKEZuz3+X88N/a9oM0tmHL+W7cfWAqG4=; b=gzPHuxCwCvVUfPZ/dxSaBWGgTLqoQGDkvNlXv0g9R7vGRrJbIdRrFR9zcKO7JQTHTu lRE2lii+MeklZNK2FxukXVMRi0VLNPnJ2JJh7YgsAthm35/vMhfq+sHxOaG1+rUPKccj EoV5gqJmJLG/uhfY0WPwlG8uHnhVSHFRiTyCacbg0TSazraOjGBxvt5ZX6LKoOTi836u PagHNmcxeEiJox2o06dlzs6Hi+m0/Q58qXE2BKPacy8YT4PvDqujdV6pMqYlxGrT4p5v DzGO7BaFSHdjlojv/Ioc3XgurCAKrPQbqEYYTadSueXhbi8d3zC0m0zK+4Gu226um1oQ IiLQ== X-Gm-Message-State: APjAAAX27BGK1LHm1hajx3U6xk66T3d1sQlkEZvY9v4zmGRFUe2HAmun itCOWJJ5U2xSxqPm8dqKciP5Tw== X-Received: by 2002:a17:90a:fc8d:: with SMTP id ci13mr610341pjb.6.1572891186945; Mon, 04 Nov 2019 10:13:06 -0800 (PST) Received: from xps15.cg.shawcable.net (S0106002369de4dac.cg.shawcable.net. [68.147.8.254]) by smtp.gmail.com with ESMTPSA id o12sm16149520pgl.86.2019.11.04.10.13.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Nov 2019 10:13:06 -0800 (PST) From: Mathieu Poirier To: gregkh@linuxfoundation.org Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH 13/14] coresight: Serialize enabling/disabling a link device. Date: Mon, 4 Nov 2019 11:12:50 -0700 Message-Id: <20191104181251.26732-14-mathieu.poirier@linaro.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20191104181251.26732-1-mathieu.poirier@linaro.org> References: <20191104181251.26732-1-mathieu.poirier@linaro.org> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Yabin Cui When tracing etm data of multiple threads on multiple cpus through perf interface, some link devices are shared between paths of different cpus. It creates race conditions when different cpus wants to enable/disable the same link device at the same time. Example 1: Two cpus want to enable different ports of a coresight funnel, thus calling the funnel enable operation at the same time. But the funnel enable operation isn't reentrantable. Example 2: For an enabled coresight dynamic replicator with refcnt=1, one cpu wants to disable it, while another cpu wants to enable it. Ideally we still have an enabled replicator with refcnt=1 at the end. But in reality the result is uncertain. Since coresight devices claim themselves when enabled for self-hosted usage, the race conditions above usually make the link devices not usable after many cycles. To fix the race conditions, this patch uses spinlocks to serialize enabling/disabling link devices. Fixes: a06ae8609b3d ("coresight: add CoreSight core layer framework") Signed-off-by: Yabin Cui Signed-off-by: Mathieu Poirier Cc: stable # 5.3 --- .../hwtracing/coresight/coresight-funnel.c | 36 +++++++++++---- .../coresight/coresight-replicator.c | 35 ++++++++++++--- .../hwtracing/coresight/coresight-tmc-etf.c | 26 ++++++++--- drivers/hwtracing/coresight/coresight.c | 45 ++++++------------- 4 files changed, 90 insertions(+), 52 deletions(-) -- 2.17.1 diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index 05f7896c3a01..b605889b507a 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -38,12 +38,14 @@ DEFINE_CORESIGHT_DEVLIST(funnel_devs, "funnel"); * @atclk: optional clock for the core parts of the funnel. * @csdev: component vitals needed by the framework. * @priority: port selection order. + * @spinlock: serialize enable/disable operations. */ struct funnel_drvdata { void __iomem *base; struct clk *atclk; struct coresight_device *csdev; unsigned long priority; + spinlock_t spinlock; }; static int dynamic_funnel_enable_hw(struct funnel_drvdata *drvdata, int port) @@ -76,11 +78,21 @@ static int funnel_enable(struct coresight_device *csdev, int inport, { int rc = 0; struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - - if (drvdata->base) - rc = dynamic_funnel_enable_hw(drvdata, inport); - + unsigned long flags; + bool first_enable = false; + + spin_lock_irqsave(&drvdata->spinlock, flags); + if (atomic_read(&csdev->refcnt[inport]) == 0) { + if (drvdata->base) + rc = dynamic_funnel_enable_hw(drvdata, inport); + if (!rc) + first_enable = true; + } if (!rc) + atomic_inc(&csdev->refcnt[inport]); + spin_unlock_irqrestore(&drvdata->spinlock, flags); + + if (first_enable) dev_dbg(&csdev->dev, "FUNNEL inport %d enabled\n", inport); return rc; } @@ -107,11 +119,19 @@ static void funnel_disable(struct coresight_device *csdev, int inport, int outport) { struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + unsigned long flags; + bool last_disable = false; + + spin_lock_irqsave(&drvdata->spinlock, flags); + if (atomic_dec_return(&csdev->refcnt[inport]) == 0) { + if (drvdata->base) + dynamic_funnel_disable_hw(drvdata, inport); + last_disable = true; + } + spin_unlock_irqrestore(&drvdata->spinlock, flags); - if (drvdata->base) - dynamic_funnel_disable_hw(drvdata, inport); - - dev_dbg(&csdev->dev, "FUNNEL inport %d disabled\n", inport); + if (last_disable) + dev_dbg(&csdev->dev, "FUNNEL inport %d disabled\n", inport); } static const struct coresight_ops_link funnel_link_ops = { diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index b29ba640eb25..43304196a1a6 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -31,11 +31,13 @@ DEFINE_CORESIGHT_DEVLIST(replicator_devs, "replicator"); * whether this one is programmable or not. * @atclk: optional clock for the core parts of the replicator. * @csdev: component vitals needed by the framework + * @spinlock: serialize enable/disable operations. */ struct replicator_drvdata { void __iomem *base; struct clk *atclk; struct coresight_device *csdev; + spinlock_t spinlock; }; static void dynamic_replicator_reset(struct replicator_drvdata *drvdata) @@ -97,10 +99,22 @@ static int replicator_enable(struct coresight_device *csdev, int inport, { int rc = 0; struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - - if (drvdata->base) - rc = dynamic_replicator_enable(drvdata, inport, outport); + unsigned long flags; + bool first_enable = false; + + spin_lock_irqsave(&drvdata->spinlock, flags); + if (atomic_read(&csdev->refcnt[outport]) == 0) { + if (drvdata->base) + rc = dynamic_replicator_enable(drvdata, inport, + outport); + if (!rc) + first_enable = true; + } if (!rc) + atomic_inc(&csdev->refcnt[outport]); + spin_unlock_irqrestore(&drvdata->spinlock, flags); + + if (first_enable) dev_dbg(&csdev->dev, "REPLICATOR enabled\n"); return rc; } @@ -137,10 +151,19 @@ static void replicator_disable(struct coresight_device *csdev, int inport, int outport) { struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + unsigned long flags; + bool last_disable = false; + + spin_lock_irqsave(&drvdata->spinlock, flags); + if (atomic_dec_return(&csdev->refcnt[outport]) == 0) { + if (drvdata->base) + dynamic_replicator_disable(drvdata, inport, outport); + last_disable = true; + } + spin_unlock_irqrestore(&drvdata->spinlock, flags); - if (drvdata->base) - dynamic_replicator_disable(drvdata, inport, outport); - dev_dbg(&csdev->dev, "REPLICATOR disabled\n"); + if (last_disable) + dev_dbg(&csdev->dev, "REPLICATOR disabled\n"); } static const struct coresight_ops_link replicator_link_ops = { diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 807416b75ecc..d0cc3985b72a 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -334,9 +334,10 @@ static int tmc_disable_etf_sink(struct coresight_device *csdev) static int tmc_enable_etf_link(struct coresight_device *csdev, int inport, int outport) { - int ret; + int ret = 0; unsigned long flags; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + bool first_enable = false; spin_lock_irqsave(&drvdata->spinlock, flags); if (drvdata->reading) { @@ -344,12 +345,18 @@ static int tmc_enable_etf_link(struct coresight_device *csdev, return -EBUSY; } - ret = tmc_etf_enable_hw(drvdata); + if (atomic_read(&csdev->refcnt[0]) == 0) { + ret = tmc_etf_enable_hw(drvdata); + if (!ret) { + drvdata->mode = CS_MODE_SYSFS; + first_enable = true; + } + } if (!ret) - drvdata->mode = CS_MODE_SYSFS; + atomic_inc(&csdev->refcnt[0]); spin_unlock_irqrestore(&drvdata->spinlock, flags); - if (!ret) + if (first_enable) dev_dbg(&csdev->dev, "TMC-ETF enabled\n"); return ret; } @@ -359,6 +366,7 @@ static void tmc_disable_etf_link(struct coresight_device *csdev, { unsigned long flags; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + bool last_disable = false; spin_lock_irqsave(&drvdata->spinlock, flags); if (drvdata->reading) { @@ -366,11 +374,15 @@ static void tmc_disable_etf_link(struct coresight_device *csdev, return; } - tmc_etf_disable_hw(drvdata); - drvdata->mode = CS_MODE_DISABLED; + if (atomic_dec_return(&csdev->refcnt[0]) == 0) { + tmc_etf_disable_hw(drvdata); + drvdata->mode = CS_MODE_DISABLED; + last_disable = true; + } spin_unlock_irqrestore(&drvdata->spinlock, flags); - dev_dbg(&csdev->dev, "TMC-ETF disabled\n"); + if (last_disable) + dev_dbg(&csdev->dev, "TMC-ETF disabled\n"); } static void *tmc_alloc_etf_buffer(struct coresight_device *csdev, diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index e6ca899fea4e..ef20f74c85fa 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -253,9 +253,9 @@ static int coresight_enable_link(struct coresight_device *csdev, struct coresight_device *parent, struct coresight_device *child) { - int ret; + int ret = 0; int link_subtype; - int refport, inport, outport; + int inport, outport; if (!parent || !child) return -EINVAL; @@ -264,29 +264,17 @@ static int coresight_enable_link(struct coresight_device *csdev, outport = coresight_find_link_outport(csdev, child); link_subtype = csdev->subtype.link_subtype; - if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG) - refport = inport; - else if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_SPLIT) - refport = outport; - else - refport = 0; - - if (refport < 0) - return refport; + if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && inport < 0) + return inport; + if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_SPLIT && outport < 0) + return outport; - if (atomic_inc_return(&csdev->refcnt[refport]) == 1) { - if (link_ops(csdev)->enable) { - ret = link_ops(csdev)->enable(csdev, inport, outport); - if (ret) { - atomic_dec(&csdev->refcnt[refport]); - return ret; - } - } - } - - csdev->enable = true; + if (link_ops(csdev)->enable) + ret = link_ops(csdev)->enable(csdev, inport, outport); + if (!ret) + csdev->enable = true; - return 0; + return ret; } static void coresight_disable_link(struct coresight_device *csdev, @@ -295,7 +283,7 @@ static void coresight_disable_link(struct coresight_device *csdev, { int i, nr_conns; int link_subtype; - int refport, inport, outport; + int inport, outport; if (!parent || !child) return; @@ -305,20 +293,15 @@ static void coresight_disable_link(struct coresight_device *csdev, link_subtype = csdev->subtype.link_subtype; if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG) { - refport = inport; nr_conns = csdev->pdata->nr_inport; } else if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_SPLIT) { - refport = outport; nr_conns = csdev->pdata->nr_outport; } else { - refport = 0; nr_conns = 1; } - if (atomic_dec_return(&csdev->refcnt[refport]) == 0) { - if (link_ops(csdev)->disable) - link_ops(csdev)->disable(csdev, inport, outport); - } + if (link_ops(csdev)->disable) + link_ops(csdev)->disable(csdev, inport, outport); for (i = 0; i < nr_conns; i++) if (atomic_read(&csdev->refcnt[i]) != 0)