From owner-freebsd-stable Fri Mar 22 17:14:49 2002 Delivered-To: freebsd-stable@freebsd.org Received: from salmon.maths.tcd.ie (salmon.maths.tcd.ie [134.226.81.11]) by hub.freebsd.org (Postfix) with SMTP id E943037B41A for ; Fri, 22 Mar 2002 17:14:42 -0800 (PST) Received: from walton.maths.tcd.ie by salmon.maths.tcd.ie with SMTP id ; 23 Mar 2002 01:14:41 +0000 (GMT) To: Bryan Liesner Cc: freebsd-stable@freebsd.org, =?ISO-8859-1?Q?S=F8ren_Schmidt?= Subject: Re: ATA MFC - Suspend/resume causes panic In-Reply-To: Your message of "Fri, 22 Mar 2002 18:52:20 EST." <3C9BC3A8.5020301@bellatlantic.net> Date: Sat, 23 Mar 2002 01:14:41 +0000 From: Ian Dowse Message-ID: <200203230114.aa01481@salmon.maths.tcd.ie> Sender: owner-freebsd-stable@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG In message <3C9BC3A8.5020301@bellatlantic.net>, Bryan Liesner writes: >The kernel repeatedly panics when waking up from a suspend: I get this too. Changing the ATA_WAIT_INTR to ATA_IMMEDIATE in ata_getparam() seems to work around the problem, but I don't fully understand what is going on. The basic issue is that tsleep() is called from an interrupt context via ata_resume. ata_getparam() calls ata_wait(), which seems to busy-loop in DELAY() waiting for the command to complete, so I'm just guessing that it makes little sense to have ata_command wait for the command to complete also. Below is the full patch I'm using at the moment. It includes the ATA_IMMEDIATE change and the spl changes I mentioned in an earlier message. Ian Index: ata-all.c =================================================================== RCS file: /home/iedowse/CVS/src/sys/dev/ata/ata-all.c,v retrieving revision 1.50.2.30 diff -u -r1.50.2.30 ata-all.c --- ata-all.c 18 Mar 2002 08:37:33 -0000 1.50.2.30 +++ ata-all.c 23 Mar 2002 00:53:32 -0000 @@ -162,7 +162,7 @@ ata_attach(device_t dev) { struct ata_channel *ch; - int error, rid; + int error, rid, s; if (!dev || !(ch = device_get_softc(dev))) return ENXIO; @@ -186,6 +186,7 @@ * otherwise attach what the probe has found in ch->devices. */ if (!ata_delayed_attach) { + s = splbio(); if (ch->devices & ATA_ATA_SLAVE) if (ata_getparam(&ch->device[SLAVE], ATA_C_ATA_IDENTIFY)) ch->devices &= ~ATA_ATA_SLAVE; @@ -210,6 +211,7 @@ if (ch->devices & ATA_ATAPI_SLAVE) atapi_attach(&ch->device[SLAVE]); #endif + splx(s); } return 0; } @@ -425,7 +427,7 @@ /* apparently some devices needs this repeated */ do { - if (ata_command(atadev, command, 0, 0, 0, ATA_WAIT_INTR)) { + if (ata_command(atadev, command, 0, 0, 0, ATA_IMMEDIATE)) { ata_prtdev(atadev, "%s identify failed\n", command == ATA_C_ATAPI_IDENTIFY ? "ATAPI" : "ATA"); return -1; @@ -469,13 +471,14 @@ ata_boot_attach(void) { struct ata_channel *ch; - int ctlr; + int ctlr, s; if (ata_delayed_attach) { config_intrhook_disestablish(ata_delayed_attach); free(ata_delayed_attach, M_TEMP); ata_delayed_attach = NULL; } + s = splbio(); /* * run through all ata devices and look for real ATA & ATAPI devices @@ -522,6 +525,7 @@ atapi_attach(&ch->device[SLAVE]); } #endif + splx(s); } static void To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-stable" in the body of the message