Re: v2.6.25: WARNING: at kernel/lockdep.c:2437 __lock_acquire+0xc69/0xfa0()

Re: v2.6.25: WARNING: at kernel/lockdep.c:2437 __lock_acquire+0xc69/0xfa0()

[Tejun Heo]

[-- Attachment #1: Type: text/plain, Size: 2564 bytes --]

Peter Zijlstra wrote:
> On Sun, 2008-04-20 at 12:29 +0200, Vegard Nossum wrote:
>> Hi,
>>
>> This appears in clean v2.6.25:
>>
>> ------------[ cut here ]------------
>> WARNING: at kernel/lockdep.c:2437 __lock_acquire+0xc69/0xfa0()
>> Pid: 1002, comm: scsi_eh_4 Not tainted 2.6.25 #379
>>  [<c011bb5c>] warn_on_slowpath+0x3c/0x50
>>  [<c013b0f9>] ? __lock_acquire+0x219/0xfa0
>>  [<c013b0f9>] ? __lock_acquire+0x219/0xfa0
>>  [<c012c2c8>] ? __kernel_text_address+0x8/0x20
>>  [<c0104fce>] ? dump_trace+0xce/0x120
>>  [<c010a56c>] ? save_stack_trace+0x1c/0x40
>>  [<c0138659>] ? save_trace+0x39/0xa0
>>  [<c013a3d1>] ? mark_lock+0x61/0x540
>>  [<c013bb49>] __lock_acquire+0xc69/0xfa0
>>  [<c012b46a>] ? __cancel_work_timer+0x5a/0x140
>>  [<c01082ac>] ? native_sched_clock+0x5c/0xb0
>>  [<c013beee>] lock_acquire+0x6e/0x90
>>  [<c02a8e23>] ? ata_scsi_error+0x33/0x6a0
>>  [<c02e4946>] _spin_lock_irqsave+0x26/0x60
>>  [<c02a8e23>] ? ata_scsi_error+0x33/0x6a0
>>  [<c02a4ee0>] ? ata_bmdma_error_handler+0x0/0x50
>>  [<c02a8e23>] ata_scsi_error+0x33/0x6a0
>>  [<c02e249a>] ? schedule+0x27a/0x410
>>  [<c0256dd1>] ? __scsi_iterate_devices+0x51/0x70
>>  [<c025c681>] ? scsi_run_host_queues+0x21/0x30
>>  [<c025ae5e>] scsi_error_handler+0x9e/0x4e0
>>  [<c02e249a>] ? schedule+0x27a/0x410
>>  [<c025adc0>] ? scsi_error_handler+0x0/0x4e0
>>  [<c012dd86>] kthread+0x36/0x60
>>  [<c012dd50>] ? kthread+0x0/0x60
>>  [<c0104c27>] kernel_thread_helper+0x7/0x10
>>  =======================
>> ---[ end trace 5e5d737e76fd4940 ]---
> 
> Indeed, quite easy to reproduce:
> 
> diff --git a/kernel/lockdep.c b/kernel/lockdep.c
> index 81a4e4a..33901ef 100644
> --- a/kernel/lockdep.c
> +++ b/kernel/lockdep.c
> @@ -2434,6 +2434,13 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
>  	 * the hash, not class->key.
>  	 */
>  	id = class - lock_classes;
> +	if (id >= MAX_LOCKDEP_KEYS) {
> +		spinlock_t *slock = container_of(lock, spinlock_t, dep_map);
> +
> +		printk(KERN_EMERG "magic: %x\n", slock->magic);
> +		printk(KERN_EMERG "class: %p\n", class);
> +		printk(KERN_EMERG "id: %u\n", id);
> +	}
>  	if (DEBUG_LOCKS_WARN_ON(id >= MAX_LOCKDEP_KEYS))
>  		return 0;
>  
> 
> 
> gives me:
> 
> ata3: PATA max PIO4 cmd 0x1e8 ctl 0x3ee irq 11
> magic: c050cf60
> class: c7c20e54
> id: 2265185251
> ------------[ cut here ]------------
> WARNING: at /mnt/md0/src/linux-2.6-2/kernel/lockdep.c:2444
> __lock_acquire+0x57b/0xfd0()
> 
> 
> Looks like someone is stomping on the spinlock or just passing us
> garbage.

Does the attached patch help?

-- 
tejun

[-- Attachment #2: legacy-fix.patch --]
[-- Type: text/x-patch, Size: 350 bytes --]

diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index 7af4b29..f7ba234 100644
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -1037,6 +1037,7 @@ static __init int legacy_init_one(struct legacy_probe *probe)
 			return 0;
 		}
 	}
