Commits
Angela Stegmaier committed fd3936494fe
rpmsg: fix lockdep warnings in virtio rpmsg bus driver
The virtio rpmsg bus framework uses endpoints as the basis for
sending and receiving messages to/from a remote processor. Each
rpmsg bus device will have a primary endpoint if the corresponding
rpmsg bus driver supports a callback, and secondary child endpoints
associated with the same rpmsg bus device. The life-cycle of these
endpoints are tied to the corresponding rpmsg device. A virtio rpmsg
bus device can also have its own endpoint for supporting name service
announcements from a corresponding remote processor to create and
delete rpmsg devices dynamically.
Each endpoint has a callback lock associated with it to provide
protection/mutual exclusion between threads that process incoming
rpmsg messages and threads that want to delete the endpoint. The
virtio rpmsg name service endpoint callback will run while holding
it's ept->cb_lock to create/delete rpmsg devices for RPMSG_NS_CREATE
and RPMSG_NS_DELETE messages respectively. The latter message
processing will destroy the requested channel, and will ultimately
result in all the secondary rpmsg device endpoints also to be
destroyed. The ept->cb_lock for the channel's endpoint is also
locked during its destruction while setting the callback to NULL.
This results in a seemingly nested locking of the ept->cb_lock even
though the locking is on different mutexes. This will result in a
false warning from the lockdep validator when it is enabled because
the lockdep deals with classes and both are the same class, although
they are different instances.
Similar circular dependency scenarios also exist with remoteproc
error recovery and existing rpmsg drivers - rpmsg_rpc and rpmsg_proto.
These issues are fixed by replacing the existing mutex_lock() calls
with the mutex_lock_nested() API variation and using different
subclasses for the NameService end-point and for the rpmsg channel
device end-points.
Following are example warning signatures that get fixed by this patch:
1. Recursive locking dependency during RPMSG_NS_DESTROY message processing
=============================================
WARNING: possible recursive locking detected
---------------------------------------------
kworker/0:1/23 is trying to acquire lock:
9fded1aa (&ept->cb_lock){+.+.}, at: __rpmsg_destroy_ept+0x40/0x6c [virtio_rpmsg_bus]
but task is already holding lock:
b33d53aa (&ept->cb_lock){+.+.}, at: rpmsg_recv_done+0x68/0x344 [virtio_rpmsg_bus]
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&ept->cb_lock);
lock(&ept->cb_lock);
*** DEADLOCK ***
May be due to missing lock nesting notatio
2. Circular locking dependency during error recovery of rpmsg-rpc driver
======================================================
WARNING: possible circular locking dependency detected
-------------------------------------------------------
kworker/0:2/69 is trying to acquire lock:
2e551f56 (&ept->cb_lock){+.+.}, at: __rpmsg_destroy_ept+0x40/0x6c [virtio_rpmsg_bus]
but task is already holding lock:
a034cfcf (&rppcdev->lock){+.+.}, at: rppc_remove+0x8c/0x244 [rpmsg_rpc]
which lock already depends on the new lock.
other info that might help us debug this:
Chain exists of:
&ept->cb_lock --> rppc_devices_lock --> &rppcdev->lock
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&rppcdev->lock);
lock(rppc_devices_lock);
lock(&rppcdev->lock);
lock(&ept->cb_lock);
*** DEADLOCK ***
3. Circular locking dependency during error recovery of rpmsg-proto driver
======================================================
WARNING: possible circular locking dependency detected
-------------------------------------------------------
kworker/0:1/23 is trying to acquire lock:
b012b19d (&ept->cb_lock){+.+.}, at: __rpmsg_destroy_ept+0x40/0x6c [virtio_rpmsg_bus]
but task is already holding lock:
d88d2852 (rpmsg_channels_lock){+.+.}, at: rpmsg_proto_remove+0x28/0x170 [rpmsg_proto]
which lock already depends on the new lock.
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(rpmsg_channels_lock);
lock(&ept->cb_lock);
lock(rpmsg_channels_lock);
lock(&ept->cb_lock);
*** DEADLOCK ***
Signed-off-by: Angela Stegmaier <angelabaker@ti.com>
[s-anna@ti.com: flip the subclass values, update patch description for 4.19]
Signed-off-by: Suman Anna <s-anna@ti.com>