From patchwork Wed Mar 12 22:49:40 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Likely X-Patchwork-Id: 26163 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-oa0-f69.google.com (mail-oa0-f69.google.com [209.85.219.69]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 1CE67236AC for ; Wed, 12 Mar 2014 22:49:55 +0000 (UTC) Received: by mail-oa0-f69.google.com with SMTP id i7sf772095oag.4 for ; Wed, 12 Mar 2014 15:49:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:to:cc:subject :date:message-id:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=mtsrxp2m+oxSTWh8zDWv49N9jd8XPdmKDxhDkjhEsmA=; b=C2XWLXbDXc7Xw9ddvwkxw9KGnwZLgwRKsZsjWfhCJ1CV9LqQB0v+D5FwUtHKsmkGmM nlxdBRtP9x4lEb01j4bysdKCz2K2+CYv/OgUMh+Sj9KeCpgrq2B2rcq+YjM968asagoO bWHQ5/vThH1icvEH9VNSjIWkc1JMnQsjy4FXaay4y9h22qyKsMm18cITRVqO/glJotYm NY22YuEFI/KiOnCqYqe0RHWa4DHXkkdB4Jlfz983v8vlQW5xMwDmPFdqfdqyPAIlev8S wWbS3zETjflHdsrfx2aa9MIig6zPteQBJqZ5WMJBw05HNH60RiXuFiwN9ZFpNeQ7rAA7 VorA== X-Gm-Message-State: ALoCoQlSsy5SOC/6694ziprfCyDf1z4TFfPma0cBgKRcCiuHHUHEWmqYGhx6sDHSon0Fq+0OSdjn X-Received: by 10.182.24.134 with SMTP id u6mr70937obf.24.1394664595238; Wed, 12 Mar 2014 15:49:55 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.44.101 with SMTP id f92ls54621qga.61.gmail; Wed, 12 Mar 2014 15:49:55 -0700 (PDT) X-Received: by 10.221.22.71 with SMTP id qv7mr59791vcb.34.1394664595133; Wed, 12 Mar 2014 15:49:55 -0700 (PDT) Received: from mail-ve0-f175.google.com (mail-ve0-f175.google.com [209.85.128.175]) by mx.google.com with ESMTPS id eb8si96561vdb.125.2014.03.12.15.49.55 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 12 Mar 2014 15:49:55 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.128.175 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.128.175; Received: by mail-ve0-f175.google.com with SMTP id oz11so214326veb.6 for ; Wed, 12 Mar 2014 15:49:55 -0700 (PDT) X-Received: by 10.58.12.169 with SMTP id z9mr2632veb.61.1394664595021; Wed, 12 Mar 2014 15:49:55 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.220.78.9 with SMTP id i9csp324230vck; Wed, 12 Mar 2014 15:49:54 -0700 (PDT) X-Received: by 10.66.102.39 with SMTP id fl7mr72862pab.43.1394664594159; Wed, 12 Mar 2014 15:49:54 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id ub8si3613368pac.271.2014.03.12.15.49.53; Wed, 12 Mar 2014 15:49:53 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of devicetree-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752031AbaCLWtw (ORCPT + 9 others); Wed, 12 Mar 2014 18:49:52 -0400 Received: from mail-ea0-f172.google.com ([209.85.215.172]:53392 "EHLO mail-ea0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751265AbaCLWtv (ORCPT ); Wed, 12 Mar 2014 18:49:51 -0400 Received: by mail-ea0-f172.google.com with SMTP id l9so142099eaj.3 for ; Wed, 12 Mar 2014 15:49:50 -0700 (PDT) X-Received: by 10.15.102.74 with SMTP id bq50mr59340eeb.21.1394664590118; Wed, 12 Mar 2014 15:49:50 -0700 (PDT) Received: from trevor.secretlab.ca (host109-153-30-112.range109-153.btcentralplus.com. [109.153.30.112]) by mx.google.com with ESMTPSA id x6sm1132944eew.20.2014.03.12.15.49.48 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 12 Mar 2014 15:49:49 -0700 (PDT) Received: by trevor.secretlab.ca (Postfix, from userid 1000) id 35228C413AC; Wed, 12 Mar 2014 22:49:43 +0000 (GMT) From: Grant Likely To: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Rob Herring Cc: Pantelis Antoniou , Grant Likely Subject: [PATCH] OF: kobj node lifecycle fixes Date: Wed, 12 Mar 2014 22:49:40 +0000 Message-Id: <1394664580-23130-1-git-send-email-grant.likely@linaro.org> X-Mailer: git-send-email 1.8.3.2 Sender: devicetree-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: devicetree@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: grant.likely@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.128.175 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , From: Pantelis Antoniou After the move to having device nodes be proper kobjects the lifecycle of the node needs to be controlled better. At first convert of_add_node() in the unflattened functions to of_init_node() which initializes the kobject so that of_node_get/put work correctly even before of_init is called. Afterwards introduce of_node_is_initialized & of_node_is_attached that query the underlying kobject about the state (attached means kobj is visible in sysfs) Using that make sure the lifecycle of the tree is correct at all times. Signed-off-by: Pantelis Antoniou [grant.likely: moved of_node_init() calls, fixed up locking, and dropped __of_populate() hunks] Signed-off-by: Grant Likely --- I've reworked Pantelis' patch to work on my tree. Here's the result. I've already got this in my tree and will be pushing it out shortly. g. drivers/of/base.c | 35 ++++++++++++++++++++++++++--------- drivers/of/fdt.c | 3 +-- drivers/of/pdt.c | 3 +-- include/linux/of.h | 19 +++++++++++++++++++ 4 files changed, 47 insertions(+), 13 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index ed3e70b84957..7083fad079a6 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -246,10 +246,19 @@ static int __of_node_add(struct device_node *np) int of_node_add(struct device_node *np) { int rc = 0; - kobject_init(&np->kobj, &of_node_ktype); + + BUG_ON(!of_node_is_initialized(np)); + + /* + * Grab the mutex here so that in a race condition between of_init() and + * of_node_add(), node addition will still be consistent. + */ mutex_lock(&of_aliases_mutex); if (of_kset) rc = __of_node_add(np); + else + /* This scenario may be perfectly valid, but report it anyway */ + pr_info("of_node_add(%s) before of_init()\n", np->full_name); mutex_unlock(&of_aliases_mutex); return rc; } @@ -259,10 +268,17 @@ static void of_node_remove(struct device_node *np) { struct property *pp; - for_each_property_of_node(np, pp) - sysfs_remove_bin_file(&np->kobj, &pp->attr); + BUG_ON(!of_node_is_initialized(np)); + + /* only remove properties if on sysfs */ + if (of_node_is_attached(np)) { + for_each_property_of_node(np, pp) + sysfs_remove_bin_file(&np->kobj, &pp->attr); + kobject_del(&np->kobj); + } - kobject_del(&np->kobj); + /* finally remove the kobj_init ref */ + of_node_put(np); } #endif @@ -1631,6 +1647,10 @@ static int of_property_notify(int action, struct device_node *np, { struct of_prop_reconfig pr; + /* only call notifiers if the node is attached */ + if (!of_node_is_attached(np)) + return 0; + pr.dn = np; pr.prop = prop; return of_reconfig_notify(action, &pr); @@ -1682,11 +1702,8 @@ int of_add_property(struct device_node *np, struct property *prop) if (rc) return rc; - /* at early boot, bail hear and defer setup to of_init() */ - if (!of_kset) - return 0; - - __of_add_property_sysfs(np, prop); + if (of_node_is_attached(np)); + __of_add_property_sysfs(np, prop); return rc; } diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 96ad1ab7f9d6..70ccc36513e7 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -202,6 +202,7 @@ static void * unflatten_dt_node(struct boot_param_header *blob, __alignof__(struct device_node)); if (allnextpp) { char *fn; + of_node_init(np); np->full_name = fn = ((char *)np) + sizeof(*np); if (new_format) { /* rebuild full path for new format */ @@ -326,8 +327,6 @@ static void * unflatten_dt_node(struct boot_param_header *blob, np->name = ""; if (!np->type) np->type = ""; - - of_node_add(np); } while (tag == OF_DT_BEGIN_NODE || tag == OF_DT_NOP) { if (tag == OF_DT_NOP) diff --git a/drivers/of/pdt.c b/drivers/of/pdt.c index e64fa3d3da5f..36b4035881b0 100644 --- a/drivers/of/pdt.c +++ b/drivers/of/pdt.c @@ -176,6 +176,7 @@ static struct device_node * __init of_pdt_create_node(phandle node, return NULL; dp = prom_early_alloc(sizeof(*dp)); + of_node_init(dp); of_pdt_incr_unique_id(dp); dp->parent = parent; @@ -213,7 +214,6 @@ static struct device_node * __init of_pdt_build_tree(struct device_node *parent, *nextp = &dp->allnext; dp->full_name = of_pdt_build_full_name(dp); - of_node_add(dp); dp->child = of_pdt_build_tree(dp, of_pdt_prom_ops->getchild(node), nextp); @@ -244,7 +244,6 @@ void __init of_pdt_build_devicetree(phandle root_node, struct of_pdt_ops *ops) of_allnodes->path_component_name = ""; #endif of_allnodes->full_name = "/"; - of_node_add(of_allnodes); nextp = &of_allnodes->allnext; of_allnodes->child = of_pdt_build_tree(of_allnodes, diff --git a/include/linux/of.h b/include/linux/of.h index 257994a420f3..a8b9dad90c64 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -76,6 +76,25 @@ struct of_phandle_args { extern int of_node_add(struct device_node *node); +/* initialize a node */ +extern struct kobj_type of_node_ktype; +static inline void of_node_init(struct device_node *node) +{ + kobject_init(&node->kobj, &of_node_ktype); +} + +/* true when node is initialized */ +static inline int of_node_is_initialized(struct device_node *node) +{ + return node && node->kobj.state_initialized; +} + +/* true when node is attached (i.e. present on sysfs) */ +static inline int of_node_is_attached(struct device_node *node) +{ + return node && node->kobj.state_in_sysfs; +} + #ifdef CONFIG_OF_DYNAMIC extern struct device_node *of_node_get(struct device_node *node); extern void of_node_put(struct device_node *node);