Re: [PATCH 2/3] thermal/drivers/tegra: Remove get_trend function

[Dmitry Osipenko <dmitry.osipenko@collabora.com>]

On 6/28/22 21:43, Guenter Roeck wrote:
> On Tue, Jun 28, 2022 at 08:10:30AM -0700, Guenter Roeck wrote:
>> On Tue, Jun 28, 2022 at 02:44:31PM +0300, Dmitry Osipenko wrote:
>>> On 6/28/22 11:41, Daniel Lezcano wrote:
>>>>
>>>> Thierry, Dmitry,
>>>>
>>>> are fine with this patch?
>>>
>>> Seems should be good. I couldn't test it using recent the linux-next
>>> because of a lockup in LM90 driver. There were quite a lot of changes in
>>> LM90 recently, adding Guenter.
>>>
>>
>> Weird, I tested those changes to death with real hardware, and I don't
>> see a code path where the mutex can be left in blocked state unless the
>> underlying i2c driver locks up for some reason. What is the platform,
>> and can you point me to the devicetree file ? Also, is there anything
>> else lm90 or i2c related in the kernel log ?
>>
> 
> Follow-up question: I see that various Tegra systems use lm90 compatible
> chips, and the interrupt line is in general wired up. Can you check if
> you get lots of interrupts on that interrupt line ? Also, can you check
> what happens if you read hwmon attributes directly ?

The number of interrupt fires is okay. It's a Nexus 7 Tegra30 tablet
device that I'm using for the testing.

Today I enabled the lockdep and it immediately showed where the problem is:

======================================================
WARNING: possible circular locking dependency detected
5.19.0-rc4-next-20220628-00002-g94e5dbbe1c58-dirty #24 Not tainted
------------------------------------------------------
irq/91-lm90/130 is trying to acquire lock:
c27ba380 (&tz->lock){+.+.}-{3:3}, at: thermal_zone_device_update+0x2c/0x64

               but task is already holding lock:
c27b42c8 (&data->update_lock){+.+.}-{3:3}, at: lm90_irq_thread+0x2c/0x68

               which lock already depends on the new lock.


               the existing dependency chain (in reverse order) is:

               -> #1 (&data->update_lock){+.+.}-{3:3}:
       __mutex_lock+0x9c/0x984
       mutex_lock_nested+0x2c/0x34
       lm90_read+0x44/0x3e8
       hwmon_thermal_get_temp+0x58/0x8c
       of_thermal_get_temp+0x38/0x44
       thermal_zone_get_temp+0x5c/0x7c
       thermal_zone_device_update.part.0+0x48/0x5fc
       thermal_zone_device_set_mode+0xa0/0xe4
       thermal_zone_device_enable+0x1c/0x20
       thermal_zone_of_sensor_register+0x18c/0x19c
       devm_thermal_zone_of_sensor_register+0x68/0xa4
       __hwmon_device_register+0x704/0x91c
       hwmon_device_register_with_info+0x6c/0x80
       devm_hwmon_device_register_with_info+0x78/0xb4
       lm90_probe+0x618/0x8c0
       i2c_device_probe+0x170/0x2e0
       really_probe+0xd8/0x300
       __driver_probe_device+0x94/0xf4
       driver_probe_device+0x40/0x118
       __device_attach_driver+0xc8/0x10c
       bus_for_each_drv+0x90/0xdc
       __device_attach+0xbc/0x1d4
       device_initial_probe+0x1c/0x20
       bus_probe_device+0x98/0xa0
       deferred_probe_work_func+0x8c/0xbc
       process_one_work+0x2b8/0x774
       worker_thread+0x17c/0x56c
       kthread+0x108/0x13c
       ret_from_fork+0x14/0x28
       0x0

               -> #0 (&tz->lock){+.+.}-{3:3}:
       __lock_acquire+0x173c/0x3198
       lock_acquire+0x128/0x3f0
       __mutex_lock+0x9c/0x984
       mutex_lock_nested+0x2c/0x34
       thermal_zone_device_update+0x2c/0x64
       hwmon_notify_event+0x128/0x138
       lm90_update_alarms_locked+0x35c/0x3b8
       lm90_irq_thread+0x38/0x68
       irq_thread_fn+0x2c/0x8c
       irq_thread+0x190/0x29c
       kthread+0x108/0x13c
       ret_from_fork+0x14/0x28
       0x0

               other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&data->update_lock);
                               lock(&tz->lock);
                               lock(&data->update_lock);
  lock(&tz->lock);

                *** DEADLOCK ***

1 lock held by irq/91-lm90/130:
 #0: c27b42c8 (&data->update_lock){+.+.}-{3:3}, at:
lm90_irq_thread+0x2c/0x68

               stack backtrace:
CPU: 1 PID: 130 Comm: irq/91-lm90 Not tainted
5.19.0-rc4-next-20220628-00002-g94e5dbbe1c58-dirty #24
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
Backtrace:
 dump_backtrace from show_stack+0x20/0x24
 r7:c33d1b60 r6:00000080 r5:60000093 r4:c168c6a4
 show_stack from dump_stack_lvl+0x68/0x98
 dump_stack_lvl from dump_stack+0x18/0x1c
 r7:c33d1b60 r6:c20328cc r5:c203c700 r4:c20328cc
 dump_stack from print_circular_bug+0x2ec/0x33c
 print_circular_bug from check_noncircular+0x104/0x168
 r10:c1a14cc8 r9:c33d1240 r8:00000001 r7:00000000 r6:dfc3dcc0 r5:c33d1b60
 r4:c33d1b80
 check_noncircular from __lock_acquire+0x173c/0x3198
 r7:c33d1b80 r6:c202bc98 r5:c33d1b60 r4:c21d92ac
 __lock_acquire from lock_acquire+0x128/0x3f0
 r10:60000013 r9:00000000 r8:00000000 r7:00000000 r6:dfc3dd40 r5:c19ac688
 r4:c19ac688
 lock_acquire from __mutex_lock+0x9c/0x984
 r10:c27ba380 r9:00000000 r8:c21d92ac r7:c33d1240 r6:00000000 r5:00000000
 r4:c27ba348
 __mutex_lock from mutex_lock_nested+0x2c/0x34
 r10:c27b4000 r9:00000000 r8:dfc3de87 r7:00000000 r6:c27ba348 r5:00000000
 r4:c27ba000
 mutex_lock_nested from thermal_zone_device_update+0x2c/0x64
 thermal_zone_device_update from hwmon_notify_event+0x128/0x138
 r7:00000000 r6:00000000 r5:c2d23ea4 r4:c33fd040
 hwmon_notify_event from lm90_update_alarms_locked+0x35c/0x3b8
 r8:c27b4378 r7:c2d23c08 r6:00000020 r5:00000000 r4:c27b4240
 lm90_update_alarms_locked from lm90_irq_thread+0x38/0x68
 r9:c01c2814 r8:00000001 r7:c33d2240 r6:c27b4290 r5:c27b4240 r4:c33fc200
 lm90_irq_thread from irq_thread_fn+0x2c/0x8c
 r7:c33d2240 r6:c27b4000 r5:c33d1240 r4:c33fc200
 irq_thread_fn from irq_thread+0x190/0x29c
 r7:c33d2240 r6:c33fc224 r5:c33d1240 r4:00000000
 irq_thread from kthread+0x108/0x13c
 r10:00000000 r9:df9ddbf4 r8:c31d2200 r7:c33fc200 r6:c01c2710 r5:c33d1240
 r4:c33fc240
 kthread from ret_from_fork+0x14/0x28

-- 
Best regards,
Dmitry