From patchwork Fri Aug 4 22:41:51 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rob Herring \(Arm\)" X-Patchwork-Id: 710350 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E916DC04E69 for ; Fri, 4 Aug 2023 22:42:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230058AbjHDWmT (ORCPT ); Fri, 4 Aug 2023 18:42:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44222 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230063AbjHDWmS (ORCPT ); Fri, 4 Aug 2023 18:42:18 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8F0F74EE1; Fri, 4 Aug 2023 15:42:17 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 263296215B; Fri, 4 Aug 2023 22:42:17 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E057AC433C7; Fri, 4 Aug 2023 22:42:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1691188936; bh=B054y3pqR16hOaTgxhtMZYYzfM1FPP+fTzP45gff++8=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=NcvEkl0jGpzfdsz6rc52Cj9eN+IhnnRBs69A4mGOw3PBCoGc1UbxBGAqxfr4xUD0r 8oe6/RGXNiBFiE+JC3PbumsPgO/eJAgvRET0trwoBICaSzpJNwQzFp+2Mja5ckJGCB +cwv9Og101zLRjajmXBHK8DmIYv13BbgygMPjyLIPFp7ThjLKEb6LP104Z152E3qt9 8bpgjgXPgoIwvBuFidAawZZIlv6vdnFrvU/caxg9Ifp7k2z12RjXSufXOpXXzQt/N+ b79e76OHavtRGucoOa4gv2EAZKhKBK9srKcS2CmcCzfp3Gyo5cjXORuq6Sn0VhoAz3 Z7kElAKBZy9Og== Received: (nullmailer pid 2346975 invoked by uid 1000); Fri, 04 Aug 2023 22:42:09 -0000 From: Rob Herring Date: Fri, 04 Aug 2023 16:41:51 -0600 Subject: [PATCH v2 1/6] of: unittest: Fix EXPECT for parse_phandle_with_args_map() test MIME-Version: 1.0 Message-Id: <20230801-dt-changeset-fixes-v2-1-c2b701579dee@kernel.org> References: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> In-Reply-To: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> To: Frank Rowand , "Enrico Weigelt, metux IT consult" , "Rafael J. Wysocki" , Sakari Ailus , Petr Mladek , Andy Shevchenko , Geert Uytterhoeven Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org X-Mailer: b4 0.13-dev Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Commit 12e17243d8a1 ("of: base: improve error msg in of_phandle_iterator_next()") added printing of the phandle value on error, but failed to update the unittest. Fixes: 12e17243d8a1 ("of: base: improve error msg in of_phandle_iterator_next()") Signed-off-by: Rob Herring --- drivers/of/unittest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index e5b0eea8011c..d943bf87c94d 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -664,12 +664,12 @@ static void __init of_unittest_parse_phandle_with_args_map(void) memset(&args, 0, sizeof(args)); EXPECT_BEGIN(KERN_INFO, - "OF: /testcase-data/phandle-tests/consumer-b: could not find phandle"); + "OF: /testcase-data/phandle-tests/consumer-b: could not find phandle 12345678"); rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-phandle", "phandle", 0, &args); EXPECT_END(KERN_INFO, - "OF: /testcase-data/phandle-tests/consumer-b: could not find phandle"); + "OF: /testcase-data/phandle-tests/consumer-b: could not find phandle 12345678"); unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); From patchwork Fri Aug 4 22:41:52 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rob Herring \(Arm\)" X-Patchwork-Id: 710689 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C8D43C04A6A for ; Fri, 4 Aug 2023 22:42:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229693AbjHDWmT (ORCPT ); Fri, 4 Aug 2023 18:42:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44202 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230041AbjHDWmQ (ORCPT ); Fri, 4 Aug 2023 18:42:16 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F240246B3; Fri, 4 Aug 2023 15:42:15 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 702066215E; Fri, 4 Aug 2023 22:42:15 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 23B32C433C7; Fri, 4 Aug 2023 22:42:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1691188934; bh=J7O07IUOpjRY/io4/uOgLa6WjHDFcDRrzq7DlljLaM8=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=rEXhsZ8CpbWlBk1Y+51kSPKl1125KzQA0JfelmLnQpCvkXJ2YvofyzkQOXDbFOQSc Q5+Vhiajid8eqfrBBwGnhfRcLmlb1cJF9YdBYxuAmeI6iwBX8iyMn98j1j3cJsgy/1 v8GTCHMmQh0woqCMEZkrsiLW29nrzxMOGzA0hxYtuFn1SjsMKXoeaz+53/knX2/ExT jN5TArDyxMyDMStrKNf7ESiBNHJOj+io47y0/MgiqwLG8tankN6D8Gd+camNyI4lZC tQOswJrU6jpLTmwuRF7dfgm2Ygg56YsYTdQcLyhWiuSmx9nEZEAVQfrC41BsMmPq3t fA7SgCXsEsrRA== Received: (nullmailer pid 2346977 invoked by uid 1000); Fri, 04 Aug 2023 22:42:09 -0000 From: Rob Herring Date: Fri, 04 Aug 2023 16:41:52 -0600 Subject: [PATCH v2 2/6] of: dynamic: Refactor action prints to not use "%pOF" inside devtree_lock MIME-Version: 1.0 Message-Id: <20230801-dt-changeset-fixes-v2-2-c2b701579dee@kernel.org> References: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> In-Reply-To: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> To: Frank Rowand , "Enrico Weigelt, metux IT consult" , "Rafael J. Wysocki" , Sakari Ailus , Petr Mladek , Andy Shevchenko , Geert Uytterhoeven Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org X-Mailer: b4 0.13-dev Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org While originally it was fine to format strings using "%pOF" while holding devtree_lock, this now causes a deadlock. Lockdep reports: of_get_parent from of_fwnode_get_parent+0x18/0x24 ^^^^^^^^^^^^^ of_fwnode_get_parent from fwnode_count_parents+0xc/0x28 fwnode_count_parents from fwnode_full_name_string+0x18/0xac fwnode_full_name_string from device_node_string+0x1a0/0x404 device_node_string from pointer+0x3c0/0x534 pointer from vsnprintf+0x248/0x36c vsnprintf from vprintk_store+0x130/0x3b4 Fix this by moving the printing in __of_changeset_entry_apply() outside the lock. As the only difference in the the multiple prints is the action name, use the existing "action_names" to refactor the prints into a single print. Fixes: a92eb7621b9fb2c2 ("lib/vsprintf: Make use of fwnode API to obtain node names and separators") Reported-by: Geert Uytterhoeven Signed-off-by: Rob Herring --- v5 (v2 in this series): - Move majority of refactoring to separate patch and minimize the fix to just moving the print out of the locked section. v4: - Add missing 'static' reported by 0-day v3: - Re-implement Geert's fix to move the prints rather than move the spinlock v1 and v2 from Geert simply moved the devtree_lock into each case statement: https://lore.kernel.org/all/c593d8389352c574b5be69d4ca4810da13326a50.1690533838.git.geert+renesas@glider.be/ --- drivers/of/dynamic.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index e311d406b170..2f0eb0053773 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -63,15 +63,13 @@ int of_reconfig_notifier_unregister(struct notifier_block *nb) } EXPORT_SYMBOL_GPL(of_reconfig_notifier_unregister); -#ifdef DEBUG -const char *action_names[] = { +static const char *action_names[] = { [OF_RECONFIG_ATTACH_NODE] = "ATTACH_NODE", [OF_RECONFIG_DETACH_NODE] = "DETACH_NODE", [OF_RECONFIG_ADD_PROPERTY] = "ADD_PROPERTY", [OF_RECONFIG_REMOVE_PROPERTY] = "REMOVE_PROPERTY", [OF_RECONFIG_UPDATE_PROPERTY] = "UPDATE_PROPERTY", }; -#endif int of_reconfig_notify(unsigned long action, struct of_reconfig_data *p) { @@ -620,21 +618,9 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce) } ret = __of_add_property(ce->np, ce->prop); - if (ret) { - pr_err("changeset: add_property failed @%pOF/%s\n", - ce->np, - ce->prop->name); - break; - } break; case OF_RECONFIG_REMOVE_PROPERTY: ret = __of_remove_property(ce->np, ce->prop); - if (ret) { - pr_err("changeset: remove_property failed @%pOF/%s\n", - ce->np, - ce->prop->name); - break; - } break; case OF_RECONFIG_UPDATE_PROPERTY: @@ -648,20 +634,17 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce) } ret = __of_update_property(ce->np, ce->prop, &old_prop); - if (ret) { - pr_err("changeset: update_property failed @%pOF/%s\n", - ce->np, - ce->prop->name); - break; - } break; default: ret = -EINVAL; } raw_spin_unlock_irqrestore(&devtree_lock, flags); - if (ret) + if (ret) { + pr_err("changeset: apply failed: cset<%p> %-15s %pOF:%s\n", + ce, action_names[ce->action], ce->np, ce->prop->name); return ret; + } switch (ce->action) { case OF_RECONFIG_ATTACH_NODE: From patchwork Fri Aug 4 22:41:53 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rob Herring \(Arm\)" X-Patchwork-Id: 710688 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1EBCDC001DB for ; Fri, 4 Aug 2023 22:42:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230263AbjHDWmb (ORCPT ); Fri, 4 Aug 2023 18:42:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44422 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230106AbjHDWm0 (ORCPT ); Fri, 4 Aug 2023 18:42:26 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C29CE4EFA; Fri, 4 Aug 2023 15:42:21 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id B9DD062161; Fri, 4 Aug 2023 22:42:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 738B1C433CA; Fri, 4 Aug 2023 22:42:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1691188940; bh=opEsAI2Xe5riNxll2Ve+4XTHqIK5b0EkHyP6PSvHTMw=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=JDmdzwkiI1HihK9Rl/8E+fZSaeZPwqYB12wp44YfdHh6MGK1H7D+aqBhib3VOm3qt iMuFSWRXHEa9ub8z+ZlauXFHS26FMdSZL1XFJYQlH+b6RQDDS/VIKydgGUuOBjILq0 /cKfr6srFy29IBuDJvyTqx361KSi0v1zO83QBhycJ2YtZqfX2AG44MvZQ7P60R8JUi b0QlM4G1GogznZi/1Y9VDey0ZrAwb/mdoghwdn2Bku2wjmeiks1O5NiEcH5ztvXhhb OTnJA3iQd8H3La2hCEY7AyINOze1vPuGztLV/TmwUYDyeXv4X7jD/9RHDCZY8nFrp9 EOCvnb3sTQK3w== Received: (nullmailer pid 2346979 invoked by uid 1000); Fri, 04 Aug 2023 22:42:09 -0000 From: Rob Herring Date: Fri, 04 Aug 2023 16:41:53 -0600 Subject: [PATCH v2 3/6] of: dynamic: Refactor changeset action printing to common helpers MIME-Version: 1.0 Message-Id: <20230801-dt-changeset-fixes-v2-3-c2b701579dee@kernel.org> References: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> In-Reply-To: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> To: Frank Rowand , "Enrico Weigelt, metux IT consult" , "Rafael J. Wysocki" , Sakari Ailus , Petr Mladek , Andy Shevchenko , Geert Uytterhoeven Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org X-Mailer: b4 0.13-dev Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Several places print the changeset action with node and property details. Refactor these into a common printing helper. The complicating factor is some prints are debug and some are errors. Solve this with a bit of preprocessor magic. Signed-off-by: Rob Herring --- v2: - Split out refactoring from fix in prior patch - Avoid pr_cont and relying on pr_debug return value which dynamic debug doesn't have --- drivers/of/dynamic.c | 53 +++++++++++----------------------------------------- 1 file changed, 11 insertions(+), 42 deletions(-) diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 2f0eb0053773..6eaa66b11a02 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -71,27 +71,21 @@ static const char *action_names[] = { [OF_RECONFIG_UPDATE_PROPERTY] = "UPDATE_PROPERTY", }; +#define _do_print(func, prefix, action, node, prop, ...) ({ \ + func("changeset: " prefix "%-15s %pOF%s%s\n", \ + ##__VA_ARGS__, action_names[action], node, \ + prop ? ":" : "", prop ? prop->name : ""); \ +}) +#define of_changeset_action_err(...) _do_print(pr_err, __VA_ARGS__) +#define of_changeset_action_debug(...) _do_print(pr_debug, __VA_ARGS__) + int of_reconfig_notify(unsigned long action, struct of_reconfig_data *p) { int rc; -#ifdef DEBUG struct of_reconfig_data *pr = p; - switch (action) { - case OF_RECONFIG_ATTACH_NODE: - case OF_RECONFIG_DETACH_NODE: - pr_debug("notify %-15s %pOF\n", action_names[action], - pr->dn); - break; - case OF_RECONFIG_ADD_PROPERTY: - case OF_RECONFIG_REMOVE_PROPERTY: - case OF_RECONFIG_UPDATE_PROPERTY: - pr_debug("notify %-15s %pOF:%s\n", action_names[action], - pr->dn, pr->prop->name); - break; + of_changeset_action_debug("notify: ", action, pr->dn, pr->prop); - } -#endif rc = blocking_notifier_call_chain(&of_reconfig_chain, action, p); return notifier_to_errno(rc); } @@ -502,30 +496,6 @@ static void __of_changeset_entry_destroy(struct of_changeset_entry *ce) kfree(ce); } -#ifdef DEBUG -static void __of_changeset_entry_dump(struct of_changeset_entry *ce) -{ - switch (ce->action) { - case OF_RECONFIG_ADD_PROPERTY: - case OF_RECONFIG_REMOVE_PROPERTY: - case OF_RECONFIG_UPDATE_PROPERTY: - pr_debug("cset<%p> %-15s %pOF/%s\n", ce, action_names[ce->action], - ce->np, ce->prop->name); - break; - case OF_RECONFIG_ATTACH_NODE: - case OF_RECONFIG_DETACH_NODE: - pr_debug("cset<%p> %-15s %pOF\n", ce, action_names[ce->action], - ce->np); - break; - } -} -#else -static inline void __of_changeset_entry_dump(struct of_changeset_entry *ce) -{ - /* empty */ -} -#endif - static void __of_changeset_entry_invert(struct of_changeset_entry *ce, struct of_changeset_entry *rce) { @@ -597,7 +567,7 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce) unsigned long flags; int ret = 0; - __of_changeset_entry_dump(ce); + of_changeset_action_debug("applying: cset<%p> ", ce->action, ce->np, ce->prop, ce); raw_spin_lock_irqsave(&devtree_lock, flags); switch (ce->action) { @@ -641,8 +611,7 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce) raw_spin_unlock_irqrestore(&devtree_lock, flags); if (ret) { - pr_err("changeset: apply failed: cset<%p> %-15s %pOF:%s\n", - ce, action_names[ce->action], ce->np, ce->prop->name); + of_changeset_action_err("apply failed: cset<%p> ", ce->action, ce->np, ce->prop, ce); return ret; } From patchwork Fri Aug 4 22:41:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rob Herring \(Arm\)" X-Patchwork-Id: 710348 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C419DC001DB for ; Fri, 4 Aug 2023 22:42:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230159AbjHDWmc (ORCPT ); Fri, 4 Aug 2023 18:42:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44490 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230181AbjHDWm3 (ORCPT ); Fri, 4 Aug 2023 18:42:29 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 220035249; Fri, 4 Aug 2023 15:42:23 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 988F362179; Fri, 4 Aug 2023 22:42:22 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2EDEEC433BA; Fri, 4 Aug 2023 22:42:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1691188942; bh=8peVQkAUMgH6YYwSmql7OBIk0sITKpX/yuaqGHaDXB4=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=J08bLM+ZIWZyOUIzecJOud+FY5i8wr9eDNnSKKs8j8Lt6897mCjFm3GwPUBlT4b5B QmvBd/vCnAOoj/14mm6KDcbgd3IuPxp1BNtG4F2L6Gw0SUa7+0MfKGAkeL5MB1/PEm 8+Mg25fqrvIqw4ICodp4w/CD2bdTNuIonurUPuAVenTaWAwTZUXzfSRC0Nxu5h/SOs oMj/243kUx2DYjVmY7uNpZJgb9KsQSCDEU//q5rBKLXNnD/Af3odc9UB3/HzWHqF/e Fgdk7yKx1YlB3fJrA5X5pS1yxm8BRiwkrU348u/p8Wq7b3yiFPWNr3u8ABOQtCs+JF gNYURO1nv1EFg== Received: (nullmailer pid 2346981 invoked by uid 1000); Fri, 04 Aug 2023 22:42:09 -0000 From: Rob Herring Date: Fri, 04 Aug 2023 16:41:54 -0600 Subject: [PATCH v2 4/6] of: dynamic: Fix race in getting old property when updating property MIME-Version: 1.0 Message-Id: <20230801-dt-changeset-fixes-v2-4-c2b701579dee@kernel.org> References: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> In-Reply-To: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> To: Frank Rowand , "Enrico Weigelt, metux IT consult" , "Rafael J. Wysocki" , Sakari Ailus , Petr Mladek , Andy Shevchenko , Geert Uytterhoeven Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org X-Mailer: b4 0.13-dev Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org __of_update_property() returns the existing property if there is one, but that value is never added to the changeset. Updates work because the existing property is also retrieved in of_changeset_action(), but that is racy as of_changeset_action() doesn't hold any locks. The property could be changed before the changeset is applied. Signed-off-by: Rob Herring --- drivers/of/dynamic.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 6eaa66b11a02..fbc7c29896a2 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -563,7 +563,7 @@ static int __of_changeset_entry_notify(struct of_changeset_entry *ce, static int __of_changeset_entry_apply(struct of_changeset_entry *ce) { - struct property *old_prop, **propp; + struct property **propp; unsigned long flags; int ret = 0; @@ -603,7 +603,7 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce) } } - ret = __of_update_property(ce->np, ce->prop, &old_prop); + ret = __of_update_property(ce->np, ce->prop, &ce->old_prop); break; default: ret = -EINVAL; @@ -904,9 +904,6 @@ int of_changeset_action(struct of_changeset *ocs, unsigned long action, ce->np = of_node_get(np); ce->prop = prop; - if (action == OF_RECONFIG_UPDATE_PROPERTY && prop) - ce->old_prop = of_find_property(np, prop->name, NULL); - /* add it to the list */ list_add_tail(&ce->node, &ocs->entries); return 0; From patchwork Fri Aug 4 22:41:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rob Herring \(Arm\)" X-Patchwork-Id: 710690 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 998F8C001DB for ; Fri, 4 Aug 2023 22:42:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229564AbjHDWmQ (ORCPT ); Fri, 4 Aug 2023 18:42:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44180 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229989AbjHDWmO (ORCPT ); Fri, 4 Aug 2023 18:42:14 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0A61846B3; Fri, 4 Aug 2023 15:42:14 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 9CB1E62159; Fri, 4 Aug 2023 22:42:13 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6CC97C433C8; Fri, 4 Aug 2023 22:42:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1691188933; bh=gQ4AyC+KFAESrH+STqjHPqsfLxlamqpZ8nc7Nwdra40=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=gHuOBsTCvlhIDAOc0px80xbtJZeBjWWiYvzNykPo2fjGCWicQJcPNAJ9Lqrt7Ebmn 30tc84SAvAFxbyYlzdnoZsuXKncO9m1qHXQddmCYiuYrC+sv2Ow3K3pFfUIAlvbsZh ptPKuabveIzhtU7RORPYeP24+Cwo9iaoHKkj3+CHySpUlcZeEKIY49H0ZhhDloXLFL xN+IRYXJjpURY1vDNfSciFIC/3IyZnGli26xNLsHbkeSQe+nwMMNMsRtu+J/pze8Z/ sIfE+0ZQ9XUu4J2CcOwjSKJ0JMSvf9/qNeclcISbTd+/CxwSE4R3tx16Xu7gOEUPg/ 3wDArUmQAeVQQ== Received: (nullmailer pid 2346983 invoked by uid 1000); Fri, 04 Aug 2023 22:42:09 -0000 From: Rob Herring Date: Fri, 04 Aug 2023 16:41:55 -0600 Subject: [PATCH v2 5/6] of: dynamic: Move dead property list check into property add/update functions MIME-Version: 1.0 Message-Id: <20230801-dt-changeset-fixes-v2-5-c2b701579dee@kernel.org> References: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> In-Reply-To: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> To: Frank Rowand , "Enrico Weigelt, metux IT consult" , "Rafael J. Wysocki" , Sakari Ailus , Petr Mladek , Andy Shevchenko , Geert Uytterhoeven Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org X-Mailer: b4 0.13-dev Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org The changeset code checks for a property in the deadprops list when adding/updating a property, but of_add_property() and of_update_property() do not. As the users of these functions are pretty simple, they have not hit this scenario or else the property lists would get corrupted. Signed-off-by: Rob Herring --- v2: - Add helper to remove property from deadprops list --- drivers/of/base.c | 19 +++++++++++++++++++ drivers/of/dynamic.c | 19 ------------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index e235f3a57ea8..c63a4cde281e 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1530,6 +1530,21 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na } EXPORT_SYMBOL(of_count_phandle_with_args); +static void __of_remove_dead_property(struct device_node *np, struct property *prop) +{ + struct property **next; + + /* If the property is in deadprops then it must be removed */ + for (next = &np->deadprops; *next; next = &(*next)->next) { + if (*next != prop) + continue; + + *next = prop->next; + prop->next = NULL; + break; + } +} + /** * __of_add_property - Add a property to a node without lock operations * @np: Caller's Device Node @@ -1539,6 +1554,8 @@ int __of_add_property(struct device_node *np, struct property *prop) { struct property **next; + __of_remove_dead_property(np, prop); + prop->next = NULL; next = &np->properties; while (*next) { @@ -1641,6 +1658,8 @@ int __of_update_property(struct device_node *np, struct property *newprop, { struct property **next, *oldprop; + __of_remove_dead_property(np, newprop); + for (next = &np->properties; *next; next = &(*next)->next) { if (of_prop_cmp((*next)->name, newprop->name) == 0) break; diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index fbc7c29896a2..769869e8b847 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -563,7 +563,6 @@ static int __of_changeset_entry_notify(struct of_changeset_entry *ce, static int __of_changeset_entry_apply(struct of_changeset_entry *ce) { - struct property **propp; unsigned long flags; int ret = 0; @@ -578,15 +577,6 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce) __of_detach_node(ce->np); break; case OF_RECONFIG_ADD_PROPERTY: - /* If the property is in deadprops then it must be removed */ - for (propp = &ce->np->deadprops; *propp; propp = &(*propp)->next) { - if (*propp == ce->prop) { - *propp = ce->prop->next; - ce->prop->next = NULL; - break; - } - } - ret = __of_add_property(ce->np, ce->prop); break; case OF_RECONFIG_REMOVE_PROPERTY: @@ -594,15 +584,6 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce) break; case OF_RECONFIG_UPDATE_PROPERTY: - /* If the property is in deadprops then it must be removed */ - for (propp = &ce->np->deadprops; *propp; propp = &(*propp)->next) { - if (*propp == ce->prop) { - *propp = ce->prop->next; - ce->prop->next = NULL; - break; - } - } - ret = __of_update_property(ce->np, ce->prop, &ce->old_prop); break; default: From patchwork Fri Aug 4 22:41:56 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rob Herring \(Arm\)" X-Patchwork-Id: 710349 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 66B55C04A6A for ; Fri, 4 Aug 2023 22:42:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230198AbjHDWm3 (ORCPT ); Fri, 4 Aug 2023 18:42:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44386 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230132AbjHDWmZ (ORCPT ); Fri, 4 Aug 2023 18:42:25 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F21284EE7; Fri, 4 Aug 2023 15:42:19 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id F058562158; Fri, 4 Aug 2023 22:42:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 94B8BC433C8; Fri, 4 Aug 2023 22:42:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1691188938; bh=XV5U4RAiGWQ2P3Dd0SD54QZTwqv55+/YNgiA55fi47g=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=XC1NGwgD+z/NQJIqzJUtEqL7Zi3OudVrKz6SXwQFsiCaNejoa1Yfbceu3Lhk+Q2Go +1Mvk8Nh/+stAt3bPKN5RbO6quXWO0KNJywyU40q+EoxMdCoE77re7UbNSC0NAS3ng Fwa85cKOq4khyo3u1Hcpao8BkOSMv8sBdnKf7eqNZkUcUR78qD55mvdcoyx9bfdTRf RL0e2AmtTrMO1SLco8+qs9B5NXsjLaU95E8PV3LIh4oCuwprTnIr06XcQhv+Nc0nig VgX6ramvoA+N55xfBhYX49vYMtp0ARQirG0ntYR0ijhD+sLkK/eqGHS/9vP/5vIO3c lzFtvEAq2hgsQ== Received: (nullmailer pid 2346985 invoked by uid 1000); Fri, 04 Aug 2023 22:42:09 -0000 From: Rob Herring Date: Fri, 04 Aug 2023 16:41:56 -0600 Subject: [PATCH v2 6/6] of: Refactor node and property manipulation function locking MIME-Version: 1.0 Message-Id: <20230801-dt-changeset-fixes-v2-6-c2b701579dee@kernel.org> References: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> In-Reply-To: <20230801-dt-changeset-fixes-v2-0-c2b701579dee@kernel.org> To: Frank Rowand , "Enrico Weigelt, metux IT consult" , "Rafael J. Wysocki" , Sakari Ailus , Petr Mladek , Andy Shevchenko , Geert Uytterhoeven Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org X-Mailer: b4 0.13-dev Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org All callers of __of_{add,remove,update}_property() and __of_{attach,detach}_node() wrap the call with the devtree_lock spinlock. Let's move the spinlock into the functions. This allows moving the sysfs update functions into those functions as well. Signed-off-by: Rob Herring --- v2: - Re-arrange exit handling --- drivers/of/base.c | 68 +++++++++++++++++++++++++++------------------------- drivers/of/dynamic.c | 51 +++++++++++++-------------------------- 2 files changed, 51 insertions(+), 68 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index c63a4cde281e..3ec134429f11 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1552,21 +1552,32 @@ static void __of_remove_dead_property(struct device_node *np, struct property *p */ int __of_add_property(struct device_node *np, struct property *prop) { + int rc = 0; + unsigned long flags; struct property **next; + raw_spin_lock_irqsave(&devtree_lock, flags); + __of_remove_dead_property(np, prop); prop->next = NULL; next = &np->properties; while (*next) { - if (strcmp(prop->name, (*next)->name) == 0) + if (strcmp(prop->name, (*next)->name) == 0) { /* duplicate ! don't insert it */ - return -EEXIST; - + rc = -EEXIST; + goto out_unlock; + } next = &(*next)->next; } *next = prop; +out_unlock: + raw_spin_unlock_irqrestore(&devtree_lock, flags); + if (rc) + return rc; + + __of_add_property_sysfs(np, prop); return 0; } @@ -1577,23 +1588,12 @@ int __of_add_property(struct device_node *np, struct property *prop) */ int of_add_property(struct device_node *np, struct property *prop) { - unsigned long flags; int rc; mutex_lock(&of_mutex); - - raw_spin_lock_irqsave(&devtree_lock, flags); rc = __of_add_property(np, prop); - raw_spin_unlock_irqrestore(&devtree_lock, flags); - - if (!rc) - __of_add_property_sysfs(np, prop); - mutex_unlock(&of_mutex); - if (!rc) - of_property_notify(OF_RECONFIG_ADD_PROPERTY, np, prop, NULL); - return rc; } EXPORT_SYMBOL_GPL(of_add_property); @@ -1601,19 +1601,30 @@ EXPORT_SYMBOL_GPL(of_add_property); int __of_remove_property(struct device_node *np, struct property *prop) { struct property **next; + unsigned long flags; + int rc = 0; + + raw_spin_lock_irqsave(&devtree_lock, flags); for (next = &np->properties; *next; next = &(*next)->next) { if (*next == prop) break; } - if (*next == NULL) - return -ENODEV; - + if (*next == NULL) { + rc = -ENODEV; + goto out_unlock; + } /* found the node */ *next = prop->next; prop->next = np->deadprops; np->deadprops = prop; +out_unlock: + raw_spin_unlock_irqrestore(&devtree_lock, flags); + if (rc) + return rc; + + __of_remove_property_sysfs(np, prop); return 0; } @@ -1629,21 +1640,13 @@ int __of_remove_property(struct device_node *np, struct property *prop) */ int of_remove_property(struct device_node *np, struct property *prop) { - unsigned long flags; int rc; if (!prop) return -ENODEV; mutex_lock(&of_mutex); - - raw_spin_lock_irqsave(&devtree_lock, flags); rc = __of_remove_property(np, prop); - raw_spin_unlock_irqrestore(&devtree_lock, flags); - - if (!rc) - __of_remove_property_sysfs(np, prop); - mutex_unlock(&of_mutex); if (!rc) @@ -1657,6 +1660,9 @@ int __of_update_property(struct device_node *np, struct property *newprop, struct property **oldpropp) { struct property **next, *oldprop; + unsigned long flags; + + raw_spin_lock_irqsave(&devtree_lock, flags); __of_remove_dead_property(np, newprop); @@ -1678,6 +1684,10 @@ int __of_update_property(struct device_node *np, struct property *newprop, *next = newprop; } + raw_spin_unlock_irqrestore(&devtree_lock, flags); + + __of_update_property_sysfs(np, newprop, oldprop); + return 0; } @@ -1693,21 +1703,13 @@ int __of_update_property(struct device_node *np, struct property *newprop, int of_update_property(struct device_node *np, struct property *newprop) { struct property *oldprop; - unsigned long flags; int rc; if (!newprop->name) return -EINVAL; mutex_lock(&of_mutex); - - raw_spin_lock_irqsave(&devtree_lock, flags); rc = __of_update_property(np, newprop, &oldprop); - raw_spin_unlock_irqrestore(&devtree_lock, flags); - - if (!rc) - __of_update_property_sysfs(np, newprop, oldprop); - mutex_unlock(&of_mutex); if (!rc) diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 769869e8b847..715365fbb8ea 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -197,6 +197,9 @@ static void __of_attach_node(struct device_node *np) { const __be32 *phandle; int sz; + unsigned long flags; + + raw_spin_lock_irqsave(&devtree_lock, flags); if (!of_node_check_flag(np, OF_OVERLAY)) { np->name = __of_get_property(np, "name", NULL); @@ -219,6 +222,10 @@ static void __of_attach_node(struct device_node *np) np->parent->child = np; of_node_clear_flag(np, OF_DETACHED); np->fwnode.flags |= FWNODE_FLAG_NOT_DEVICE; + + raw_spin_unlock_irqrestore(&devtree_lock, flags); + + __of_attach_node_sysfs(np); } /** @@ -228,17 +235,12 @@ static void __of_attach_node(struct device_node *np) int of_attach_node(struct device_node *np) { struct of_reconfig_data rd; - unsigned long flags; memset(&rd, 0, sizeof(rd)); rd.dn = np; mutex_lock(&of_mutex); - raw_spin_lock_irqsave(&devtree_lock, flags); __of_attach_node(np); - raw_spin_unlock_irqrestore(&devtree_lock, flags); - - __of_attach_node_sysfs(np); mutex_unlock(&of_mutex); of_reconfig_notify(OF_RECONFIG_ATTACH_NODE, &rd); @@ -249,13 +251,15 @@ int of_attach_node(struct device_node *np) void __of_detach_node(struct device_node *np) { struct device_node *parent; + unsigned long flags; - if (WARN_ON(of_node_check_flag(np, OF_DETACHED))) - return; + raw_spin_lock_irqsave(&devtree_lock, flags); parent = np->parent; - if (WARN_ON(!parent)) + if (WARN_ON(of_node_check_flag(np, OF_DETACHED) || !parent)) { + raw_spin_unlock_irqrestore(&devtree_lock, flags); return; + } if (parent->child == np) parent->child = np->sibling; @@ -272,6 +276,10 @@ void __of_detach_node(struct device_node *np) /* race with of_find_node_by_phandle() prevented by devtree_lock */ __of_phandle_cache_inv_entry(np->phandle); + + raw_spin_unlock_irqrestore(&devtree_lock, flags); + + __of_detach_node_sysfs(np); } /** @@ -281,17 +289,12 @@ void __of_detach_node(struct device_node *np) int of_detach_node(struct device_node *np) { struct of_reconfig_data rd; - unsigned long flags; memset(&rd, 0, sizeof(rd)); rd.dn = np; mutex_lock(&of_mutex); - raw_spin_lock_irqsave(&devtree_lock, flags); __of_detach_node(np); - raw_spin_unlock_irqrestore(&devtree_lock, flags); - - __of_detach_node_sysfs(np); mutex_unlock(&of_mutex); of_reconfig_notify(OF_RECONFIG_DETACH_NODE, &rd); @@ -563,12 +566,10 @@ static int __of_changeset_entry_notify(struct of_changeset_entry *ce, static int __of_changeset_entry_apply(struct of_changeset_entry *ce) { - unsigned long flags; int ret = 0; of_changeset_action_debug("applying: cset<%p> ", ce->action, ce->np, ce->prop, ce); - raw_spin_lock_irqsave(&devtree_lock, flags); switch (ce->action) { case OF_RECONFIG_ATTACH_NODE: __of_attach_node(ce->np); @@ -589,32 +590,12 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce) default: ret = -EINVAL; } - raw_spin_unlock_irqrestore(&devtree_lock, flags); if (ret) { of_changeset_action_err("apply failed: cset<%p> ", ce->action, ce->np, ce->prop, ce); return ret; } - switch (ce->action) { - case OF_RECONFIG_ATTACH_NODE: - __of_attach_node_sysfs(ce->np); - break; - case OF_RECONFIG_DETACH_NODE: - __of_detach_node_sysfs(ce->np); - break; - case OF_RECONFIG_ADD_PROPERTY: - /* ignore duplicate names */ - __of_add_property_sysfs(ce->np, ce->prop); - break; - case OF_RECONFIG_REMOVE_PROPERTY: - __of_remove_property_sysfs(ce->np, ce->prop); - break; - case OF_RECONFIG_UPDATE_PROPERTY: - __of_update_property_sysfs(ce->np, ce->prop, ce->old_prop); - break; - } - return 0; }