RE: kexec and aacraid broken

Salyzyn, Mark
2007-05-30T11:44:16Z

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

I believe this issue is a result of the aacraid_commit_reset patch (as
posted for scsi-misc-2.6, enclosed to permit testing) not yet propagated
to the 2.6.22-rc3 tree.

This is the adapter taking longer than 3 minutes to start after a reset.
I seriously doubt either of these patches suggested below will have an
affect. And if they do, they are not root cause, one reduces the chances
that the card will be reset during initialization (thus applied would
likely mitigate this problem), the other prevents a panic when the
Adapter is reset (removed, would result in dogs and cats sleeping with
each other).

Please use kernel parameter aacraid.startup_timeout=540 (merely larger
than the default 180 seconds) when spawning the kexec or see if the
aacraid_commit_reset.patch resolves the issue to confirm my hunch.

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: Andrew Morton [mailto:akpm@linux-foundation.org]
> Sent: Tuesday, May 29, 2007 10:14 PM
> To: Yinghai Lu
> Cc: Vivek Goyal; Eric W. Biederman; AACRAID; Linux Kernel
> Mailing List; linux-scsi@vger.kernel.org; Michal Piotrowski
> Subject: Re: kexec and aacraid broken
>
>
> On Tue, 29 May 2007 18:59:32 -0700 "Yinghai Lu"
> <yhlu.kernel@gmail.com> wrote:
>
> > latest tree, can not use kexec to load 2.6.22-rc3 at least.
> >
> > got:
> >
> > AAC0: adapter kernel panic'd fffffffd
> > AAC0: adapter kernel failed to start, init status=0
>
> One of the two diffs below, I guess. Please do a `patch -R
> -p1' of this
> email and retest?
>
> >
> > but can load 2.6.21.3
> >
>
> Michal, can you please add this to the regression list?
>
>
>
>
> commit 9e4d4a5d71d673901d9c1df5146ce545c2cc0cc0
> Author: Salyzyn, Mark <mark_salyzyn@adaptec.com>
> Date: Tue May 1 11:43:06 2007 -0400
>
> [SCSI] aacraid: superfluous adapter reset for IBM 8
> series ServeRAID controllers
>
> The kexec patch introduced a superfluous (and otherwise
> inert) reset of
> some adapters. The register can have a hardware default
> value that has
> zeros for the undefined interrupts. This patch refines
> the test of the
> interrupt enable register to focus on only the interrupts
> that affect
> the driver in order to detect if an incomplete shutdown
> of the Adapter
> had occurred (kdump).
>
> Signed-off-by: Mark Salyzyn <aacraid@adaptec.com>
> Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
>
> diff --git a/drivers/scsi/aacraid/rx.c b/drivers/scsi/aacraid/rx.c
> index b6ee3c0..291cd14 100644
> --- a/drivers/scsi/aacraid/rx.c
> +++ b/drivers/scsi/aacraid/rx.c
> @@ -542,7 +542,7 @@ int _aac_rx_init(struct aac_dev *dev)
> dev->a_ops.adapter_sync_cmd = rx_sync_cmd;
> dev->a_ops.adapter_enable_int = aac_rx_disable_interrupt;
> dev->OIMR = status = rx_readb (dev, MUnit.OIMR);
> - if ((((status & 0xff) != 0xff) || reset_devices) &&
> + if ((((status & 0x0c) != 0x0c) || reset_devices) &&
> !aac_rx_restart_adapter(dev, 0))
> ++restart;
> /*
> commit a5694ec545a880f9d23463fddc894f5096cc68fa
> Author: Salyzyn, Mark <mark_salyzyn@adaptec.com>
> Date: Mon Apr 30 13:22:24 2007 -0400
>
> [SCSI] aacraid: kexec fix (reset interrupt handler)
>
> Another layer on this onion also discovered by Duane, the
> interrupt enable handler also needed to be set ... The
> interrupt enable
> was called from within the synchronous command handler.
>
> Signed-off-by: Mark Salyzyn <aacraid@adaptec.com>
> Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
>
> diff --git a/drivers/scsi/aacraid/rx.c b/drivers/scsi/aacraid/rx.c
> index 0c71315..b6ee3c0 100644
> --- a/drivers/scsi/aacraid/rx.c
> +++ b/drivers/scsi/aacraid/rx.c
> @@ -539,6 +539,8 @@ int _aac_rx_init(struct aac_dev *dev)
> }
>
> /* Failure to reset here is an option ... */
> + dev->a_ops.adapter_sync_cmd = rx_sync_cmd;
> + dev->a_ops.adapter_enable_int = aac_rx_disable_interrupt;
> dev->OIMR = status = rx_readb (dev, MUnit.OIMR);
> if ((((status & 0xff) != 0xff) || reset_devices) &&
> !aac_rx_restart_adapter(dev, 0))
>
>

[-- Attachment #2: aacraid_commit_reset.patch --]
[-- Type: application/octet-stream, Size: 3499 bytes --]

diff -ru a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
--- a/drivers/scsi/aacraid/aachba.c 2007-05-16 10:29:25.697735367 -0400
+++ b/drivers/scsi/aacraid/aachba.c 2007-05-16 10:37:33.537128485 -0400
@@ -146,7 +146,7 @@
static int nondasd = -1;
static int dacmode = -1;

-static int commit = -1;
+int aac_commit = -1;
int startup_timeout = 180;
int aif_timeout = 120;

@@ -154,7 +154,7 @@
MODULE_PARM_DESC(nondasd, "Control scanning of hba for nondasd devices. 0=off, 1=on");
module_param(dacmode, int, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(dacmode, "Control whether dma addressing is using 64 bit DAC. 0=off, 1=on");
-module_param(commit, int, S_IRUGO|S_IWUSR);
+module_param_named(commit, aac_commit, int, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(commit, "Control whether a COMMIT_CONFIG is issued to the adapter for foreign arrays.\nThis is typically needed in systems that do not have a BIOS. 0=off, 1=on");
module_param(startup_timeout, int, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(startup_timeout, "The duration of time in seconds to wait for adapter to have it's kernel up and\nrunning. This is typically adjusted for large systems that do not have a BIOS.");
@@ -173,6 +173,9 @@
module_param(expose_physicals, int, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(expose_physicals, "Expose physical components of the arrays. -1=protect 0=off, 1=on");

+int aac_reset_devices = 0;
+module_param_named(reset_devices, aac_reset_devices, int, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(reset_devices, "Force an adapter reset at initialization.");

static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
struct fib *fibptr) {
@@ -246,7 +249,7 @@
aac_fib_complete(fibptr);
/* Send a CT_COMMIT_CONFIG to enable discovery of devices */
if (status >= 0) {
- if ((commit == 1) || commit_flag) {
+ if ((aac_commit == 1) || commit_flag) {
struct aac_commit_config * dinfo;
aac_fib_init(fibptr);
dinfo = (struct aac_commit_config *) fib_data(fibptr);
@@ -261,7 +264,7 @@
1, 1,
NULL, NULL);
aac_fib_complete(fibptr);
- } else if (commit == 0) {
+ } else if (aac_commit == 0) {
printk(KERN_WARNING
"aac_get_config_status: Foreign device configurations are being ignored\n");
}
diff -ru a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
--- a/drivers/scsi/aacraid/aacraid.h 2007-05-16 10:29:25.697735367 -0400
+++ b/drivers/scsi/aacraid/aacraid.h 2007-05-16 10:37:33.538128354 -0400
@@ -1829,3 +1829,5 @@
extern int startup_timeout;
extern int aif_timeout;
extern int expose_physicals;
+extern int aac_reset_devices;
+extern int aac_commit;
diff -ru a/drivers/scsi/aacraid/rx.c b/drivers/scsi/aacraid/rx.c
--- a/drivers/scsi/aacraid/rx.c 2007-05-16 10:29:25.699735113 -0400
+++ b/drivers/scsi/aacraid/rx.c 2007-05-16 10:37:33.539128223 -0400
@@ -488,6 +488,8 @@
return -EINVAL;
if (rx_readl(dev, MUnit.OMRx[0]) & KERNEL_PANIC)
return -ENODEV;
+ if (startup_timeout < 300)
+ startup_timeout = 300;
return 0;
}

@@ -542,7 +544,7 @@
dev->a_ops.adapter_sync_cmd = rx_sync_cmd;
dev->a_ops.adapter_enable_int = aac_rx_disable_interrupt;
dev->OIMR = status = rx_readb (dev, MUnit.OIMR);
- if ((((status & 0x0c) != 0x0c) || reset_devices) &&
+ if ((((status & 0x0c) != 0x0c) || aac_reset_devices || reset_devices) &&
!aac_rx_restart_adapter(dev, 0))
++restart;
/*
@@ -594,6 +596,8 @@
}
msleep(1);
}
+ if (restart)
+ aac_commit = 1;
/*
* Fill in the common function dispatch table.
*/