From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Michael Turquette <mturquette@baylibre.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>,
grahamr@codeaurora.org,
Peter De Schrijver <pdeschrijver@nvidia.com>,
Stephen Boyd <sboyd@kernel.org>,
Viresh Kumar <viresh.kumar@linaro.org>,
linux-clk <linux-clk@vger.kernel.org>,
Linux PM list <linux-pm@vger.kernel.org>,
Doug Anderson <dianders@chromium.org>,
tdas@codeaurora.org, Rajendra Nayak <rnayak@codeaurora.org>,
anischal@codeaurora.org,
Vincent Guittot <vincent.guittot@linaro.org>,
amit.kucheria@linaro.org, linux-clk-owner@vger.kernel.org
Subject: Re: [RFD] Voltage dependencies for clocks (DVFS)
Date: Tue, 25 Sep 2018 15:11:21 +0200 [thread overview]
Message-ID: <CAMuHMdU7J0dbK6cPrCoBtJQHARd9K2=x=LM-_11XEBO0my9mQA@mail.gmail.com> (raw)
In-Reply-To: <20180919180726.67076.48303@harbor.lan>
Hi Mike,
On Wed, Sep 19, 2018 at 8:16 PM Michael Turquette
<mturquette@baylibre.com> wrote:
> Quoting Geert Uytterhoeven (2018-09-19 00:05:59)
> > On Wed, Sep 19, 2018 at 1:01 AM Michael Turquette
> > <mturquette@baylibre.com> wrote:
> > > Quoting Ulf Hansson (2018-08-23 06:20:11)
> > > > Anyway, in regards to control the performance state for these clock
> > > > controller devices, to me it seems like there are no other way, but
> > > > explicitly allow clock drivers to call an "OPP API" to request a
> > > > performance state. Simply, because it's the clock driver that needs
> > > > the performance state for its device. Whether the "OPP API" is the
> > > > new, dev_pm_genpd_set_performance_state() or something not even
> > > > invented yet, is another question.
> > >
> > > I completely agree, with the exception that I don't think it will be an
> > > "OPP API" but instead I hope it will be some runtime pm performance api.
> > >
> > > > My conclusion so far is, that we seems to fall back to a potential
> > > > locking problem. In regards to that, I am wondering whether that is
> > > > actually more of hypothetical problem than a real problem for your
> > > > case.
> > >
> > > For reference, this is why we allow reentrancy into the clock framework.
> > > It is common that consumer A calls clk_set_rate to set clock X to a
> > > rate, but in order for clock X to acheive that rate the clock provider
> > > might need to call clk_set_rate on another clock. We support reentrancy
> > > for this type of case.
> > >
> > > The problem described by Graham seems analogous. There are times when a
> > > performance provider itself will need to adjust it's own performance (as
> > > consumed by some other parent provider). I'm under the impression that
> > > runtime pm allows reentrancy and genpd allows for nested genpds, so
> > > hopefully this should Just Work.
> >
> > BTW, from time to time, lockdep spews out warnings about genpd/clock
> > interaction. I believe they are false positives.
>
> I'm happy to take a look at a spew if you can capture it and tell me how
> to reproduce.
Unfortunately I cannot trigger it at will.
Just saw it again (on Renesas Salvator-XS with R-Car H3 ES2.0), so here's
the spew:
genpd_add_device: Add e6601000.crypto to always-on
======================================================
WARNING: possible circular locking dependency detected
4.19.0-rc5-arm64-renesas-03635-gf5e6b692aefccbe2 #55 Not tainted
------------------------------------------------------
swapper/0/1 is trying to acquire lock:
(____ptrval____) (prepare_lock){+.+.}, at: clk_prepare_lock+0x48/0xa8
but task is already holding lock:
(____ptrval____) (of_clk_mutex){+.+.}, at:
__of_clk_get_from_provider.part.23+0x3c/0x120
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #5 (of_clk_mutex){+.+.}:
__mutex_lock+0x70/0x7f8
mutex_lock_nested+0x1c/0x28
__of_clk_get_from_provider.part.23+0x3c/0x120
of_clk_get_from_provider+0x20/0x30
cpg_mssr_attach_dev+0xb0/0x1c0
genpd_add_device.constprop.12+0x94/0x270
__genpd_dev_pm_attach+0xb4/0x220
genpd_dev_pm_attach+0x64/0x70
dev_pm_domain_attach+0x1c/0x30
platform_drv_probe+0x38/0xa0
really_probe+0x1c4/0x298
driver_probe_device+0x58/0x100
__driver_attach+0xdc/0xe0
bus_for_each_dev+0x74/0xc8
driver_attach+0x20/0x28
bus_add_driver+0x1a4/0x210
driver_register+0x60/0x110
__platform_driver_register+0x40/0x48
sh_mobile_i2c_adap_init+0x18/0x20
do_one_initcall+0x180/0x36c
kernel_init_freeable+0x470/0x518
kernel_init+0x10/0x108
ret_from_fork+0x10/0x1c
-> #4 (&genpd->mlock){+.+.}:
__mutex_lock+0x70/0x7f8
mutex_lock_nested+0x1c/0x28
genpd_lock_mtx+0x14/0x20
genpd_runtime_resume+0xbc/0x240
__rpm_callback+0xdc/0x228
rpm_callback+0x20/0x80
rpm_resume+0x4e0/0x7b0
__pm_runtime_resume+0x50/0x78
usb_dmac_alloc_chan_resources+0x88/0xa0
dma_chan_get+0x28/0x80
find_candidate+0x108/0x1a8
__dma_request_channel+0x68/0xc8
usb_dmac_of_xlate+0x60/0x88
of_dma_request_slave_channel+0x174/0x280
dma_request_chan+0x38/0x1d8
usbhsf_dma_init.isra.8+0x64/0x128
usbhs_fifo_probe+0x70/0x148
usbhs_probe+0x2e4/0x670
platform_drv_probe+0x50/0xa0
really_probe+0x1c4/0x298
driver_probe_device+0x58/0x100
__driver_attach+0xdc/0xe0
bus_for_each_dev+0x74/0xc8
driver_attach+0x20/0x28
bus_add_driver+0x1a4/0x210
driver_register+0x60/0x110
__platform_driver_register+0x40/0x48
renesas_usbhs_driver_init+0x18/0x20
do_one_initcall+0x180/0x36c
kernel_init_freeable+0x470/0x518
kernel_init+0x10/0x108
ret_from_fork+0x10/0x1c
-> #3 (dma_list_mutex){+.+.}:
__mutex_lock+0x70/0x7f8
mutex_lock_nested+0x1c/0x28
__dma_request_channel+0x38/0xc8
usb_dmac_of_xlate+0x60/0x88
of_dma_request_slave_channel+0x174/0x280
dma_request_chan+0x38/0x1d8
usbhsf_dma_init.isra.8+0x64/0x128
usbhs_fifo_probe+0x70/0x148
usbhs_probe+0x2e4/0x670
platform_drv_probe+0x50/0xa0
really_probe+0x1c4/0x298
driver_probe_device+0x58/0x100
__driver_attach+0xdc/0xe0
bus_for_each_dev+0x74/0xc8
driver_attach+0x20/0x28
bus_add_driver+0x1a4/0x210
driver_register+0x60/0x110
__platform_driver_register+0x40/0x48
renesas_usbhs_driver_init+0x18/0x20
do_one_initcall+0x180/0x36c
kernel_init_freeable+0x470/0x518
kernel_init+0x10/0x108
ret_from_fork+0x10/0x1c
-> #2 (of_dma_lock){+.+.}:
__mutex_lock+0x70/0x7f8
mutex_lock_nested+0x1c/0x28
of_dma_request_slave_channel+0x130/0x280
dma_request_chan+0x38/0x1d8
rcar_i2c_master_xfer+0x154/0x480
__i2c_transfer+0x148/0x8e0
i2c_smbus_xfer_emulated+0x158/0x5b0
__i2c_smbus_xfer+0x17c/0x7f8
i2c_smbus_xfer+0x64/0x98
i2c_smbus_read_byte_data+0x40/0x70
cs2000_bset.isra.1+0x2c/0x68
__cs2000_set_rate.constprop.7+0x80/0x148
cs2000_probe+0xd4/0x260
i2c_device_probe+0x290/0x2b0
really_probe+0x1c4/0x298
driver_probe_device+0x58/0x100
__device_attach_driver+0x90/0xd0
bus_for_each_drv+0x64/0xc8
__device_attach+0xd8/0x130
device_initial_probe+0x10/0x18
bus_probe_device+0x98/0xa0
device_add+0x3f8/0x620
device_register+0x1c/0x28
i2c_new_device+0x12c/0x310
of_i2c_register_device+0x74/0x98
of_i2c_register_devices+0xa4/0x150
i2c_register_adapter+0x160/0x418
__i2c_add_numbered_adapter+0x60/0x98
i2c_add_adapter+0xa4/0xd0
i2c_add_numbered_adapter+0x24/0x30
rcar_i2c_probe+0x33c/0x450
platform_drv_probe+0x50/0xa0
really_probe+0x1c4/0x298
driver_probe_device+0x58/0x100
__driver_attach+0xdc/0xe0
bus_for_each_dev+0x74/0xc8
driver_attach+0x20/0x28
bus_add_driver+0x1a4/0x210
driver_register+0x60/0x110
__platform_driver_register+0x40/0x48
rcar_i2c_driver_init+0x18/0x20
do_one_initcall+0x180/0x36c
kernel_init_freeable+0x470/0x518
kernel_init+0x10/0x108
ret_from_fork+0x10/0x1c
-> #1 (i2c_register_adapter){+.+.}:
rt_mutex_lock_nested+0x2c/0x48
i2c_adapter_lock_bus+0x20/0x30
i2c_smbus_xfer+0x44/0x98
i2c_smbus_read_byte_data+0x40/0x70
cs2000_recalc_rate+0x38/0x90
clk_register+0x3b8/0x6e8
clk_hw_register+0xc/0x20
cs2000_probe+0x134/0x260
i2c_device_probe+0x290/0x2b0
really_probe+0x1c4/0x298
driver_probe_device+0x58/0x100
__device_attach_driver+0x90/0xd0
bus_for_each_drv+0x64/0xc8
__device_attach+0xd8/0x130
device_initial_probe+0x10/0x18
bus_probe_device+0x98/0xa0
device_add+0x3f8/0x620
device_register+0x1c/0x28
i2c_new_device+0x12c/0x310
of_i2c_register_device+0x74/0x98
of_i2c_register_devices+0xa4/0x150
i2c_register_adapter+0x160/0x418
__i2c_add_numbered_adapter+0x60/0x98
i2c_add_adapter+0xa4/0xd0
i2c_add_numbered_adapter+0x24/0x30
rcar_i2c_probe+0x33c/0x450
platform_drv_probe+0x50/0xa0
really_probe+0x1c4/0x298
driver_probe_device+0x58/0x100
__driver_attach+0xdc/0xe0
bus_for_each_dev+0x74/0xc8
driver_attach+0x20/0x28
bus_add_driver+0x1a4/0x210
driver_register+0x60/0x110
__platform_driver_register+0x40/0x48
rcar_i2c_driver_init+0x18/0x20
do_one_initcall+0x180/0x36c
kernel_init_freeable+0x470/0x518
kernel_init+0x10/0x108
ret_from_fork+0x10/0x1c
-> #0 (prepare_lock){+.+.}:
lock_acquire+0xc0/0x228
__mutex_lock+0x70/0x7f8
mutex_lock_nested+0x1c/0x28
clk_prepare_lock+0x48/0xa8
__clk_create_clk.part.21+0x68/0xa0
__of_clk_get_from_provider.part.23+0xd0/0x120
__of_clk_get_from_provider+0x10/0x20
__of_clk_get+0x88/0xa0
__of_clk_get_by_name+0xa4/0x130
clk_get+0x30/0x70
devm_clk_get+0x4c/0xa8
ccree_probe+0xa4/0x5f8
platform_drv_probe+0x50/0xa0
really_probe+0x1c4/0x298
driver_probe_device+0x58/0x100
__driver_attach+0xdc/0xe0
bus_for_each_dev+0x74/0xc8
driver_attach+0x20/0x28
bus_add_driver+0x1a4/0x210
driver_register+0x60/0x110
__platform_driver_register+0x40/0x48
ccree_init+0x24/0x2c
do_one_initcall+0x180/0x36c
kernel_init_freeable+0x470/0x518
kernel_init+0x10/0x108
ret_from_fork+0x10/0x1c
other info that might help us debug this:
Chain exists of:
prepare_lock --> &genpd->mlock --> of_clk_mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(of_clk_mutex);
lock(&genpd->mlock);
lock(of_clk_mutex);
lock(prepare_lock);
*** DEADLOCK ***
2 locks held by swapper/0/1:
#0: (____ptrval____) (&dev->mutex){....}, at: __driver_attach+0x5c/0xe0
#1: (____ptrval____) (of_clk_mutex){+.+.}, at:
__of_clk_get_from_provider.part.23+0x3c/0x120
stack backtrace:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.19.0-rc5-arm64-renesas-03635-gf5e6b692aefccbe2 #55
Hardware name: Renesas Salvator-X 2nd version board based on
r8a7795 ES2.0+ (DT)
Call trace:
dump_backtrace+0x0/0x1c8
show_stack+0x14/0x20
dump_stack+0xbc/0xf4
print_circular_bug.isra.19+0x1d4/0x2e8
__lock_acquire+0x1804/0x1868
lock_acquire+0xc0/0x228
__mutex_lock+0x70/0x7f8
mutex_lock_nested+0x1c/0x28
clk_prepare_lock+0x48/0xa8
__clk_create_clk.part.21+0x68/0xa0
__of_clk_get_from_provider.part.23+0xd0/0x120
__of_clk_get_from_provider+0x10/0x20
__of_clk_get+0x88/0xa0
__of_clk_get_by_name+0xa4/0x130
clk_get+0x30/0x70
devm_clk_get+0x4c/0xa8
ccree_probe+0xa4/0x5f8
platform_drv_probe+0x50/0xa0
really_probe+0x1c4/0x298
driver_probe_device+0x58/0x100
__driver_attach+0xdc/0xe0
bus_for_each_dev+0x74/0xc8
driver_attach+0x20/0x28
bus_add_driver+0x1a4/0x210
driver_register+0x60/0x110
__platform_driver_register+0x40/0x48
ccree_init+0x24/0x2c
do_one_initcall+0x180/0x36c
kernel_init_freeable+0x470/0x518
kernel_init+0x10/0x108
ret_from_fork+0x10/0x1c
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
next prev parent reply other threads:[~2018-09-25 13:11 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-18 20:44 [RFD] Voltage dependencies for clocks (DVFS) grahamr
2018-07-02 5:13 ` Michael Turquette
2018-07-04 6:55 ` Viresh Kumar
2018-07-04 12:50 ` Ulf Hansson
2018-07-04 12:54 ` Rafael J. Wysocki
2018-07-04 12:58 ` Ulf Hansson
2018-07-20 17:12 ` Stephen Boyd
2018-07-20 17:56 ` Michael Turquette
2018-07-24 23:13 ` Stephen Boyd
2018-07-25 5:51 ` Michael Turquette
2018-07-23 8:26 ` Peter De Schrijver
2018-07-24 23:04 ` Stephen Boyd
2018-07-25 5:44 ` Michael Turquette
2018-07-25 11:27 ` Peter De Schrijver
2018-07-25 18:40 ` Michael Turquette
2018-07-31 11:56 ` Ulf Hansson
2018-07-31 20:02 ` grahamr
2018-08-23 13:20 ` Ulf Hansson
2018-09-18 23:00 ` Michael Turquette
2018-09-19 7:05 ` Geert Uytterhoeven
2018-09-19 18:07 ` Michael Turquette
2018-09-25 13:11 ` Geert Uytterhoeven [this message]
2018-09-25 13:11 ` Geert Uytterhoeven
2018-09-25 21:26 ` grahamr
2018-09-25 21:26 ` grahamr
2018-10-01 19:00 ` Michael Turquette
2018-10-04 0:37 ` Graham Roff
2018-10-04 21:23 ` Michael Turquette
2018-09-18 17:25 ` Kevin Hilman
2018-08-03 23:05 ` Michael Turquette
2018-08-23 12:13 ` Ulf Hansson
2018-09-18 22:48 ` Michael Turquette
2018-07-31 10:35 ` Ulf Hansson
2018-08-03 21:11 ` Michael Turquette
2018-08-23 11:10 ` Ulf Hansson
2018-07-05 8:19 ` Peter De Schrijver
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMuHMdU7J0dbK6cPrCoBtJQHARd9K2=x=LM-_11XEBO0my9mQA@mail.gmail.com' \
--to=geert@linux-m68k.org \
--cc=amit.kucheria@linaro.org \
--cc=anischal@codeaurora.org \
--cc=dianders@chromium.org \
--cc=grahamr@codeaurora.org \
--cc=linux-clk-owner@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=pdeschrijver@nvidia.com \
--cc=rnayak@codeaurora.org \
--cc=sboyd@kernel.org \
--cc=tdas@codeaurora.org \
--cc=ulf.hansson@linaro.org \
--cc=vincent.guittot@linaro.org \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).