[RFC] of: overlay: update phandle cache on overlay apply and remove
diff mbox series

Message ID 1529251416-14814-1-git-send-email-frowand.list@gmail.com
State New, archived
Headers show
Series
  • [RFC] of: overlay: update phandle cache on overlay apply and remove
Related show

Commit Message

Frank Rowand June 17, 2018, 4:03 p.m. UTC
From: Frank Rowand <frank.rowand@sony.com>

A comment in the review of the patch adding the phandle cache said that
the cache would have to be updated when modules are applied and removed.
This patch implements the cache updates.

Fixes: 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()")
Reported-by: Alan Tull <atull@kernel.org>
Suggested-by: Alan Tull <atull@kernel.org>
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---

Compiles for one configuration.
NOT boot tested.
Not run through my normal process to check for new warnings, etc.

It is late, I'm tired, my brain is fuzzy.  I need to review this more to have
any confidence in it.  But I wanted to get a version out for Alan to see (and
test if he wants).

 drivers/of/base.c       |  6 +++---
 drivers/of/of_private.h |  2 ++
 drivers/of/overlay.c    | 12 ++++++++++++
 3 files changed, 17 insertions(+), 3 deletions(-)

Comments

Frank Rowand June 21, 2018, 5:52 a.m. UTC | #1
On 06/20/18 11:23, Rob Herring wrote:
> On Sun, Jun 17, 2018 at 10:03 AM,  <frowand.list@gmail.com> wrote:
>> From: Frank Rowand <frank.rowand@sony.com>
>>
>> A comment in the review of the patch adding the phandle cache said that
>> the cache would have to be updated when modules are applied and removed.
>> This patch implements the cache updates.
>>
>> Fixes: 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()")
>> Reported-by: Alan Tull <atull@kernel.org>
>> Suggested-by: Alan Tull <atull@kernel.org>
>> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
>> ---
>>
>> Compiles for one configuration.
>> NOT boot tested.
>> Not run through my normal process to check for new warnings, etc.
> 
> I'm assuming you will resend a non-RFC version for me to apply.

Yes, I will.

> 
> I think it would be a bit better if callers didn't have to do free and
> populate themselves, but just made an invalidate call (like a normal
> cache) and re-populating the cache could happen on demand. Or if it
> was done as a single call, you could just copy the old entries to the
> new larger array. But maybe there would be a race condition in doing
> that? In any case, all that could be a subsequent patch.

Yes, the unspoken, underlying issue is a race condition.  I'll update
the commit comment to explain the race issues a little bit.  And maybe
add a code comment if I can be concise enough.

-Frank

> 
> Rob
>

Patch
diff mbox series

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 848f549164cd..466e3c8582f0 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -102,7 +102,7 @@  static u32 phandle_cache_mask;
  *   - the phandle lookup overhead reduction provided by the cache
  *     will likely be less
  */
-static void of_populate_phandle_cache(void)
+void of_populate_phandle_cache(void)
 {
 	unsigned long flags;
 	u32 cache_entries;
@@ -134,8 +134,7 @@  static void of_populate_phandle_cache(void)
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 }
 
-#ifndef CONFIG_MODULES
-static int __init of_free_phandle_cache(void)
+int of_free_phandle_cache(void)
 {
 	unsigned long flags;
 
@@ -148,6 +147,7 @@  static int __init of_free_phandle_cache(void)
 
 	return 0;
 }
+#if !defined(CONFIG_MODULES)
 late_initcall_sync(of_free_phandle_cache);
 #endif
 
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 891d780c076a..216175d11d3d 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -79,6 +79,8 @@  int of_resolve_phandles(struct device_node *tree);
 #if defined(CONFIG_OF_OVERLAY)
 void of_overlay_mutex_lock(void);
 void of_overlay_mutex_unlock(void);
+int of_free_phandle_cache(void);
+void of_populate_phandle_cache(void);
 #else
 static inline void of_overlay_mutex_lock(void) {};
 static inline void of_overlay_mutex_unlock(void) {};
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 7baa53e5b1d7..3f76e58fbec4 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -804,6 +804,8 @@  static int of_overlay_apply(const void *fdt, struct device_node *tree,
 		goto err_free_overlay_changeset;
 	}
 
+	of_populate_phandle_cache();
+
 	ret = __of_changeset_apply_notify(&ovcs->cset);
 	if (ret)
 		pr_err("overlay changeset entry notify error %d\n", ret);
@@ -1046,8 +1048,18 @@  int of_overlay_remove(int *ovcs_id)
 
 	list_del(&ovcs->ovcs_list);
 
+	/*
+	 * Empty and disable phandle cache.  Must empty here so that
+	 * changeset notifiers do not use stale cache entry for a removed
+	 * phandle.
+	 */
+	of_free_phandle_cache();
+
 	ret_apply = 0;
 	ret = __of_changeset_revert_entries(&ovcs->cset, &ret_apply);
+
+	of_populate_phandle_cache();
+
 	if (ret) {
 		if (ret_apply)
 			devicetree_state_flags |= DTSF_REVERT_FAIL;