Clock framework deadlock with external SPI clockchip
Hi,
I'm currently facing a deadlock in the common clock framework that unfortunately is not addressed by the reentrancy patches. I have a external clock chip that is controlled via SPI. So for example to configure the rate of the clock chip you need to send a SPI message. Naturally the clock framework will hold the prepare lock while configuring the rate. Communication in the SPI framework happens asynchronously, spi_sync() will enqueue a message in the SPI masters queue and then wait using wait_for_completion(). The master will call complete() once the transfer has been finished. The SPI master runs in it's own thread in which it processes the messages. In this thread it also calls clk_set_rate() to configure the SPI transfer clock rate based on what the message says. Now the deadlock happens as we try to take the prepare_lock again and since the clock chip and the SPI master run in different threads the reentrancy code does not kick in.
The basic sequence is like this:
=== Clock chip driver === === SPI master driver ===
clk_prepare_lock()
spi_sync()
wait_for_completion(X)
clk_get_rate() clk_prepare_lock() <--- DEADLOCK clk_prepare_unlock() ... complete(X)
...
clk_prepare_unlock()
I'm wondering if you have any idea how this can be fixed. In my opinion we'd need a per clock mutex to address this properly.
Thanks,
- Lars
Quoted 28 lines · Lars-Peter Clausen
Hi,
I'm currently facing a deadlock in the common clock framework that unfortunately is not addressed by the reentrancy patches. I have a external clock chip that is controlled via SPI. So for example to configure the rate of the clock chip you need to send a SPI message. Naturally the clock framework will hold the prepare lock while configuring the rate. Communication in the SPI framework happens asynchronously,
spi_sync()will enqueue a message in the SPI masters queue and then wait usingwait_for_completion(). The master will callcomplete()once the transfer has been finished. The SPI master runs in it's own thread in which it processes the messages. In this thread it also callsclk_set_rate()to configure the SPI transfer clock rate based on what the message says. Now the deadlock happens as we try to take theprepare_lockagain and since the clock chip and the SPI master run in different threads the reentrancy code does not kick in.The basic sequence is like this:
=== Clock chip driver === === SPI master driver ===
clk_prepare_lock()
spi_sync()
wait_for_completion(X)clk_get_rate() clk_prepare_lock() <--- DEADLOCK clk_prepare_unlock() ... complete(X)
Quoted 5 lines
...
clk_prepare_unlock()I'm wondering if you have any idea how this can be fixed. In my opinion we'd need a per clock mutex to address this properly.
One workaround is to leave the SPI masters clock always prepared. A similar problem can occur with I2C and DVFS using notifiers.
Cheers,
Peter.
Quoted 32 lines · Peter De Schrijver
Quoted 31 lines · Lars-Peter Clausen
Hi,
I'm currently facing a deadlock in the common clock framework that unfortunately is not addressed by the reentrancy patches. I have a external clock chip that is controlled via SPI. So for example to configure the rate of the clock chip you need to send a SPI message. Naturally the clock framework will hold the prepare lock while configuring the rate. Communication in the SPI framework happens asynchronously,
spi_sync()will enqueue a message in the SPI masters queue and then wait usingwait_for_completion(). The master will callcomplete()once the transfer has been finished. The SPI master runs in it's own thread in which it processes the messages. In this thread it also callsclk_set_rate()to configure the SPI transfer clock rate based on what the message says. Now the deadlock happens as we try to take theprepare_lockagain and since the clock chip and the SPI master run in different threads the reentrancy code does not kick in.The basic sequence is like this:
=== Clock chip driver === === SPI master driver ===
clk_prepare_lock()
spi_sync()
wait_for_completion(X)clk_get_rate() clk_prepare_lock() <--- DEADLOCK clk_prepare_unlock() ... complete(X)...
clk_prepare_unlock()
Quoted 5 lines
Quoted 2 lines
I'm wondering if you have any idea how this can be fixed. In my opinion we'd need a per clock mutex to address this properly.
One workaround is to leave the SPI masters clock always prepared. A similar problem can occur with I2C and DVFS using notifiers.
That's kind of what I'm doing right now as a temporary solution. But we'd basically need to have all SPI or I2C master drivers to be aware that they might be used for controlling a external clock chip. And there are also situations where the workaround does not work. E.g. if the clk API is used to configure the rate of the SPI SCK signal, since the rate can be set by the spi message.
- Lars
Quoting Lars-Peter Clausen (2013-08-30 06:24:45)
Quoted 23 lines
Hi,
I'm currently facing a deadlock in the common clock framework that unfortunately is not addressed by the reentrancy patches. I have a external clock chip that is controlled via SPI. So for example to configure the rate of the clock chip you need to send a SPI message. Naturally the clock framework will hold the prepare lock while configuring the rate. Communication in the SPI framework happens asynchronously,
spi_sync()will enqueue a message in the SPI masters queue and then wait usingwait_for_completion(). The master will callcomplete()once the transfer has been finished. The SPI master runs in it's own thread in which it processes the messages. In this thread it also callsclk_set_rate()to configure the SPI transfer clock rate based on what the message says. Now the deadlock happens as we try to take theprepare_lockagain and since the clock chip and the SPI master run in different threads the reentrancy code does not kick in.The basic sequence is like this:
=== Clock chip driver === === SPI master driver ===
clk_prepare_lock()
spi_sync()
wait_for_completion(X)
Quoted 7 lines
clk_get_rate() clk_prepare_lock() <--- DEADLOCK clk_prepare_unlock() ... complete(X)...
clk_prepare_unlock()
Is there a synchronous equivalent to spi_sync()?
Quoted 3 lines
I'm wondering if you have any idea how this can be fixed. In my opinion we'd need a per clock mutex to address this properly.
Increasing the number of locks is a pain in the ass. Per-clock mutexes not only have a memory impact but in general nested locks do not play nice with lockdep (which assumes a depth no greater than 8 nested locks, which is often not enough for complex clock trees). And generating unique lock subclasses helps with this but makes for terrible, ugly code.
Another idea (which I have not thought about much) is to have async clk api's. I think there might be many uses for async clock api's besides just this case. Think about a GPU driver that scales the main clock feeding a GPU. Maybe the driver doesn't care to block on clk_set_rate which might block for 100mS if voltage scaling is involved. clk_set_rate_async() might preferable.
Regards,
Mike
Quoted 3 lines
Thanks,
- Lars
Quoted 8 lines · Mike Turquette
Quoting Lars-Peter Clausen (2013-08-30 06:24:45)
Quoted 4 lines
=== Clock chip driver === === SPI master driver ===
clk_prepare_lock()
spi_sync()
wait_for_completion(X)Is there a synchronous equivalent to
spi_sync()?
spi_sync() is synchronous? Obviously everything is asynchronous up to that point but the only way the driver can tell if the I/O completed is using spi_sync() or the equivalent thereof.
If you're asking if there is a way of doing SPI I/O that isn't multi
threaded then there isn't, in order to keep pushing transfers into the
SPI controller to saturate the bus we have a high priority task to push
more data in. SPI buses can be quite high bandwidth in some
applications.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130904/e2ce5315/attachment-0001.sig>