+	ata_host_detach(host);
 fail:
 	platform_device_unregister(pdev);
 	return ret;

Re: v2.6.25: WARNING: at kernel/lockdep.c:2437 __lock_acquire+0xc69/0xfa0()

[Jeff Garzik]

Tejun Heo wrote:
> Peter Zijlstra wrote:
>> Indeed, quite easy to reproduce:
>>
>> diff --git a/kernel/lockdep.c b/kernel/lockdep.c
>> index 81a4e4a..33901ef 100644
>> --- a/kernel/lockdep.c
>> +++ b/kernel/lockdep.c
>> @@ -2434,6 +2434,13 @@ static int __lock_acquire(struct lockdep_map 
>> *lock, unsigned int subclass,
>>       * the hash, not class->key.
>>       */
>>      id = class - lock_classes;
>> +    if (id >= MAX_LOCKDEP_KEYS) {
>> +        spinlock_t *slock = container_of(lock, spinlock_t, dep_map);
>> +
>> +        printk(KERN_EMERG "magic: %x\n", slock->magic);
>> +        printk(KERN_EMERG "class: %p\n", class);
>> +        printk(KERN_EMERG "id: %u\n", id);
>> +    }
>>      if (DEBUG_LOCKS_WARN_ON(id >= MAX_LOCKDEP_KEYS))
>>          return 0;
>>  
>>
>>
>> gives me:
>>
>> ata3: PATA max PIO4 cmd 0x1e8 ctl 0x3ee irq 11
>> magic: c050cf60
>> class: c7c20e54
>> id: 2265185251
>> ------------[ cut here ]------------
>> WARNING: at /mnt/md0/src/linux-2.6-2/kernel/lockdep.c:2444
>> __lock_acquire+0x57b/0xfd0()
>>
>>
>> Looks like someone is stomping on the spinlock or just passing us
>> garbage.
> 
> Does the attached patch help?
> 
> diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
> index 7af4b29..f7ba234 100644
> --- a/drivers/ata/pata_legacy.c
> +++ b/drivers/ata/pata_legacy.c
> @@ -1037,6 +1037,7 @@ static __init int legacy_init_one(struct legacy_probe *probe)
>  			return 0;
>  		}
>  	}
> +	ata_host_detach(host);
>  fail:
>  	platform_device_unregister(pdev);
>  	return ret;


(going through old, unresolved email)

It looks like this might still be needed?  I never saw an "it works" 
response to this email.

	Jeff

Re: v2.6.25: WARNING: at kernel/lockdep.c:2437 __lock_acquire+0xc69/0xfa0()

[Tejun Heo]

Jeff Garzik wrote:
> (going through old, unresolved email)
> 
> It looks like this might still be needed?  I never saw an "it works"
> response to this email.

Indeed.  Peter Zijlstra, can you please test it?

-- 
tejun

[PATCH #upstream-fixes] pata_legacy: fix no device fail path

[Tejun Heo]

When pata_legacy can't detect any device, it unregisters the
platform_device and fails detection.  However, it forgets to detach
ata host triggering weird failures as the host later gets freed by
devres while still attached.  Fix it.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Peter Zijlstra <peterz@infradead.org>
---
Well, with or without test verification, this patch is correct.  Thanks.

 drivers/ata/pata_legacy.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index 3f830f0..0c6dde8 100644
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -1032,6 +1032,7 @@ static __init int legacy_init_one(struct legacy_probe *probe)
 			return 0;
 		}
 	}
+	ata_host_detach(host);
 fail:
 	platform_device_unregister(pdev);
 	return ret;

Re: [PATCH #upstream-fixes] pata_legacy: fix no device fail path

[Jeff Garzik]

Tejun Heo wrote:
> When pata_legacy can't detect any device, it unregisters the
> platform_device and fails detection.  However, it forgets to detach
> ata host triggering weird failures as the host later gets freed by
> devres while still attached.  Fix it.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Peter Zijlstra <peterz@infradead.org>
> ---
> Well, with or without test verification, this patch is correct.  Thanks.
> 
>  drivers/ata/pata_legacy.c |    1 +
>  1 file changed, 1 insertion(+)

applied