Re: WD_QUIRK_FORCE_LBA48

[Christoph Badura <bad%bsd.de@localhost>]

Jonathan A. Kollasch <jakllsch <at> kollasch.net> writes:

> On Sat, Dec 05, 2009 at 03:06:50PM +0000, David Laight wrote:
> > My suspicion is that the problems only relate to transfers that
> > actually cross the LBA48 boundary.
> > 
> > So quite possibly checking:
> >     wd->sc_wdc_bio.blkno + nblks > LBA48_THRESHOLD
> > would remove the need for the quirk.
> 
> I have Seagate and Hitachi drives that refuse accesses across the LBA28
> boundary with LBA28 commands.

I just came across that same problem with a new WD2500BEVE in a Thinkpad T43.

When the cgd-scrubbing crosses the LBA48 boundary i get the following errors
(transscribed manually):

piixide0:0:0 bus-master DMA error: status=0x26
wd0g: LBA48 bug writing fsbn 226228288 of 226228288-226228351 (wd0 bn 268435407;
cn 266304 tn 15 sn 30), retrying
piixide0:0:0: lost interrupt
        type: ata b_count: 32768 tc_skip: 0
piixide0:0:0: bus-master DMA error: missing interrupt, status=0x21
piixide0:0:0: device timeout, c_bcount=32768, c_skip0
wd0g: device timeout writing fsbn 226228288 of 226228288-226228351 (wd0 bn
268435407; cn 266304 tn 15 sn 30), retrying
piixide0 channel 0: reset failed for drive 0

and the machine locks up hard with the HDD LED on solid.

I can resolve the lockup by resetting the drive from the "LBA48 bug" code in
wd.c:

Index: wd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ata/wd.c,v
retrieving revision 1.363.8.1
diff -u -r1.363.8.1 wd.c
--- wd.c        18 Feb 2009 00:23:09 -0000      1.363.8.1
+++ wd.c        13 Dec 2009 22:07:04 -0000
@@ -817,7 +817,7 @@
                                errmsg = "LBA48 bug";
                                wd->sc_quirks |= WD_QUIRK_FORCE_LBA48;
                                do_perror = 0;
-                               goto retry2;
+                               goto retry;
                        }
                }
 retry:         /* Just reset and retry. Can we do more ? */

Maybe we want to commit that.  I get only one error and then the message
that the soft error was corrected.

On the other hand I find it obvious that we can't expect a LBA28 command to
access the disk across the LBA48 boundary.  So we need to actaually check 
whether the last block of requested transfer is beyond the LBA48 barrier,
as David Laight suggested, and use an LBA48 transfer in that case.
Incidentally, wddump() does this already.

That makes the above disk work for me without throwing spurious errors.

I don't expect this to be a problem with controllers that do mishandle LBA48
commands.  LBA48 commands will only be used when accessing sectors beyond the
LBA48 border and those sectors are inaccessible by these controllers anyway.

The other option would be to split the transfer similar to the MOD15 case.
But I see no need for that.

Index: wd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ata/wd.c,v
retrieving revision 1.363.8.1
diff -u -r1.363.8.1 wd.c
--- wd.c        18 Feb 2009 00:23:09 -0000      1.363.8.1
+++ wd.c        13 Dec 2009 22:06:33 -0000
@@ -732,6 +732,7 @@
        }
 
        wd->sc_wdc_bio.blkno = bp->b_rawblkno;
+       wd->sc_wdc_bio.bcount = bp->b_bcount;
        wd->sc_wdc_bio.blkdone =0;
        wd->sc_bp = bp;
        /*
@@ -744,14 +745,15 @@
        else
                wd->sc_wdc_bio.flags = 0;
        if (wd->sc_flags & WDF_LBA48 &&
-           (wd->sc_wdc_bio.blkno > LBA48_THRESHOLD ||
+           (wd->sc_wdc_bio.blkno +
+               wd->sc_wdc_bio.bcount / wd->sc_dk.dk_label->d_secsize >
+               LBA48_THRESHOLD ||
            (wd->sc_quirks & WD_QUIRK_FORCE_LBA48) != 0))
                wd->sc_wdc_bio.flags |= ATA_LBA48;
        if (wd->sc_flags & WDF_LBA)
                wd->sc_wdc_bio.flags |= ATA_LBA;
        if (bp->b_flags & B_READ)
                wd->sc_wdc_bio.flags |= ATA_READ;
-       wd->sc_wdc_bio.bcount = bp->b_bcount;
        wd->sc_wdc_bio.databuf = bp->b_data;
        /* Instrumentation. */
        disk_busy(&wd->sc_dk);
@@ -1634,7 +1636,7 @@
        wd->sc_wdc_bio.blkno = blkno;
        wd->sc_wdc_bio.flags = ATA_POLL;
        if (wd->sc_flags & WDF_LBA48 &&
-           (blkno > LBA48_THRESHOLD ||
+           (blkno + nblks > LBA48_THRESHOLD ||
            (wd->sc_quirks & WD_QUIRK_FORCE_LBA48) != 0))
                wd->sc_wdc_bio.flags |= ATA_LBA48;
        if (wd->sc_flags & WDF_LBA)

I'm tempted to move the initialization of sc_bdc_bio.bcount and databuf before
the sc_flags setting logic in any case, because it makes the code more reable.

--chris