| Message ID | e0df59de670b48a923246fae1f972317b84b2764.1603785323.git.viresh.kumar@linaro.org |
|---|---|
| State | Accepted |
| Commit | e0df59de670b48a923246fae1f972317b84b2764 |
| Headers | show |
| Series | opp: Reduce the size of critical section in _opp_table_kref_release() | expand |
Quoting Viresh Kumar (2020-10-27 00:57:06) > There is a lot of stuff here which can be done outside of the big > opp_table_lock, do that. This helps avoiding few circular dependency > lockdeps around debugfs and interconnects. > > Reported-by: Rob Clark <robdclark@gmail.com> > Reported-by: Dmitry Osipenko <digetx@gmail.com> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- Any Fixes tag? Reviewed-by: Stephen Boyd <sboyd@kernel.org>
On 28-10-20, 16:32, Stephen Boyd wrote: > Quoting Viresh Kumar (2020-10-27 00:57:06) > > There is a lot of stuff here which can be done outside of the big > > opp_table_lock, do that. This helps avoiding few circular dependency > > lockdeps around debugfs and interconnects. > > > > Reported-by: Rob Clark <robdclark@gmail.com> > > Reported-by: Dmitry Osipenko <digetx@gmail.com> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > --- > > Any Fixes tag? The circular dependency lockdep appears with 5.11 stuff and so we don't need to fix it in any of the earlier releases. And so I didn't wanted to bother with the fixes tag, as the code had been like this for ever. > Reviewed-by: Stephen Boyd <sboyd@kernel.org> Thanks a lot. I was a bit worried about the crazy idea I had to solve this :) -- viresh
27.10.2020 10:57, Viresh Kumar пишет: > There is a lot of stuff here which can be done outside of the big > opp_table_lock, do that. This helps avoiding few circular dependency > lockdeps around debugfs and interconnects. > > Reported-by: Rob Clark <robdclark@gmail.com> > Reported-by: Dmitry Osipenko <digetx@gmail.com> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > Rob: I know this only fixes the issue partially for you and I am still > looking into that. I just wanted to get this merged in early as this > fixes lockdep for other users as well. > > drivers/opp/core.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index 4ac4e7ce6b8b..0e0a5269dc82 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -1181,6 +1181,10 @@ static void _opp_table_kref_release(struct kref *kref) > struct opp_device *opp_dev, *temp; > int i; > > + /* Drop the lock as soon as we can */ > + list_del(&opp_table->node); > + mutex_unlock(&opp_table_lock); > + > _of_clear_opp_table(opp_table); > > /* Release clk */ > @@ -1208,10 +1212,7 @@ static void _opp_table_kref_release(struct kref *kref) > > mutex_destroy(&opp_table->genpd_virt_dev_lock); > mutex_destroy(&opp_table->lock); > - list_del(&opp_table->node); > kfree(opp_table); > - > - mutex_unlock(&opp_table_lock); > } > > void dev_pm_opp_put_opp_table(struct opp_table *opp_table) > Fixes the NVIDIA Tegra devfreq driver lockup using WIP ICC patches, thanks. Tested-by: Dmitry Osipenko <digetx@gmail.com>
On 29-10-20, 09:40, Viresh Kumar wrote: > Thanks a lot. I was a bit worried about the crazy idea I had to solve > this :) Hmm, I thought this is the other patch where I had that crazy idea. This one was quite straight forward :)
Quoting Viresh Kumar (2020-10-29 21:20:00) > On 29-10-20, 09:40, Viresh Kumar wrote: > > Thanks a lot. I was a bit worried about the crazy idea I had to solve > > this :) > > Hmm, I thought this is the other patch where I had that crazy idea. > This one was quite straight forward :) > What's the other crazy idea patch?
On 30-10-20, 19:53, Stephen Boyd wrote: > Quoting Viresh Kumar (2020-10-29 21:20:00) > > On 29-10-20, 09:40, Viresh Kumar wrote: > > > Thanks a lot. I was a bit worried about the crazy idea I had to solve > > > this :) > > > > Hmm, I thought this is the other patch where I had that crazy idea. > > This one was quite straight forward :) > > > > What's the other crazy idea patch? Surprisingly I never sent it properly, just shared in another email thread with Rob Clark to test it out. Sent that now. -- viresh
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 4ac4e7ce6b8b..0e0a5269dc82 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1181,6 +1181,10 @@ static void _opp_table_kref_release(struct kref *kref) struct opp_device *opp_dev, *temp; int i; + /* Drop the lock as soon as we can */ + list_del(&opp_table->node); + mutex_unlock(&opp_table_lock); + _of_clear_opp_table(opp_table); /* Release clk */ @@ -1208,10 +1212,7 @@ static void _opp_table_kref_release(struct kref *kref) mutex_destroy(&opp_table->genpd_virt_dev_lock); mutex_destroy(&opp_table->lock); - list_del(&opp_table->node); kfree(opp_table); - - mutex_unlock(&opp_table_lock); } void dev_pm_opp_put_opp_table(struct opp_table *opp_table)
There is a lot of stuff here which can be done outside of the big opp_table_lock, do that. This helps avoiding few circular dependency lockdeps around debugfs and interconnects. Reported-by: Rob Clark <robdclark@gmail.com> Reported-by: Dmitry Osipenko <digetx@gmail.com> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- Rob: I know this only fixes the issue partially for you and I am still looking into that. I just wanted to get this merged in early as this fixes lockdep for other users as well. drivers/opp/core.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)