Re: [PATCH v5 01/21] gpu: host1x: Use different lock classes for each client

Mikko Perttunen
2021-03-22T15:20:09Z

On 22.3.2021 16.48, Dmitry Osipenko wrote:
> 22.03.2021 17:46, Thierry Reding пишет:
>> On Mon, Jan 11, 2021 at 02:59:59PM +0200, Mikko Perttunen wrote:
>>> To avoid false lockdep warnings, give each client lock a different
>>> lock class, passed from the initialization site by macro.
>>>
>>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>>> ---
>>> drivers/gpu/host1x/bus.c | 7 ++++---
>>> include/linux/host1x.h | 9 ++++++++-
>>> 2 files changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
>>> index 347fb962b6c9..8fc79e9cb652 100644
>>> --- a/drivers/gpu/host1x/bus.c
>>> +++ b/drivers/gpu/host1x/bus.c
>>> @@ -715,13 +715,14 @@ EXPORT_SYMBOL(host1x_driver_unregister);
>>> * device and call host1x_device_init(), which will in turn call each client's
>>> * &host1x_client_ops.init implementation.
>>> */
>>> -int host1x_client_register(struct host1x_client *client)
>>> +int __host1x_client_register(struct host1x_client *client,
>>> + struct lock_class_key *key)
>>
>> I've seen the kbuild robot warn about this because the kerneldoc is now
>> out of date.
>>
>>> {
>>> struct host1x *host1x;
>>> int err;
>>>
>>> INIT_LIST_HEAD(&client->list);
>>> - mutex_init(&client->lock);
>>> + __mutex_init(&client->lock, "host1x client lock", key);
>>
>> Should we maybe attempt to make this unique? Could we use something like
>> dev_name(client->dev) for this?
>
> I'm curious who the lockdep warning could be triggered at all, I don't
> recall ever seeing it. Mikko, could you please clarify how to reproduce
> the warning?
>

This is pretty difficult to read but I guess it's some interaction
related to the delayed initialization of host1x clients? In any case, I
consistently get it at boot (though it may be triggered by vic probe
instead of nvdec).

I'll fix the kbuild robot warnings and see if I can add a
client-specific lock name for v6.

Mikko

[ 38.128257] WARNING: possible recursive locking detected
[ 38.133567] 5.11.0-rc2-next-20210108+ #102 Tainted: G S
[ 38.140089] --------------------------------------------
[ 38.145395] systemd-udevd/239 is trying to acquire lock:
[ 38.150703] ffff0000997aa218 (&client->lock){+.+.}-{3:3}, at:
host1x_client_resume+0x30/0x100 [host1x]
[ 38.160142]
[ 38.160142] but task is already holding lock:
[ 38.165968] ffff000080c3b148 (&client->lock){+.+.}-{3:3}, at:
host1x_client_resume+0x30/0x100 [host1x]
[ 38.175398]
[ 38.175398] other info that might help us debug this:
[ 38.181918] Possible unsafe locking scenario:
[ 38.181918]
[ 38.187830] CPU0
[ 38.190275] ----
[ 38.192719] lock(&client->lock);
[ 38.196129] lock(&client->lock);
[ 38.199537]
[ 38.199537] *** DEADLOCK ***
[ 38.199537]
[ 38.205449] May be due to missing lock nesting notation
[ 38.205449]
[ 38.212228] 6 locks held by systemd-udevd/239:
[ 38.216669] #0: ffff00009261c188 (&dev->mutex){....}-{3:3}, at:
device_driver_attach+0x60/0x130
[ 38.225487] #1: ffff800009a17168 (devices_lock){+.+.}-{3:3}, at:
host1x_client_register+0x7c/0x220 [host1x]
[ 38.235441] #2: ffff000083f94bb8 (&host->devices_lock){+.+.}-{3:3},
at: host1x_client_register+0xac/0x220 [host1x]
[ 38.245996] #3: ffff0000a2267190 (&dev->mutex){....}-{3:3}, at:
__device_attach+0x8c/0x230
[ 38.254372] #4: ffff000092c880f0 (&wgrp->lock){+.+.}-{3:3}, at:
tegra_display_hub_prepare+0xd8/0x170 [tegra_drm]
[ 38.264788] #5: ffff000080c3b148 (&client->lock){+.+.}-{3:3}, at:
host1x_client_resume+0x30/0x100 [host1x]
[ 38.274658]
[ 38.274658] stack backtrace:
[ 38.279012] CPU: 0 PID: 239 Comm: systemd-udevd Tainted: G S
5.11.0-rc2-next-20210108+ #102
[ 38.288660] Hardware name: NVIDIA Jetson TX2 Developer Kit (DT)
[ 38.294577] Call trace:
[ 38.297022] dump_backtrace+0x0/0x2c0
[ 38.300695] show_stack+0x18/0x6c
[ 38.304013] dump_stack+0x120/0x19c
[ 38.307507] __lock_acquire+0x171c/0x2c34
[ 38.311521] lock_acquire.part.0+0x230/0x490
[ 38.315793] lock_acquire+0x70/0x90
[ 38.319285] __mutex_lock+0x11c/0x6d0
[ 38.322952] mutex_lock_nested+0x58/0x90
[ 38.326877] host1x_client_resume+0x30/0x100 [host1x]
[ 38.332047] host1x_client_resume+0x44/0x100 [host1x]
[ 38.337200] tegra_display_hub_prepare+0xf8/0x170 [tegra_drm]
[ 38.343084] host1x_drm_probe+0x1fc/0x4f0 [tegra_drm]
[ 38.348256] host1x_device_probe+0x3c/0x50 [host1x]
[ 38.353240] really_probe+0x148/0x6f0
[ 38.356906] driver_probe_device+0x78/0xe4
[ 38.361005] __device_attach_driver+0x10c/0x170
[ 38.365536] bus_for_each_drv+0xf0/0x160
[ 38.369461] __device_attach+0x168/0x230
[ 38.373385] device_initial_probe+0x14/0x20
[ 38.377571] bus_probe_device+0xec/0x100
[ 38.381494] device_add+0x580/0xbcc
[ 38.384985] host1x_subdev_register+0x178/0x1cc [host1x]
[ 38.390397] host1x_client_register+0x138/0x220 [host1x]
[ 38.395808] nvdec_probe+0x240/0x3ec [tegra_drm]
[ 38.400549] platform_probe+0x8c/0x110
[ 38.404302] really_probe+0x148/0x6f0
[ 38.407966] driver_probe_device+0x78/0xe4
[ 38.412065] device_driver_attach+0x120/0x130
[ 38.416423] __driver_attach+0xb4/0x190
[ 38.420261] bus_for_each_dev+0xe8/0x160
[ 38.424185] driver_attach+0x34/0x44
[ 38.427761] bus_add_driver+0x1a4/0x2b0
[ 38.431598] driver_register+0xe0/0x210
[ 38.435437] __platform_register_drivers+0x6c/0x104
[ 38.440318] host1x_drm_init+0x54/0x1000 [tegra_drm]
[ 38.445405] do_one_initcall+0xec/0x5e0
[ 38.449244] do_init_module+0xe0/0x384
[ 38.453000] load_module+0x32d8/0x3c60