From patchwork Tue Apr 25 14:35:27 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Clark X-Patchwork-Id: 13223414 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 90142C77B7C for ; Tue, 25 Apr 2023 14:37:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-Id:Date:Subject:Cc :To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=+cVC4pdt4dOpMLZglo37agivaSLRRdl9nBbCSN8QCQs=; b=pnegPnKydMEyt3 QDnib52uanV+Uz7eWJwNkn81dz9A2LugnSrbtsJkyBewDRT/RnFlXttwrLTIW/SU5hCxqxQEBCaWU XuexoNk42YZxq7Bs2Pj70NF6DQyPNrNwiZe8kZPLBFyF6oEIKtoH7N1c5AWKRGsAo+Ii6iCkO4xpj wpPualtHnyRTp2n9quUmq1Za9D5kz10aEOyrHddFkPBlGeH7O0RFQMF6fnutWoedeSy1SOh9QSL3D PNxwpDiULEC+0n6fGnF8qiYymXmVhC2JlI3+1m3I2s4Wc5Bt+5iz5mM+5kiSeYaTTr+YZ3ke99HV0 /j+Vo9yBtRrCFKNECBRg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1prJmI-001P2W-30; Tue, 25 Apr 2023 14:36:22 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1prJmF-001P0q-0e for linux-arm-kernel@lists.infradead.org; Tue, 25 Apr 2023 14:36:21 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B59C74B3; Tue, 25 Apr 2023 07:36:57 -0700 (PDT) Received: from localhost.localdomain (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 2CF693F587; Tue, 25 Apr 2023 07:36:12 -0700 (PDT) From: James Clark To: coresight@lists.linaro.org, quic_jinlmao@quicinc.com, mike.leach@linaro.org, suzuki.poulose@arm.com Cc: James Clark , Leo Yan , Alexander Shishkin , Mathieu Poirier , Greg Kroah-Hartman , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH v6 00/13] coresight: Fix CTI module refcount leak by making it a helper device Date: Tue, 25 Apr 2023 15:35:27 +0100 Message-Id: <20230425143542.2305069-1-james.clark@arm.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230425_073619_331116_0AE2A5E8 X-CRM114-Status: GOOD ( 17.92 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Changes since v5: * Formatting fixes * helper->type != CORESIGHT_DEV_TYPE_HELPER -> !coresight_is_helper() * Remove ect from coresight_dev_type and add a static assert so that it stays in sync with the enum ------------------ Changes since v4: * Update commit message on patch 1 * Drop previous change that added lockdep checks to coresight_add_helper() because they are already in mutex_lock(). But keep extra comment. * Check for allocation failure in coresight_add_out_conn() ------------------ Changes since v3: * Put connection loss fix at the beginning so that it can be backported * Replace coresight_find_link_{x}() with coresight_find_out_connection() * Reorder coresight_enable_source() arguments for consistency * Add source and destination reference counts so that two link devices connected together don't clash * Add coresight_is_helper() * Fix overwriting csdev bug in coresight_orphan_match() * Don't clear conns[i]->dest_fwnode in coresight_remove_conns() in case it's used again * Use dev instead of adev->dev for devmem allocation in acpi_coresight_parse_graph() so that it's consistent with DT mode and doesn't cause a warning on free. * Rename coresight_add_helper_mutex() -> coresight_add_helper() * Ensure coresight_mutex isn't already held in coresight_add_helper() * Return new connection from coresight_add_out_conn() * Comment and formatting improvements ------------------ Changes since v2: * Make out_conns array contiguous instead of sparse which simplifies filling and using it. New connections are always added to the end * Store pointers to individual connection objects so that they can be shared between inputs and outputs * Fix an existing bug where connection info was lost when unloading a device * Simplify connection fixup functions. Now the orphan mechanism is used for inputs in the same way as outputs to guarantee that all connections have both an input and an output set * Use input connections to disconnect devices on unload instead of iterating through them all * Make refcount a property of the connection rather than use it's own array based on the number of inputs and outputs * Fix a bug in v2 where helpers attached to the source device weren't disabled because coresight-etm-perf.c was making a raw call to disable rather than using a helper. * Change names of connection members to make direction explicit now that the connection is shared between input and outputs ------------------ Changes since v1: * Don't dereference handle in tmc_etr_get_buffer() when not in perf mode. * Fix some W=1 warnings * Add a commit to rename child/output in terms of local/remote ------------------- Currently there is a refcount leak in CTI when using system wide mode or tracing multithreaded applications. See the last commit for a reproducer. This prevents the module from being unloaded. Historically there have been a few issues and fixes attempted around here which have resulted in some extra logic and a member to keep track of CTI being enabled 'struct coresight_device->ect_enabled'. The fix in commit 665c157e0204 ("coresight: cti: Fix hang in cti_disable_hw()") was also related to CTI having its own enable/disable path which came later than other devices. If we make CTI a helper device and enable helper devices adjacent to the path we get very similar enable/disable behavior to now, but with more reuse of the existing reference counting logic in the coresight core code. This also affects CATU which can have a little bit of its hard coded enable/disable code removed. Enabling CATU on the generic path does require that input connections are tracked so that it can get its associated ETR buffer. Applies to coresight/next (18996a113f256) but everything except the first fixes commit requires the realloc_array patch here [1]. Also available in full here [2]. [1]: https://lore.kernel.org/linux-arm-kernel/20230320145710.1120469-1-james.clark@arm.com/ [2]: https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-cs-cti-module-refcount-fix-v6 James Clark (13): coresight: Fix loss of connection info when a module is unloaded coresight: Use enum type for cs_mode wherever possible coresight: Change name of pdata->conns coresight: Rename nr_outports to nr_outconns coresight: Rename connection members to make the direction explicit coresight: Dynamically add connections coresight: Store pointers to connections rather than an array of them coresight: Simplify connection fixup mechanism coresight: Store in-connections as well as out-connections coresight: Make refcount a property of the connection coresight: Refactor out buffer allocation function for ETR coresight: Enable and disable helper devices adjacent to the path coresight: Fix CTI module refcount leak by making it a helper device drivers/hwtracing/coresight/coresight-catu.c | 21 +- drivers/hwtracing/coresight/coresight-core.c | 605 +++++++++--------- .../hwtracing/coresight/coresight-cti-core.c | 52 +- .../hwtracing/coresight/coresight-cti-sysfs.c | 4 +- drivers/hwtracing/coresight/coresight-cti.h | 4 +- drivers/hwtracing/coresight/coresight-etb10.c | 13 +- .../hwtracing/coresight/coresight-etm-perf.c | 4 +- .../coresight/coresight-etm3x-core.c | 6 +- .../coresight/coresight-etm4x-core.c | 6 +- .../hwtracing/coresight/coresight-funnel.c | 26 +- .../hwtracing/coresight/coresight-platform.c | 269 +++----- drivers/hwtracing/coresight/coresight-priv.h | 17 +- .../coresight/coresight-replicator.c | 23 +- drivers/hwtracing/coresight/coresight-stm.c | 6 +- drivers/hwtracing/coresight/coresight-sysfs.c | 17 +- .../hwtracing/coresight/coresight-tmc-etf.c | 26 +- .../hwtracing/coresight/coresight-tmc-etr.c | 110 ++-- drivers/hwtracing/coresight/coresight-tmc.h | 2 + drivers/hwtracing/coresight/coresight-tpda.c | 23 +- drivers/hwtracing/coresight/coresight-tpdm.c | 4 +- drivers/hwtracing/coresight/coresight-tpiu.c | 7 +- drivers/hwtracing/coresight/coresight-trbe.c | 3 +- drivers/hwtracing/coresight/ultrasoc-smb.c | 11 +- drivers/hwtracing/coresight/ultrasoc-smb.h | 2 +- include/linux/coresight.h | 126 ++-- 25 files changed, 714 insertions(+), 673 deletions(-)