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

Mon Mar 22 15:19:12 UTC 2021

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 at 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