[PATCH] Panic in acd_geom_detach if device got lost (was: CFR: ACPI Dock driver)

Gavin Atkinson <gavin.atkinson_at_ury.york.ac.uk> wrote:

> On Fri, 14 Apr 2006, [ISO-8859-1] Søren Schmidt wrote:
> 
> > On Fre, 2006-04-14 at 23:30 +0900, Mitsuru IWASAKI wrote:
> >> Hi, Soren.
> >> I've found the problem with detached ATA channel suspending.
> >> After detaching second channel, the system drops into infinity
> >> loop at ata_suspend().
> >> I'll commit the following patches with acpi_dock patches, so
> >> please refine these if they have any problem.
> >
> > Uhm, would this simple patch help you ?
> >
> > --- ata-all.c   31 Mar 2006 08:09:04 -0000      1.271
> > +++ ata-all.c   14 Apr 2006 15:39:20 -0000
> > _at__at_ -277,8 +277,8 _at__at_
> >     if (!dev || !(ch = device_get_softc(dev)))
> >        return ENXIO;
> >
> > -    /* wait for the channel to be IDLE before entering suspend
> > mode */
> > -    while (1) {
> > +    /* wait for the channel to be IDLE or detached before
> > suspending */
> > +    while (ch->r_irq) {
> >        mtx_lock(&ch->state_mtx);
> >        if (ch->state == ATA_IDLE) {
> >            ch->state = ATA_ACTIVE;
> >
> > That I'll gladly commit for you instead...
> 
> Is there any chance this will also fix the panic myself and several
> others have been seeing with suspend and CD-ROM drives?
> http://unix.derkeiler.com/Mailing-Lists/FreeBSD/current/2005-08/0996.html
> 
> I'll take my laptop to top-of-tree CURRENT in the next day or so to
> try it.

I tried the above patch on RELENG_6 and it doesn't seem to change my
situation (which isn't suspend related anyway). If I remove my
ThinkPad's cdrom drive, FreeBSD becomes unresponsive after atacontrol
reinit ata1.

It seems to freeze in this part of ata-lowlevel.c:

492:    /* in some setups we dont want to test for a slave */
493-    if (!(ch->flags & ATA_NO_SLAVE)) {
494-    ATA_IDX_OUTB(ch, ATA_DRIVE, ATA_D_IBM | ATA_D_LBA | ATA_SLAVE);
495-    DELAY(10);      
496-    ostat1 = ATA_IDX_INB(ch, ATA_STATUS);
497-    if ((ostat1 & 0xf8) != 0xf8 && ostat1 != 0xa5) {
498-        stat1 = ATA_S_BUSY;
499-        mask |= 0x02;
500-    }
501-    }
502-

More precisely, it doesn't get past the macro in line 494.

Any ideas why "ATA_IDX_OUTB(ch, ATA_DRIVE, ATA_D_IBM | ATA_D_LBA | ATA_SLAVE);"
could cause an unresponsive system? I ruled out a nullpointer, but I haven't
found the code the macro leads to yet. 

If I comment out this whole block, I reach the panic Gavin already
analysed in the link above. It can be prevented with the following
patch:

fk_at_TP51 /usr/src/sys/dev/ata $diff -uN atapi-cd.c.original atapi-cd.c 
--- atapi-cd.c.original Sat Apr 15 16:59:46 2006
+++ atapi-cd.c  Sat Apr 15 17:12:32 2006
_at__at_ -193,9 +193,12 _at__at_
 {   
     struct acd_softc *cdp = device_get_ivars(arg);
 
-    /* signal geom so we dont get any further requests */
-    g_wither_geom(cdp->gp, ENXIO);
-
+    if (cdp != NULL) {    
+        /* signal geom so we dont get any further requests */
+        g_wither_geom(cdp->gp, ENXIO);
+    } else {
+        printf("acd_geom_detach: Device already detached?\n");
+    }
     /* fail requests on the queue and any thats "in flight" for this device */
     ata_fail_requests(arg);

Now I can remove my cdrom drive while FreeBSD is running and
atacontrol reinit no longer causes the panic:

fk_at_TP51 ~ $sudo atacontrol reinit ata1
Master: acd0 <HL-DT-STCD-RW/DVD DRIVE GCC-4241N/1.04> ATA/ATAPI revision 5
Slave:       no device present  
[Removing drive]
fk_at_TP51 ~ $sudo atacontrol reinit ata1
Master:      no device present  
Slave:       no device present

Fabian
-- 
http://www.fabiankeil.de/