There are valid cases where we might get in here with an unexpected
refcount. One example are devlinks between suppliers <-> consumers.
When a link is created, it will grab a reference to both the supplier and
the consumer devices (which can, potentially, hold a reference to it's
of_node) and this reference is not synchronously dropped when unbinding the
device from the driver. Instead, a work item is queued (see
devlink_dev_release()). This async nature will make that
__of_changeset_entry_destroy() is reached with a refcount > 1. But,
eventually, all the refcounts are released and of_node_release() is
reached.
So, the best we can do is to just warn for a possible leak and "inform" the
node that the changeset is already gone so, if we do reach
of_node_release(), we can free the node instead assuming something bad
has happened.
Signed-off-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
---
drivers/of/dynamic.c | 31 +++++++++++++++++++++++++++----
include/linux/of.h | 1 +
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index e311d406b170..354e641f63df 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -360,8 +360,10 @@ void of_node_release(struct kobject *kobj)
return;
if (of_node_check_flag(node, OF_OVERLAY)) {
-
- if (!of_node_check_flag(node, OF_OVERLAY_FREE_CSET)) {
+ if (of_node_check_flag(node, OF_OVERLAY_CSET_GONE)) {
+ pr_info("INFO: node %s is now being freed after overlay changeset. All should be fine now...\n",
+ node->full_name);
+ } else if (!of_node_check_flag(node, OF_OVERLAY_FREE_CSET)) {
/* premature refcount of zero, do not free memory */
pr_err("ERROR: memory leak before free overlay changeset, %pOF\n",
node);
@@ -492,8 +494,29 @@ static void __of_changeset_entry_destroy(struct of_changeset_entry *ce)
if (ce->action == OF_RECONFIG_ATTACH_NODE &&
of_node_check_flag(ce->np, OF_OVERLAY)) {
if (kref_read(&ce->np->kobj.kref) > 1) {
- pr_err("ERROR: memory leak, expected refcount 1 instead of %d, of_node_get()/of_node_put() unbalanced - destroy cset entry: attach overlay node %pOF\n",
- kref_read(&ce->np->kobj.kref), ce->np);
+ pr_warn("WARNING: Possible memory leak, expected refcount 1 instead of %d, of_node_get()/of_node_put() unbalanced - destroy cset entry: attach overlay node %pOF\n",
+ kref_read(&ce->np->kobj.kref), ce->np);
+ /*
+ * There are valid cases where we might get in here with
+ * an unexpected refcount. One example are devlinks
+ * between suppliers <-> consumers. When a link is
+ * created, it will grab a reference to both the
+ * supplier and the consumer devices (which can,
+ * potentially, hold a reference to it's of_node) and
+ * this reference is not synchronously dropped when
+ * unbinding the device from the driver. Instead, a work
+ * item is queued (see devlink_dev_release()). This
+ * async nature will make that
+ * __of_changeset_entry_destroy() is reached with a
+ * refcount > 1. But, eventually, all the refcounts are
+ * released and of_node_release() is reached.
+ *
+ * So, the best we can do is to just warn for a possible
+ * leak and "inform" the node that the changeset is already
+ * gone so, if we do reach of_node_release(), we can free
+ * the node.
+ */
+ of_node_set_flag(ce->np, OF_OVERLAY_CSET_GONE);
} else {
of_node_set_flag(ce->np, OF_OVERLAY_FREE_CSET);
}
diff --git a/include/linux/of.h b/include/linux/of.h
index 6ecde0515677..4f562b3f601d 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -152,6 +152,7 @@ extern struct device_node *of_stdout;
#define OF_POPULATED_BUS 4 /* platform bus created for children */
#define OF_OVERLAY 5 /* allocated for an overlay */
#define OF_OVERLAY_FREE_CSET 6 /* in overlay cset being freed */
+#define OF_OVERLAY_CSET_GONE 7 /* overlay cset already freed */
#define OF_BAD_ADDR ((u64)-1)
--
2.40.1
