* [PATCH 1/1] devicetree: of_node_put() does not require holding devtree_lock
@ 2017-02-22 9:29 Sakari Ailus
2017-02-23 16:02 ` Rob Herring
[not found] ` <1487755758-6066-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
0 siblings, 2 replies; 7+ messages in thread
From: Sakari Ailus @ 2017-02-22 9:29 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA
While holding a reference to a device_node it is allowed to put that node
without holding devtree_lock spinlock. Move of_node_put() after releasing
the spinlock.
Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/of/base.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d4bea3c..a8d7fed 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -286,8 +286,8 @@ struct device_node *of_find_all_nodes(struct device_node *prev)
raw_spin_lock_irqsave(&devtree_lock, flags);
np = __of_find_all_nodes(prev);
of_node_get(np);
- of_node_put(prev);
raw_spin_unlock_irqrestore(&devtree_lock, flags);
+ of_node_put(prev);
return np;
}
EXPORT_SYMBOL(of_find_all_nodes);
@@ -660,8 +660,8 @@ struct device_node *of_get_next_parent(struct device_node *node)
raw_spin_lock_irqsave(&devtree_lock, flags);
parent = of_node_get(node->parent);
- of_node_put(node);
raw_spin_unlock_irqrestore(&devtree_lock, flags);
+ of_node_put(node);
return parent;
}
EXPORT_SYMBOL(of_get_next_parent);
@@ -732,8 +732,8 @@ struct device_node *of_get_next_available_child(const struct device_node *node,
if (of_node_get(next))
break;
}
- of_node_put(prev);
raw_spin_unlock_irqrestore(&devtree_lock, flags);
+ of_node_put(prev);
return next;
}
EXPORT_SYMBOL(of_get_next_available_child);
@@ -875,8 +875,8 @@ struct device_node *of_find_node_by_name(struct device_node *from,
if (np->name && (of_node_cmp(np->name, name) == 0)
&& of_node_get(np))
break;
- of_node_put(from);
raw_spin_unlock_irqrestore(&devtree_lock, flags);
+ of_node_put(from);
return np;
}
EXPORT_SYMBOL(of_find_node_by_name);
@@ -904,8 +904,8 @@ struct device_node *of_find_node_by_type(struct device_node *from,
if (np->type && (of_node_cmp(np->type, type) == 0)
&& of_node_get(np))
break;
- of_node_put(from);
raw_spin_unlock_irqrestore(&devtree_lock, flags);
+ of_node_put(from);
return np;
}
EXPORT_SYMBOL(of_find_node_by_type);
@@ -935,8 +935,8 @@ struct device_node *of_find_compatible_node(struct device_node *from,
if (__of_device_is_compatible(np, compatible, type, NULL) &&
of_node_get(np))
break;
- of_node_put(from);
raw_spin_unlock_irqrestore(&devtree_lock, flags);
+ of_node_put(from);
return np;
}
EXPORT_SYMBOL(of_find_compatible_node);
@@ -970,8 +970,8 @@ struct device_node *of_find_node_with_property(struct device_node *from,
}
}
out:
- of_node_put(from);
raw_spin_unlock_irqrestore(&devtree_lock, flags);
+ of_node_put(from);
return np;
}
EXPORT_SYMBOL(of_find_node_with_property);
@@ -1051,8 +1051,8 @@ struct device_node *of_find_matching_node_and_match(struct device_node *from,
break;
}
}
- of_node_put(from);
raw_spin_unlock_irqrestore(&devtree_lock, flags);
+ of_node_put(from);
return np;
}
EXPORT_SYMBOL(of_find_matching_node_and_match);
--
2.7.4
--
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] devicetree: of_node_put() does not require holding devtree_lock
2017-02-22 9:29 [PATCH 1/1] devicetree: of_node_put() does not require holding devtree_lock Sakari Ailus
@ 2017-02-23 16:02 ` Rob Herring
[not found] ` <1487755758-6066-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
1 sibling, 0 replies; 7+ messages in thread
From: Rob Herring @ 2017-02-23 16:02 UTC (permalink / raw)
To: Sakari Ailus; +Cc: devicetree@vger.kernel.org
On Wed, Feb 22, 2017 at 3:29 AM, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> While holding a reference to a device_node it is allowed to put that node
> without holding devtree_lock spinlock. Move of_node_put() after releasing
> the spinlock.
True, but why do you really care? It is not a heavyweight operation.
Rob
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> drivers/of/base.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d4bea3c..a8d7fed 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -286,8 +286,8 @@ struct device_node *of_find_all_nodes(struct device_node *prev)
> raw_spin_lock_irqsave(&devtree_lock, flags);
> np = __of_find_all_nodes(prev);
> of_node_get(np);
> - of_node_put(prev);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(prev);
> return np;
> }
> EXPORT_SYMBOL(of_find_all_nodes);
> @@ -660,8 +660,8 @@ struct device_node *of_get_next_parent(struct device_node *node)
>
> raw_spin_lock_irqsave(&devtree_lock, flags);
> parent = of_node_get(node->parent);
> - of_node_put(node);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(node);
> return parent;
> }
> EXPORT_SYMBOL(of_get_next_parent);
> @@ -732,8 +732,8 @@ struct device_node *of_get_next_available_child(const struct device_node *node,
> if (of_node_get(next))
> break;
> }
> - of_node_put(prev);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(prev);
> return next;
> }
> EXPORT_SYMBOL(of_get_next_available_child);
> @@ -875,8 +875,8 @@ struct device_node *of_find_node_by_name(struct device_node *from,
> if (np->name && (of_node_cmp(np->name, name) == 0)
> && of_node_get(np))
> break;
> - of_node_put(from);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(from);
> return np;
> }
> EXPORT_SYMBOL(of_find_node_by_name);
> @@ -904,8 +904,8 @@ struct device_node *of_find_node_by_type(struct device_node *from,
> if (np->type && (of_node_cmp(np->type, type) == 0)
> && of_node_get(np))
> break;
> - of_node_put(from);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(from);
> return np;
> }
> EXPORT_SYMBOL(of_find_node_by_type);
> @@ -935,8 +935,8 @@ struct device_node *of_find_compatible_node(struct device_node *from,
> if (__of_device_is_compatible(np, compatible, type, NULL) &&
> of_node_get(np))
> break;
> - of_node_put(from);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(from);
> return np;
> }
> EXPORT_SYMBOL(of_find_compatible_node);
> @@ -970,8 +970,8 @@ struct device_node *of_find_node_with_property(struct device_node *from,
> }
> }
> out:
> - of_node_put(from);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(from);
> return np;
> }
> EXPORT_SYMBOL(of_find_node_with_property);
> @@ -1051,8 +1051,8 @@ struct device_node *of_find_matching_node_and_match(struct device_node *from,
> break;
> }
> }
> - of_node_put(from);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(from);
> return np;
> }
> EXPORT_SYMBOL(of_find_matching_node_and_match);
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread[parent not found: <1487755758-6066-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH 1/1] devicetree: of_node_put() does not require holding devtree_lock
[not found] ` <1487755758-6066-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2017-02-26 21:08 ` Frank Rowand
[not found] ` <58B343C6.9060604-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-27 14:54 ` Frank Rowand
1 sibling, 1 reply; 7+ messages in thread
From: Frank Rowand @ 2017-02-26 21:08 UTC (permalink / raw)
To: Sakari Ailus, devicetree-u79uwXL29TY76Z2rM5mHXA; +Cc: Rob Herring
Adding Rob.
On 02/22/17 01:29, Sakari Ailus wrote:
> While holding a reference to a device_node it is allowed to put that node
> without holding devtree_lock spinlock. Move of_node_put() after releasing
> the spinlock.
Please explain why the change fixes or improves the code. In other words,
what is the point of the change?
Please use getmaintainer to find who to send the patch to.
-Frank
>
> Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
> drivers/of/base.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d4bea3c..a8d7fed 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -286,8 +286,8 @@ struct device_node *of_find_all_nodes(struct device_node *prev)
> raw_spin_lock_irqsave(&devtree_lock, flags);
> np = __of_find_all_nodes(prev);
> of_node_get(np);
> - of_node_put(prev);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(prev);
> return np;
> }
> EXPORT_SYMBOL(of_find_all_nodes);
< snip >
--
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] devicetree: of_node_put() does not require holding devtree_lock
[not found] ` <1487755758-6066-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-02-26 21:08 ` Frank Rowand
@ 2017-02-27 14:54 ` Frank Rowand
[not found] ` <58B43DB6.4040009-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
1 sibling, 1 reply; 7+ messages in thread
From: Frank Rowand @ 2017-02-27 14:54 UTC (permalink / raw)
To: Sakari Ailus, devicetree-u79uwXL29TY76Z2rM5mHXA; +Cc: Rob Herring, Frank Rowand
On 02/22/17 01:29, Sakari Ailus wrote:
> While holding a reference to a device_node it is allowed to put that node
> without holding devtree_lock spinlock. Move of_node_put() after releasing
> the spinlock.
>
> Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
I would rather leave this code the way it is.
The change is a micro-optimization that is not going to have any real
impact on performance.
The change also makes the code less clear and readable. (Not
significantly, but very slightly). THe code in the current form
slightly emphasizes the balancing of gets and puts. I know that
this is nit picking, but so be it.
Thus the change is on balance code churn.
-Frank
> ---
> drivers/of/base.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d4bea3c..a8d7fed 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -286,8 +286,8 @@ struct device_node *of_find_all_nodes(struct device_node *prev)
> raw_spin_lock_irqsave(&devtree_lock, flags);
> np = __of_find_all_nodes(prev);
> of_node_get(np);
> - of_node_put(prev);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(prev);
> return np;
> }
> EXPORT_SYMBOL(of_find_all_nodes);
> @@ -660,8 +660,8 @@ struct device_node *of_get_next_parent(struct device_node *node)
>
> raw_spin_lock_irqsave(&devtree_lock, flags);
> parent = of_node_get(node->parent);
> - of_node_put(node);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(node);
> return parent;
> }
> EXPORT_SYMBOL(of_get_next_parent);
> @@ -732,8 +732,8 @@ struct device_node *of_get_next_available_child(const struct device_node *node,
> if (of_node_get(next))
> break;
> }
> - of_node_put(prev);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(prev);
> return next;
> }
> EXPORT_SYMBOL(of_get_next_available_child);
> @@ -875,8 +875,8 @@ struct device_node *of_find_node_by_name(struct device_node *from,
> if (np->name && (of_node_cmp(np->name, name) == 0)
> && of_node_get(np))
> break;
> - of_node_put(from);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(from);
> return np;
> }
> EXPORT_SYMBOL(of_find_node_by_name);
> @@ -904,8 +904,8 @@ struct device_node *of_find_node_by_type(struct device_node *from,
> if (np->type && (of_node_cmp(np->type, type) == 0)
> && of_node_get(np))
> break;
> - of_node_put(from);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(from);
> return np;
> }
> EXPORT_SYMBOL(of_find_node_by_type);
> @@ -935,8 +935,8 @@ struct device_node *of_find_compatible_node(struct device_node *from,
> if (__of_device_is_compatible(np, compatible, type, NULL) &&
> of_node_get(np))
> break;
> - of_node_put(from);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(from);
> return np;
> }
> EXPORT_SYMBOL(of_find_compatible_node);
> @@ -970,8 +970,8 @@ struct device_node *of_find_node_with_property(struct device_node *from,
> }
> }
> out:
> - of_node_put(from);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(from);
> return np;
> }
> EXPORT_SYMBOL(of_find_node_with_property);
> @@ -1051,8 +1051,8 @@ struct device_node *of_find_matching_node_and_match(struct device_node *from,
> break;
> }
> }
> - of_node_put(from);
> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> + of_node_put(from);
> return np;
> }
> EXPORT_SYMBOL(of_find_matching_node_and_match);
>
--
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-02-28 6:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-22 9:29 [PATCH 1/1] devicetree: of_node_put() does not require holding devtree_lock Sakari Ailus
2017-02-23 16:02 ` Rob Herring
[not found] ` <1487755758-6066-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-02-26 21:08 ` Frank Rowand
[not found] ` <58B343C6.9060604-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-27 10:08 ` Sakari Ailus
2017-02-27 14:54 ` Frank Rowand
[not found] ` <58B43DB6.4040009-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-28 0:42 ` Rob Herring
[not found] ` <CAL_JsqJyMqaaMDU81Jes-M8jvxewJnyFD33pqd9NUCZFa0HYcg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-28 6:36 ` Sakari Ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).