Re: kern/40569: Faild RAIDframe parity rewrite prevents system shutdown

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

The following reply was made to PR kern/40569; it has been noted by GNATS.

From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
To: Matthias Scheler <tron%zhadum.org.uk@localhost>
Cc: gnats-bugs%NetBSD.org@localhost
Subject: Re: kern/40569: Faild RAIDframe parity rewrite prevents system shutdown
Date: Tue, 10 Feb 2009 23:24:40 +0100

 --ZPt4rx8FFjLCG7dd
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 On Tue, Feb 10, 2009 at 10:17:06PM +0000, Matthias Scheler wrote:
 > On Tue, Feb 10, 2009 at 11:12:38PM +0100, Manuel Bouyer wrote:
 > > I've reproduced it on a system here and I think I've a fix.
 > 
 > That's great news.
 > 
 > > Just to confirm; you're using a AHCI controller, right ?
 > 
 > Yes, I do:
 > 
 > atabus4 at ahcisata0 channel 2
 > atabus5 at ahcisata0 channel 3
 > [...]
 > ahcisata0 port 2: device present, speed: 1.5Gb/s
 > ahcisata0 port 3: device present, speed: 3.0Gb/s
 
 OK, so it's probably an issue with the ahci controller: b_resid was set to
 0 even in case of failure; and it's used in the LBA48 workaround detection
 to see if we crossed the boundary ... I think the attached patch fixes it but
 unfortunably my test box didn't reboot after panic to I can't test before
 tomorow.
 
 -- 
 Manuel Bouyer <bouyer%antioche.eu.org@localhost>
      NetBSD: 26 ans d'experience feront toujours la difference
 --
 
 --ZPt4rx8FFjLCG7dd
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename=diff
 
 Index: ahcisata_core.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/ic/ahcisata_core.c,v
 retrieving revision 1.18
 diff -u -p -u -r1.18 ahcisata_core.c
 --- ahcisata_core.c    3 Oct 2008 13:02:08 -0000       1.18
 +++ ahcisata_core.c    10 Feb 2009 22:22:42 -0000
 @@ -1065,7 +1065,7 @@ ahci_bio_complete(struct ata_channel *ch
                ata_bio->error = TIMEOUT;
        } else {
                callout_stop(&chp->ch_callout);
 -              ata_bio->error = 0;
 +              ata_bio->error = NOERROR;
        }
  
        chp->ch_queue->active_xfer = NULL;
 @@ -1095,7 +1095,14 @@ ahci_bio_complete(struct ata_channel *ch
            BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
        AHCIDEBUG_PRINT(("ahci_bio_complete bcount %ld",
            ata_bio->bcount), DEBUG_XFERS);
 -      ata_bio->bcount -= le32toh(achp->ahcic_cmdh[slot].cmdh_prdbc);
 +      /* 
 +       * if it was a write, complete data buffer may have been transfered
 +       * before error detection; in this case don't use cmdh_prdbc
 +       * as it won't reflect what was written to media. Assume nothing
 +       * was transfered and leave bcount as-is.
 +       */
 +      if ((ata_bio->flags & ATA_READ) || ata_bio->error != NOERROR)
 +              ata_bio->bcount -= le32toh(achp->ahcic_cmdh[slot].cmdh_prdbc);
        AHCIDEBUG_PRINT((" now %ld\n", ata_bio->bcount), DEBUG_XFERS);
        (*chp->ch_drive[drive].drv_done)(chp->ch_drive[drive].drv_softc);
        atastart(chp);
 
 --ZPt4rx8FFjLCG7dd--