| Message ID | 20220718133632.937290-2-windhl@126.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | [v2,1/2] OPP: Fix refcount leak bug for opp_table | expand |
On 18-07-22, 21:36, Liang He wrote: > In _opp_add_static_v2(), we need call of_node_get() for the new > reference created when "new_opp->np = np;" as new_opp is escaped out. > Here we should also take care of the of_node_put() when 'new_opp' is > freed based on the function description: "The opp can be controlled > ... and may be removed by dev_pm_opp_remove". > For example, _opp_add_static_v2() is called by _of_add_opp_table_v2() > in a for_each_available_child_of_node() which will automatically > increase and decrease the refcount. So we need an additional > of_node_get() for the new reference created in _opp_add_static_v2(). > > Signed-off-by: Liang He <windhl@126.com> Applied with few changes. Thanks. commit 3466ea2cd6b66e4647a9af2381c0d0cd3d579354 Author: Liang He <windhl@126.com> Date: Mon Jul 18 21:36:32 2022 +0800 OPP: Don't drop opp->np reference while it is still in use The struct dev_pm_opp contains a reference of the DT node, opp->np, throughout its lifetime. We should increase the refcount for the same from _opp_add_static_v2(), and drop it while removing the OPP finally. Signed-off-by: Liang He <windhl@126.com> [ Viresh: Updated subject / commit log, create _of_clear_opp() and drop reference from it] Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/opp/core.c | 2 +- drivers/opp/of.c | 12 +++++++++--- drivers/opp/opp.h | 6 ++---- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 4f4a285886fa..77d1ba3a4154 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1553,7 +1553,7 @@ static void _opp_kref_release(struct kref *kref) * frequency/voltage list. */ blocking_notifier_call_chain(&opp_table->head, OPP_EVENT_REMOVE, opp); - _of_opp_free_required_opps(opp_table, opp); + _of_clear_opp(opp_table, opp); opp_debug_remove_one(opp); kfree(opp); } diff --git a/drivers/opp/of.c b/drivers/opp/of.c index e4002b075422..7fa8263e38cb 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -254,8 +254,8 @@ void _of_clear_opp_table(struct opp_table *opp_table) * Release all resources previously acquired with a call to * _of_opp_alloc_required_opps(). */ -void _of_opp_free_required_opps(struct opp_table *opp_table, - struct dev_pm_opp *opp) +static void _of_opp_free_required_opps(struct opp_table *opp_table, + struct dev_pm_opp *opp) { struct dev_pm_opp **required_opps = opp->required_opps; int i; @@ -275,6 +275,12 @@ void _of_opp_free_required_opps(struct opp_table *opp_table, kfree(required_opps); } +void _of_clear_opp(struct opp_table *opp_table, struct dev_pm_opp *opp) +{ + _of_opp_free_required_opps(opp_table, opp); + of_node_put(opp->np); +} + /* Populate all required OPPs which are part of "required-opps" list */ static int _of_opp_alloc_required_opps(struct opp_table *opp_table, struct dev_pm_opp *opp) @@ -938,7 +944,7 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, new_opp->turbo = of_property_read_bool(np, "turbo-mode"); - new_opp->np = np; + new_opp->np = of_node_get(np); new_opp->dynamic = false; new_opp->available = true; diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index 816009eaafee..3a6e077df386 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -267,14 +267,12 @@ static inline bool lazy_linking_pending(struct opp_table *opp_table) void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index); void _of_clear_opp_table(struct opp_table *opp_table); struct opp_table *_managed_opp(struct device *dev, int index); -void _of_opp_free_required_opps(struct opp_table *opp_table, - struct dev_pm_opp *opp); +void _of_clear_opp(struct opp_table *opp_table, struct dev_pm_opp *opp); #else static inline void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) {} static inline void _of_clear_opp_table(struct opp_table *opp_table) {} static inline struct opp_table *_managed_opp(struct device *dev, int index) { return NULL; } -static inline void _of_opp_free_required_opps(struct opp_table *opp_table, - struct dev_pm_opp *opp) {} +static inline void _of_clear_opp(struct opp_table *opp_table, struct dev_pm_opp *opp) {} #endif #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index cc2671322e41..2f7988a05cb7 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1549,6 +1549,7 @@ static void _opp_kref_release(struct kref *kref) list_del(&opp->node); mutex_unlock(&opp_table->lock); + of_node_put(opp->np); /* * Notify the changes in the availability of the operable * frequency/voltage list. diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 374544afe9e4..ac4a3ff12677 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -937,7 +937,7 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, new_opp->turbo = of_property_read_bool(np, "turbo-mode"); - new_opp->np = np; + new_opp->np = of_node_get(np); new_opp->dynamic = false; new_opp->available = true;
In _opp_add_static_v2(), we need call of_node_get() for the new reference created when "new_opp->np = np;" as new_opp is escaped out. Here we should also take care of the of_node_put() when 'new_opp' is freed based on the function description: "The opp can be controlled ... and may be removed by dev_pm_opp_remove". For example, _opp_add_static_v2() is called by _of_add_opp_table_v2() in a for_each_available_child_of_node() which will automatically increase and decrease the refcount. So we need an additional of_node_get() for the new reference created in _opp_add_static_v2(). Signed-off-by: Liang He <windhl@126.com> --- drivers/opp/core.c | 1 + drivers/opp/of.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)