| Message ID | 20210812101353.14318-1-digetx@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] opp: Release current OPP properly | expand |
On 12-08-21, 13:13, Dmitry Osipenko wrote: > The current_opp is released only when whole OPP table is released, > otherwise it's only marked as removed by dev_pm_opp_remove_table(). > Functions like dev_pm_opp_put_clkname() and dev_pm_opp_put_supported_hw() > are checking whether OPP table is empty and it's not if current_opp is > set since it holds the refcount of OPP, this produces a noisy warning > from these functions about busy OPP table. Release current_opp when > OPP table is removed to fix it. > > Cc: stable@vger.kernel.org > Fixes: 81c4d8a3c414 ("opp: Keep track of currently programmed OPP") > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/opp/core.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index b335c077f215..73da968b5c86 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -1378,9 +1378,6 @@ static void _opp_table_kref_release(struct kref *kref) > list_del(&opp_table->node); > mutex_unlock(&opp_table_lock); > > - if (opp_table->current_opp) > - dev_pm_opp_put(opp_table->current_opp); > - > _of_clear_opp_table(opp_table); > > /* Release clk */ > @@ -2901,6 +2898,12 @@ void dev_pm_opp_remove_table(struct device *dev) > if (_opp_remove_all_static(opp_table)) > dev_pm_opp_put_opp_table(opp_table); > > + /* Drop reference taken by _find_current_opp() */ > + if (opp_table->current_opp) { > + dev_pm_opp_put(opp_table->current_opp); > + opp_table->current_opp = NULL; > + } > + It is better to drop the reference when the OPP table is really getting removed. I think the WARN_ON() in the put_* functions can be dropped. It is important to check this when the stuff is getting set, like in dev_pm_opp_set_supported_hw(), but removal is just fine. -- viresh
13.08.2021 07:12, Viresh Kumar пишет: > On 12-08-21, 13:13, Dmitry Osipenko wrote: >> The current_opp is released only when whole OPP table is released, >> otherwise it's only marked as removed by dev_pm_opp_remove_table(). >> Functions like dev_pm_opp_put_clkname() and dev_pm_opp_put_supported_hw() >> are checking whether OPP table is empty and it's not if current_opp is >> set since it holds the refcount of OPP, this produces a noisy warning >> from these functions about busy OPP table. Release current_opp when >> OPP table is removed to fix it. >> >> Cc: stable@vger.kernel.org >> Fixes: 81c4d8a3c414 ("opp: Keep track of currently programmed OPP") >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/opp/core.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/opp/core.c b/drivers/opp/core.c >> index b335c077f215..73da968b5c86 100644 >> --- a/drivers/opp/core.c >> +++ b/drivers/opp/core.c >> @@ -1378,9 +1378,6 @@ static void _opp_table_kref_release(struct kref *kref) >> list_del(&opp_table->node); >> mutex_unlock(&opp_table_lock); >> >> - if (opp_table->current_opp) >> - dev_pm_opp_put(opp_table->current_opp); >> - >> _of_clear_opp_table(opp_table); >> >> /* Release clk */ >> @@ -2901,6 +2898,12 @@ void dev_pm_opp_remove_table(struct device *dev) >> if (_opp_remove_all_static(opp_table)) >> dev_pm_opp_put_opp_table(opp_table); >> >> + /* Drop reference taken by _find_current_opp() */ >> + if (opp_table->current_opp) { >> + dev_pm_opp_put(opp_table->current_opp); >> + opp_table->current_opp = NULL; >> + } >> + > > It is better to drop the reference when the OPP table is really > getting removed. > > I think the WARN_ON() in the put_* functions can be dropped. It is > important to check this when the stuff is getting set, like in > dev_pm_opp_set_supported_hw(), but removal is just fine. > Alright, I'll make another patch then.
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index b335c077f215..73da968b5c86 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1378,9 +1378,6 @@ static void _opp_table_kref_release(struct kref *kref) list_del(&opp_table->node); mutex_unlock(&opp_table_lock); - if (opp_table->current_opp) - dev_pm_opp_put(opp_table->current_opp); - _of_clear_opp_table(opp_table); /* Release clk */ @@ -2901,6 +2898,12 @@ void dev_pm_opp_remove_table(struct device *dev) if (_opp_remove_all_static(opp_table)) dev_pm_opp_put_opp_table(opp_table); + /* Drop reference taken by _find_current_opp() */ + if (opp_table->current_opp) { + dev_pm_opp_put(opp_table->current_opp); + opp_table->current_opp = NULL; + } + /* Drop reference taken by _find_opp_table() */ dev_pm_opp_put_opp_table(opp_table); }
The current_opp is released only when whole OPP table is released, otherwise it's only marked as removed by dev_pm_opp_remove_table(). Functions like dev_pm_opp_put_clkname() and dev_pm_opp_put_supported_hw() are checking whether OPP table is empty and it's not if current_opp is set since it holds the refcount of OPP, this produces a noisy warning from these functions about busy OPP table. Release current_opp when OPP table is removed to fix it. Cc: stable@vger.kernel.org Fixes: 81c4d8a3c414 ("opp: Keep track of currently programmed OPP") Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/opp/core.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)