| Message ID | 20240325054403.592298-6-sboyd@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix a deadlock with clk_pm_runtime_get() | expand |
Hi, On Sun, Mar 24, 2024 at 10:44 PM Stephen Boyd <sboyd@kernel.org> wrote: > > Similar to the previous commit, we should make sure that all devices are > runtime resumed before printing the clk_summary through debugfs. Failure > to do so would result in a deadlock if the thread is resuming a device > to print clk state and that device is also runtime resuming in another > thread, e.g the screen is turning on and the display driver is starting > up. > > Fixes: 1bb294a7981c ("clk: Enable/Disable runtime PM for clk_summary") > Cc: Taniya Das <quic_tdas@quicinc.com> > Cc: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Stephen Boyd <sboyd@kernel.org> > --- > drivers/clk/clk.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) Shouldn't this also squash in a revert of commit 1bb294a7981c ("clk: Enable/Disable runtime PM for clk_summary")? As it is, clk_summary_show_subtree() is left with an extra/unnecessary clk_pm_runtime_get() / clk_pm_runtime_put(), right? Other than that, this looks good to me: Reviewed-by: Douglas Anderson <dianders@chromium.org>
Quoting Doug Anderson (2024-03-25 09:19:51) > Hi, > > On Sun, Mar 24, 2024 at 10:44 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > > Similar to the previous commit, we should make sure that all devices are > > runtime resumed before printing the clk_summary through debugfs. Failure > > to do so would result in a deadlock if the thread is resuming a device > > to print clk state and that device is also runtime resuming in another > > thread, e.g the screen is turning on and the display driver is starting > > up. > > > > Fixes: 1bb294a7981c ("clk: Enable/Disable runtime PM for clk_summary") > > Cc: Taniya Das <quic_tdas@quicinc.com> > > Cc: Douglas Anderson <dianders@chromium.org> > > Signed-off-by: Stephen Boyd <sboyd@kernel.org> > > --- > > drivers/clk/clk.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > Shouldn't this also squash in a revert of commit 1bb294a7981c ("clk: > Enable/Disable runtime PM for clk_summary")? As it is, > clk_summary_show_subtree() is left with an extra/unnecessary > clk_pm_runtime_get() / clk_pm_runtime_put(), right? Sure, it is superfluous now. I suppose it means we can remove clk_pm_runtime_get()/put() calls in clk_{disable,unprepare}_unused_subtree() as well. > > Other than that, this looks good to me: > > Reviewed-by: Douglas Anderson <dianders@chromium.org>
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 31998ca67b1e..10792599bec1 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3332,7 +3332,7 @@ static int clk_summary_show(struct seq_file *s, void *data) seq_puts(s, " clock count count count rate accuracy phase cycle enable consumer id\n"); seq_puts(s, "---------------------------------------------------------------------------------------------------------------------------------------------\n"); - + clk_pm_runtime_get_all(); clk_prepare_lock(); for (; *lists; lists++) @@ -3340,6 +3340,7 @@ static int clk_summary_show(struct seq_file *s, void *data) clk_summary_show_subtree(s, c, 0); clk_prepare_unlock(); + clk_pm_runtime_put_all(); return 0; } @@ -3389,6 +3390,8 @@ static int clk_dump_show(struct seq_file *s, void *data) struct hlist_head **lists = s->private; seq_putc(s, '{'); + + clk_pm_runtime_get_all(); clk_prepare_lock(); for (; *lists; lists++) { @@ -3401,6 +3404,7 @@ static int clk_dump_show(struct seq_file *s, void *data) } clk_prepare_unlock(); + clk_pm_runtime_put_all(); seq_puts(s, "}\n"); return 0;
Similar to the previous commit, we should make sure that all devices are runtime resumed before printing the clk_summary through debugfs. Failure to do so would result in a deadlock if the thread is resuming a device to print clk state and that device is also runtime resuming in another thread, e.g the screen is turning on and the display driver is starting up. Fixes: 1bb294a7981c ("clk: Enable/Disable runtime PM for clk_summary") Cc: Taniya Das <quic_tdas@quicinc.com> Cc: Douglas Anderson <dianders@chromium.org> Signed-off-by: Stephen Boyd <sboyd@kernel.org> --- drivers/clk/clk.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)