From patchwork Wed Dec 13 15:13:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Nuno Sa via B4 Relay X-Patchwork-Id: 178084 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:3b04:b0:fb:cd0c:d3e with SMTP id c4csp7846012dys; Wed, 13 Dec 2023 07:14:14 -0800 (PST) X-Google-Smtp-Source: AGHT+IGbHdrBLkxHkXrAsl5Nx/P0BSNNbLkMFI0AC+vjNKPsBb2QgbkldgxrY0tV/7BI02lqfO46 X-Received: by 2002:a05:6a20:4295:b0:190:38ef:3a57 with SMTP id o21-20020a056a20429500b0019038ef3a57mr11409511pzj.28.1702480454645; Wed, 13 Dec 2023 07:14:14 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702480454; cv=none; d=google.com; s=arc-20160816; b=PSbctN0i8QojTHt0nxwhAXmbyFVAtJ4kDyqyifyYpjcxklDNmwz4hbTOnXGtsp9ppx hNhGSKCEg83in10O8nQH7n6+pjcS+Blrjc+pc2jv+ntDHVzpDutMfUzs8dG8gpA3bDRe WR6RYvMDmPwy97dnx8FeqE3KFqPGBv8NTLjkyxAupENuUQ38P2WdH2emw0ixGoWC9EZA ZVJC6dBHcAYcb6dloQtxT4UoOCx5vtoOSjC663KJV3xXYn2VzXqBO/LZynFq4jxFtaik Y0V3PeuHOCmB+Sd0qt0x/wvSz3RytCO9Ow/N9TyPHWBiu2HAbl8ggqBxYbYWwSxa+hSR VoeA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:reply-to:cc:to:message-id :content-transfer-encoding:mime-version:subject:date:from :dkim-signature; bh=698hi5Ggb3qH+zPEyneR8x5RjFflP8E9ubk0NFENxrU=; fh=CZvX4Wnljry2jinlhOU7x3JnN3amQ52zUIRNeXvXzu0=; b=jU4SE1dsLDgE2N799bicUOhIdRNIHSxlTWqAqvoj37se5WiI6ILWe0w6iYCPaO9A4h If4o9rFAHkl5wvoB2Jw4mVddg6y/wYKmZXjYB0GYQX+jx9sFMXz1ep6jUwViFrSJEStR hQ+P6qAHx2RZnFl32HClFQw+g44ptWkev054z48u5JfLC7KdUnfQBo+6gAHjQMojI1yx obJZLdzdFJmOPLpVYiUHHpDYoN+LAfzqRbhojh3kKr1gNaadJVrvhczQpyaSqEkuPXrr smMuJAOHiYneRo+SL26xVNPZ4me9hb9IOJTqRwjeU3lakymL9I1leOYo41ZX6zHsktGj KCZA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=gSEmqayp; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from pete.vger.email (pete.vger.email. [2620:137:e000::3:6]) by mx.google.com with ESMTPS id 14-20020a63104e000000b005c7024c429esi7793129pgq.488.2023.12.13.07.14.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Dec 2023 07:14:14 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=gSEmqayp; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by pete.vger.email (Postfix) with ESMTP id 9DE1A81A329F; Wed, 13 Dec 2023 07:14:10 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at pete.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1442280AbjLMPOB (ORCPT + 99 others); Wed, 13 Dec 2023 10:14:01 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60258 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1442095AbjLMPN7 (ORCPT ); Wed, 13 Dec 2023 10:13:59 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3534EE3 for ; Wed, 13 Dec 2023 07:14:06 -0800 (PST) Received: by smtp.kernel.org (Postfix) with ESMTPS id BE95BC433C7; Wed, 13 Dec 2023 15:14:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1702480445; bh=RCKOlm/rfJFGAluA0uC53gYGTmOCx5LlFHEsyolIc+I=; h=From:Date:Subject:To:Cc:Reply-To:From; b=gSEmqayp2smuoWiLQmUclE5sR26aeI0Yub9E4AkG18zY+z6p6Hhd8nSJxjIjTDxOp 4HScECyDsw5KjQFWXEpqy3nFPZ4VTG/3zN/wAoSZ9txm/CJiVMSYVKhFFLcpZVoQrQ WrsIJO/E3DFQytgwC2B2+IbtJXiakHWK0JemlNrfnC2K9lGP+3WrIEiK7OQ9U3+5Nx AMgbRr4eZkQyGHnf5h2BPZoFjWKp7GmhWq5mIdYEB9TLvwYOpW5uugVA3bIYP/77iN a/+7RRqM9I7S4joIqRK0lpPxg678QyU7Zn43z+g7NE7nQgDYqrW22gunkhvi2vSeMd WZZy21qbDipLQ== Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9F443C4332F; Wed, 13 Dec 2023 15:14:05 +0000 (UTC) From: Nuno Sa via B4 Relay Date: Wed, 13 Dec 2023 16:13:59 +0100 Subject: [PATCH RESEND RFC] driver: core: don't queue device links removal for dt overlays MIME-Version: 1.0 Message-Id: <20231213-fix-device-links-overlays-v1-1-f091b213777c@analog.com> X-B4-Tracking: v=1; b=H4sIADbKeWUC/32NMQ+CMBCF/4q52TNcAREnE8XRAUfD0NAWGrE1r WkkhP9uJc5Ol3f53vsm8NJp6WG/msDJoL22JgZar6DtuekkahEzsISlRCxDpd8oIthKHLS5e7R BuoGPHikVZaFUSRlJiP2nkxFetm9QV9fqcvq+6/MRmnh77V/WjYs50AL9JMUfSSAkFEWW7lS+F QnLD9zwwXab1j6gmef5A0ES52LSAAAA To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Frank Rowand , Rob Herring X-Mailer: b4 0.12.3 X-Developer-Signature: v=1; a=ed25519-sha256; t=1702480444; l=4326; i=nuno.sa@analog.com; s=20231116; h=from:subject:message-id; bh=8QbmA+HGKDxoC5NcwfVBHQDhv97DYzCEC3G4ZKRaqFg=; b=h7llQVIwtSkkBrHpTDv5Kmr8hPHmtfoiroWKx19gGJBYsjg71X5oGUH/+PQZ5SbkhnTrVfuBs SQwuaubmPnTDMaAvNPNwDS7BJvSQqKlhdvW9lZ1t9NfGy6SQJvA71FC X-Developer-Key: i=nuno.sa@analog.com; a=ed25519; pk=3NQwYA013OUYZsmDFBf8rmyyr5iQlxV/9H4/Df83o1E= X-Endpoint-Received: by B4 Relay for nuno.sa@analog.com/20231116 with auth_id=100 X-Original-From: Nuno Sa Reply-To: X-Spam-Status: No, score=-1.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Wed, 13 Dec 2023 07:14:11 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1785180144812412467 X-GMAIL-MSGID: 1785180144812412467 From: Nuno Sa For device links, releasing the supplier/consumer devices references happens asynchronously in device_link_release_fn(). Hence, the possible release of an of_node is also asynchronous. If these nodes were added through overlays we have a problem because this does not respect the devicetree overlays assumptions that when a changeset is being removed in __of_changeset_entry_destroy(), it must hold the last reference to that node. Due to the async nature of device links that cannot be guaranteed. Given the above, in case one of the link consumer/supplier is part of an overlay node we call directly device_link_release_fn() instead of queueing it. Yes, it might take some significant time for device_link_release_fn() to complete because of synchronize_srcu() but we would need to, anyways, wait for all OF references to be released if we want to respect overlays assumptions. Signed-off-by: Nuno Sa --- This RFC is a follow up of a previous one that I sent to the devicetree folks [1]. It got rejected because it was not really fixing the root cause of the issue (which I do agree). Please see the link where I fully explain what the issue is. I did also some git blaming and did saw that commit 80dd33cf72d1 ("drivers: base: Fix device link removal") introduced queue_work() as we could be releasing the last device reference and hence sleeping which is against SRCU callback requirements. However, that same commit is now making use of synchronize_srcu() which may take significant time (and I think that's the reason for the work item?). However, given the dt overlays requirements, I'm not seeing any reason to not be able to run device_link_release_fn() synchronously if we detect an OVERLAY node is being released. I mean, even if we come up (and I did some experiments in this regard) with some async mechanism to release the OF nodes refcounts, we still need a synchronization point somewhere. Anyways, I would like to have some feedback on how acceptable would this be or what else could I do so we can have a "clean" dt overlay removal. I'm also cc'ing dts folks so they can give some comments on the new device_node_overlay_removal() function. My goal is to try to detect when an overlay is being removed (maybe we could even have an explicit flag for it?) and only directly call device_link_release_fn() in that case. [1]: https://lore.kernel.org/linux-devicetree/20230511151047.1779841-1-nuno.sa@analog.com/ --- drivers/base/core.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) --- base-commit: 98b1cc82c4affc16f5598d4fa14b1858671b2263 change-id: 20231124-fix-device-links-overlays-13d97ff9141e -- Thanks! - Nuno Sá diff --git a/drivers/base/core.c b/drivers/base/core.c index 67ba592afc77..8466b63b89c3 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -497,6 +497,18 @@ static struct attribute *devlink_attrs[] = { }; ATTRIBUTE_GROUPS(devlink); +static bool device_node_overlay_removal(struct device *dev) +{ + if (!dev_of_node(dev)) + return false; + if (!of_node_check_flag(dev->of_node, OF_DETACHED)) + return false; + if (!of_node_check_flag(dev->of_node, OF_OVERLAY)) + return false; + + return true; +} + static void device_link_release_fn(struct work_struct *work) { struct device_link *link = container_of(work, struct device_link, rm_work); @@ -532,8 +544,19 @@ static void devlink_dev_release(struct device *dev) * synchronization in device_link_release_fn() and if the consumer or * supplier devices get deleted when it runs, so put it into the "long" * workqueue. + * + * However, if any of the supplier, consumer nodes is being removed + * through overlay removal, the expectation in + * __of_changeset_entry_destroy() is for the node 'kref' to be 1 which + * cannot be guaranteed with the async nature of + * device_link_release_fn(). Hence, do it synchronously for the overlay + * case. */ - queue_work(system_long_wq, &link->rm_work); + if (device_node_overlay_removal(link->consumer) || + device_node_overlay_removal(link->supplier)) + device_link_release_fn(&link->rm_work); + else + queue_work(system_long_wq, &link->rm_work); } static struct class devlink_class = {