All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>,
	Pantelis Antoniou
	<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
Subject: Re: [PATCH 2/3] of: Make of_find_node_by_path() handle /aliases
Date: Tue, 20 May 2014 19:55:45 -0700	[thread overview]
Message-ID: <537C15B1.3020006@gmail.com> (raw)
In-Reply-To: <1399993115-21552-3-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 5/13/2014 7:58 AM, Grant Likely wrote:

> Make of_find_node_by_path() handle aliases as prefixes. To make this

> work the name search is refactored to search by path component instead

> of by full string. This should be a more efficient search, and it makes

> it possible to start a search at a subnode of a tree.

> 

> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

> [grant.likely: Rework to not require allocating at runtime]

> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> Signed-off-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

> ---

>  drivers/of/base.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++----

>  1 file changed, 56 insertions(+), 4 deletions(-)



Was this patch created against a tree that has modifications to device tree

locking?
  I get a hang due to deadlock when I apply it.  Patch to verify the
cause is below.




From: Frank Rowand <frank.rowand-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>

Do not apply this patch -- it is not a valid fix.  The point of this patch
is to confirm the cause of a bug.

Problem seen on 3.15-rc1 with dragonboard platform patches added.

The dragonboard hangs on boot the very first time that of_find_node_by_path()
is called:

   of_find_node_by_path()
      raw_spin_lock_irqsave(&devtree_lock, flags)
      np = __of_find_node_by_path(np, path)
         for_each_child_of_node(parent, child) {

            // This is a define:
	    #define for_each_child_of_node(parent, child) \
	       for (child = of_get_next_child(parent, NULL); child != NULL; \

	          // Resulting in a deadlock when of_get_next_child()
		  // attempts to lock the same lock:
	          raw_spin_lock_irqsave(&devtree_lock, flags)

When this patch is applied, the first lock is commented out so the
deadlock does not occur and the boot completes.

Not-signed-off-by: Frank Rowand <frank.rowand-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
---
 drivers/of/base.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: b/drivers/of/base.c
===================================================================
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -835,13 +835,17 @@ struct device_node *of_find_node_by_path
 	}
 
 	/* Step down the tree matching path components */
+#if 0
 	raw_spin_lock_irqsave(&devtree_lock, flags);
+#endif
 	while (np && *path == '/') {
 		path++; /* Increment past '/' delimiter */
 		np = __of_find_node_by_path(np, path);
 		path = strchrnul(path, '/');
 	}
+#if 0
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
+#endif
 	return np;
 }
 EXPORT_SYMBOL(of_find_node_by_path);
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Frank Rowand <frowand.list@gmail.com>
To: Grant Likely <grant.likely@linaro.org>
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	David Daney <david.daney@cavium.com>,
	Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Subject: Re: [PATCH 2/3] of: Make of_find_node_by_path() handle /aliases
Date: Tue, 20 May 2014 19:55:45 -0700	[thread overview]
Message-ID: <537C15B1.3020006@gmail.com> (raw)
In-Reply-To: <1399993115-21552-3-git-send-email-grant.likely@linaro.org>

On 5/13/2014 7:58 AM, Grant Likely wrote:

> Make of_find_node_by_path() handle aliases as prefixes. To make this

> work the name search is refactored to search by path component instead

> of by full string. This should be a more efficient search, and it makes

> it possible to start a search at a subnode of a tree.

> 

> Signed-off-by: David Daney <david.daney@cavium.com>

> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>

> [grant.likely: Rework to not require allocating at runtime]

> Acked-by: Rob Herring <robh@kernel.org>

> Signed-off-by: Grant Likely <grant.likely@linaro.org>

> ---

>  drivers/of/base.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++----

>  1 file changed, 56 insertions(+), 4 deletions(-)



Was this patch created against a tree that has modifications to device tree

locking?
  I get a hang due to deadlock when I apply it.  Patch to verify the
cause is below.




From: Frank Rowand <frank.rowand@sonymobile.com>

Do not apply this patch -- it is not a valid fix.  The point of this patch
is to confirm the cause of a bug.

Problem seen on 3.15-rc1 with dragonboard platform patches added.

The dragonboard hangs on boot the very first time that of_find_node_by_path()
is called:

   of_find_node_by_path()
      raw_spin_lock_irqsave(&devtree_lock, flags)
      np = __of_find_node_by_path(np, path)
         for_each_child_of_node(parent, child) {

            // This is a define:
	    #define for_each_child_of_node(parent, child) \
	       for (child = of_get_next_child(parent, NULL); child != NULL; \

	          // Resulting in a deadlock when of_get_next_child()
		  // attempts to lock the same lock:
	          raw_spin_lock_irqsave(&devtree_lock, flags)

When this patch is applied, the first lock is commented out so the
deadlock does not occur and the boot completes.

Not-signed-off-by: Frank Rowand <frank.rowand@sonymobile.com>
---
 drivers/of/base.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: b/drivers/of/base.c
===================================================================
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -835,13 +835,17 @@ struct device_node *of_find_node_by_path
 	}
 
 	/* Step down the tree matching path components */
+#if 0
 	raw_spin_lock_irqsave(&devtree_lock, flags);
+#endif
 	while (np && *path == '/') {
 		path++; /* Increment past '/' delimiter */
 		np = __of_find_node_by_path(np, path);
 		path = strchrnul(path, '/');
 	}
+#if 0
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
+#endif
 	return np;
 }
 EXPORT_SYMBOL(of_find_node_by_path);

  parent reply	other threads:[~2014-05-21  2:55 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-13 14:58 [PATCH 0/3] Rework of_find_node_by_path() code Grant Likely
2014-05-13 14:58 ` [PATCH 1/3] lib: add glibc style strchrnul() variant Grant Likely
2014-05-15 22:19   ` Frank Rowand
2014-05-16 15:00     ` Grant Likely
2014-05-13 14:58 ` [PATCH 2/3] of: Make of_find_node_by_path() handle /aliases Grant Likely
2014-05-16  2:51   ` Frank Rowand
     [not found]     ` <53757D25.1060005-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-05-16 10:54       ` Grant Likely
2014-05-16 10:54         ` Grant Likely
2014-05-18  9:27         ` Grant Likely
     [not found]           ` <20140518092730.109AAC40B8A-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2014-05-21  2:41             ` Frank Rowand
2014-05-21  2:41               ` Frank Rowand
     [not found]               ` <537C1252.3090900-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-05-21  2:46                 ` Frank Rowand
2014-05-21  2:46                   ` Frank Rowand
2014-05-22  3:13                   ` Grant Likely
     [not found]                     ` <20140522031306.DD6B7C41847-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2014-05-23  0:53                       ` Frank Rowand
2014-05-23  0:53                         ` Frank Rowand
2014-05-22  1:16               ` Grant Likely
     [not found]                 ` <20140522011600.C0014C40B13-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2014-05-23  1:14                   ` Frank Rowand
2014-05-23  1:14                     ` Frank Rowand
2014-05-23 21:13                     ` Grant Likely
     [not found]   ` <1399993115-21552-3-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2014-05-21  2:55     ` Frank Rowand [this message]
2014-05-21  2:55       ` Frank Rowand
2014-05-21 16:09       ` Grant Likely
2014-05-22  1:27         ` Frank Rowand
     [not found] ` <1399993115-21552-1-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2014-05-13 14:58   ` [PATCH 3/3] of: Add a testcase for of_find_node_by_path() Grant Likely
2014-05-13 14:58     ` Grant Likely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=537C15B1.3020006@gmail.com \
    --to=frowand.list-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.