rpmsg: fix lockdep warnings in virtio rpmsg bus driver
authorAngela Stegmaier <angelabaker@ti.com>
Sat, 4 Mar 2017 22:29:19 +0000 (16:29 -0600)
committerSuman Anna <s-anna@ti.com>
Fri, 8 Mar 2019 02:02:56 +0000 (20:02 -0600)
commitfd3936494fe852f2d3603d2e125f3d5b26fff444
treeb8b4eac29fca53c2cd7853abcaad9bb24c119130
parente90fa6b23e178ca989653297669d6423d0631143
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>
drivers/rpmsg/virtio_rpmsg_bus.c
include/linux/rpmsg.h