Re: 2.6.30-rc1: invalid opcode with call trace

[Vegard Nossum <vegard.nossum@gmail.com>]

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

2009/4/8 Ingo Molnar <mingo@elte.hu>:
>
> * Jens Axboe <jens.axboe@oracle.com> wrote:
>
>> On Wed, Apr 08 2009, Vegard Nossum wrote:

>> > Would you please try this patch? It has the same symptoms as a few
>> > other reports, only that this is 32-bit (and that makes it a bit
>> > different).
>> >
>> > http://marc.info/?l=linux-kernel&m=123909566829773&w=2
>> >
>> > I think Len Brown has applied it to the ACPI tree already.
>>
>> Works for me!
>
> My 'boot hang' problem is independent of that bug i think.

I agree.

The problem is that you have two async port probes:

[   24.177306] calling  1_async_port_probe+0x0/0xaa @ 2841
[   24.177825] calling  2_async_port_probe+0x0/0xaa @ 2842

of which only the first completes, because the first async call itself
tries to flush the async list while holding a lock (the
&shost->scan_mutex in __scsi_add_device), causing deadlock.

In short, I don't think we should call async_synchronize_full() from
scsi_complete_async_scans() at all. I'm including a more detailed
description/justification in the patch (attached).

Arjan, can you comment?


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036

[-- Attachment #2: 0001-scsi-don-t-wait-for-async-operations-in-scsi_comple.patch --]
[-- Type: application/octet-stream, Size: 1759 bytes --]

From bd12de4e40743d94eb90caad7c49d05e4a045b1e Mon Sep 17 00:00:00 2001
From: Vegard Nossum <vegard.nossum@gmail.com>
Date: Wed, 8 Apr 2009 18:05:43 +0200
Subject: [PATCH] scsi: don't wait for async operations in scsi_complete_async_scans()

scsi_complete_async_scans() cannot call async_synchronize_full() because
it may hold the &shost->scan_mutex (in __scsi_add_device(), for example).

I think it was an error to introduce the call to async_synchronize_full()
in the first place, because according to scsi_complete_async_scans(), we
should only wait for those hosts which have started scanning, and not
all of them:

/**
 * scsi_complete_async_scans - Wait for asynchronous scans to complete
 *
 * When this function returns, any host which started scanning before
 * this function was called will have finished its scan.  Hosts which
 * started scanning after this function was called may or may not have
 * finished.
 */

Those async calls which have not even started will not count as having
started by the scsi code either, so even without this call, we are
entirely within the specification of what the function is supposed to
do.

This should fix the deadlock and not introduce any regressions.

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Vegard Nossum <vegard.nossum@gmail.com>
---
 drivers/scsi/scsi_scan.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index a14d245..6f51ca4 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -180,8 +180,6 @@ int scsi_complete_async_scans(void)
 	spin_unlock(&async_scan_lock);
 
 	kfree(data);
-	/* Synchronize async operations globally */
-	async_synchronize_full();
 	return 0;
 }
 
-- 
1.6.0.6