Skip to contentSkip to conversation

fix sata_sil quirk

PATCH In review Jeff Garzik 5 participants 12 messages Jun 22, 2004 – Sep 27, 2004

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix sata_sil quirk

Here's my suggested fix... good catch Ricky.

Yes, unfortunately performance will be dog slow.

Silicon Image 311x is fully SATA compliant -- but it's the only controller that sends odd-sized packets to the SATA device. That causes no end of problems, including the thing that SIL_QUIRK_MOD15WRITE attempts to work around.

I've got contacts at Silicon Image, and have been meaning to bug them for a "real fix" for a while. It is rumored that there is a much better fix, which allows full performance while at the same time not killing your SATA drive due to odd-sized SATA frames on the wire.

Unfortunately, at this point in time, we must err on the side of caution and cripple performance, for stability.

Jeff

===== drivers/scsi/libata-scsi.c 1.39 vs edited =====

drivers/scsi/libata-scsi.c +2 -1
@@ -182,7 +182,8 @@
 		 * 65534 when Jens Axboe's patch for dynamically
 		 * determining max_sectors is merged.
 		 */
-		if (dev->flags & ATA_DFLAG_LBA48) {
+		if ((dev->flags & ATA_DFLAG_LBA48) &&
+		    ((dev->flags & ATA_DFLAG_LOCK_SECTORS) == 0)) {
 			sdev->host->max_sectors = 2048;
 			blk_queue_max_sectors(sdev->request_queue, 2048);
 		}

===== drivers/scsi/sata_sil.c 1.26 vs edited =====

drivers/scsi/sata_sil.c +1 -0
@@ -302,6 +302,7 @@
 		       ap->id, dev->devno);
 		ap->host->max_sectors = 15;
 		ap->host->hostt->max_sectors = 15;
+		dev->flags |= ATA_DFLAG_LOCK_SECTORS;
 		return;
 	}
 

===== include/linux/libata.h 1.38 vs edited =====

include/linux/libata.h +1 -0
@@ -91,6 +91,7 @@
 	ATA_DFLAG_MASTER	= (1 << 2), /* is device 0? */
 	ATA_DFLAG_WCACHE	= (1 << 3), /* has write cache we can
 					     * (hopefully) flush? */
+	ATA_DFLAG_LOCK_SECTORS	= (1 << 4), /* don't adjust max_sectors */
 
 	ATA_DEV_UNKNOWN		= 0,	/* unknown device */
 	ATA_DEV_ATA		= 1,	/* ATA device */
#1 lore raw
Quoted 1 line · Jeff Garzik

Here's my suggested fix... good catch Ricky.

And I don't even know why I looked at max_sectors :-) (I need more Dew.)

Quoted 1 line

Yes, unfortunately performance will be dog slow.

Well, at least puppy slow...
Device: tps kB_read/s kB_wrtn/s kB_read kB_wrtn

sda            1811.65         0.00      9629.85          0     577887
sdb            1807.15         0.00      9629.60          0     577872
sdc            1807.25         0.00      9629.86          0     577888
sdd            1807.05         0.00      9629.86          0     577888
md_d0         14444.64         0.00     48148.84          0    2889412
md_d0p2        9629.78         0.00     38519.11          0    2311532

(over 60sec, 8M O_DIRECT accesses, 128 stripes * 16k RAID0)

Without the MOD15 hack, the numbers are 2x higher, but they stop after a few minutes :-)

Quoted 4 lines

I've got contacts at Silicon Image, and have been meaning to bug them for a "real fix" for a while. It is rumored that there is a much better fix, which allows full performance while at the same time not killing your SATA drive due to odd-sized SATA frames on the wire.

Ask them what they do in their driver? (the linux one and the windows one) Looking at the linux driver, the mod15 quirk is there, but there doesn't appear to be any associated device list. (I've already post the single Maxtor device listed.) FreeBSD detects the stall, resets the chip and hopes that clears the problem. (People are not happy about that.)

--Ricky

===== drivers/scsi/libata-scsi.c 1.39 vs edited =====

drivers/scsi/libata-scsi.c +2 -1
@@ -182,7 +182,8 @@
 		 * 65534 when Jens Axboe's patch for dynamically
 		 * determining max_sectors is merged.
 		 */
-		if (dev->flags & ATA_DFLAG_LBA48) {
+		if ((dev->flags & ATA_DFLAG_LBA48) &&
+		    ((dev->flags & ATA_DFLAG_LOCK_SECTORS) == 0)) {
 			sdev->host->max_sectors = 2048;
 			blk_queue_max_sectors(sdev->request_queue, 2048);
 		}

===== drivers/scsi/sata_sil.c 1.26 vs edited =====

drivers/scsi/sata_sil.c +1 -0
@@ -302,6 +302,7 @@
 		       ap->id, dev->devno);
 		ap->host->max_sectors = 15;
 		ap->host->hostt->max_sectors = 15;
+		dev->flags |= ATA_DFLAG_LOCK_SECTORS;
 		return;
 	}
 

===== include/linux/libata.h 1.38 vs edited =====

include/linux/libata.h +1 -0
@@ -91,6 +91,7 @@
 	ATA_DFLAG_MASTER	= (1 << 2), /* is device 0? */
 	ATA_DFLAG_WCACHE	= (1 << 3), /* has write cache we can
 					     * (hopefully) flush? */
+	ATA_DFLAG_LOCK_SECTORS	= (1 << 4), /* don't adjust max_sectors */
 
 	ATA_DEV_UNKNOWN		= 0,	/* unknown device */
 	ATA_DEV_ATA		= 1,	/* ATA device */
#2 lore raw
Quoted 15 lines · Ricky Beam
Quoted 1 line · Jeff Garzik

Here's my suggested fix... good catch Ricky.

And I don't even know why I looked at max_sectors :-) (I need more Dew.)

Quoted 1 line

Yes, unfortunately performance will be dog slow.

Well, at least puppy slow...
Device: tps kB_read/s kB_wrtn/s kB_read kB_wrtn

sda            1811.65         0.00      9629.85          0     577887
sdb            1807.15         0.00      9629.60          0     577872
sdc            1807.25         0.00      9629.86          0     577888
sdd            1807.05         0.00      9629.86          0     577888
md_d0         14444.64         0.00     48148.84          0    2889412
md_d0p2        9629.78         0.00     38519.11          0    2311532
Quoted 4 lines

(over 60sec, 8M O_DIRECT accesses, 128 stripes * 16k RAID0)

Without the MOD15 hack, the numbers are 2x higher, but they stop after a few minutes :-)

Stability first!

Quoted 10 lines
Quoted 4 lines

I've got contacts at Silicon Image, and have been meaning to bug them for a "real fix" for a while. It is rumored that there is a much better fix, which allows full performance while at the same time not killing your SATA drive due to odd-sized SATA frames on the wire.

Ask them what they do in their driver? (the linux one and the windows one) Looking at the linux driver, the mod15 quirk is there, but there doesn't appear to be any associated device list. (I've already post the single Maxtor device listed.) FreeBSD detects the stall, resets the chip and hopes that clears the problem. (People are not happy about that.)

I just poked them. I'm not satisfied with what any Linux or FreeBSD driver does, I want to Get It Right(tm) :) I'm willing to bet that their Linux driver does mod15 only because they didn't know kernel internals that well.

Jeff

#3 lore raw
In reply to Ricky Beam
Quoted 18 lines · Ricky Beam
Quoted 1 line · Jeff Garzik

Here's my suggested fix... good catch Ricky.

And I don't even know why I looked at max_sectors :-) (I need more Dew.)

Quoted 1 line

Yes, unfortunately performance will be dog slow.

Well, at least puppy slow...
Device: tps kB_read/s kB_wrtn/s kB_read kB_wrtn

sda            1811.65         0.00      9629.85          0     577887
sdb            1807.15         0.00      9629.60          0     577872
sdc            1807.25         0.00      9629.86          0     577888
sdd            1807.05         0.00      9629.86          0     577888
md_d0         14444.64         0.00     48148.84          0    2889412
md_d0p2        9629.78         0.00     38519.11          0    2311532
Quoted 4 lines

(over 60sec, 8M O_DIRECT accesses, 128 stripes * 16k RAID0)

Without the MOD15 hack, the numbers are 2x higher, but they stop after a few minutes :-)

Is this with my patch?

If so, I'll go ahead and forward it upstream, since I would certainly like a stabilization fix applied ASAP.

Quoted 11 lines
Quoted 4 lines

I've got contacts at Silicon Image, and have been meaning to bug them for a "real fix" for a while. It is rumored that there is a much better fix, which allows full performance while at the same time not killing your SATA drive due to odd-sized SATA frames on the wire.

Ask them what they do in their driver? (the linux one and the windows one) Looking at the linux driver, the mod15 quirk is there, but there doesn't appear to be any associated device list. (I've already post the single Maxtor device listed.) FreeBSD detects the stall, resets the chip and hopes that clears the problem. (People are not happy about that.)

The full-speed fix requires splitting affected DMA writes into two separate commands, when the sector count matches "sectors % 15 == 1".

Jeff

#4 lore raw
Quoted 1 line · Jeff Garzik

Is this with my patch?

It is now. Either way, max_sectors is staying 15. I'll post the results after it's had time to zero 40G a few dozen times.

Quoted 2 lines

The full-speed fix requires splitting affected DMA writes into two separate commands, when the sector count matches "sectors % 15 == 1".

Why 15 I wonder?

--Ricky

#5 lore raw
In reply to Jeff Garzik
Quoted 5 lines · Jeff Garzik

Here's my suggested fix... good catch Ricky.

Yes, unfortunately performance will be dog slow.

Silicon Image 311x is fully SATA compliant -- but it's the only controller that sends odd-sized packets to the SATA device. That causes no end of problems, including the thing that SIL_QUIRK_MOD15WRITE attempts to work around.

As an extra data point, i have:

# cat /proc/scsi/scsi
Attached devices:
Host: scsi0 Channel: 00 Id: 00 Lun: 00
  Vendor: ATA      Model: WDC WD1600JD-00G Rev: 02.0
  Type:   Direct-Access                    ANSI SCSI revision: 05
Host: scsi1 Channel: 00 Id: 00 Lun: 00
  Vendor: ATA      Model: WDC WD1600JD-00G Rev: 02.0
  Type:   Direct-Access                    ANSI SCSI revision: 05
Host: scsi2 Channel: 00 Id: 00 Lun: 00
  Vendor: ATA      Model: WDC WD1600JD-00G Rev: 02.0
  Type:   Direct-Access                    ANSI SCSI revision: 05
# lspci  | grep Sil
00:09.0 RAID bus controller: Silicon Image, Inc. (formerly CMD Technology Inc) Silicon Image Serial ATARaid Controller [ CMD/Sil
3112/3112A ] (rev 02)
00:0a.0 RAID bus controller: Silicon Image, Inc. (formerly CMD Technology Inc) Silicon Image Serial ATARaid Controller
[ CMD/Sil 3112/3112A ] (rev 02)
# modinfo sata_sil
author:         Jeff Garzik
description:    low-level driver for Silicon Image SATA controller
license:        GPL
vermagic:       2.6.6-1.397.root K6 REGPARM gcc-3.3

And am not having (touch wood) any stability problems using these disks with linux md RAID1 and RAID5. Though, they're in a K6-II 350, so performance is slow anyway. (i get about 25MB/s absolute max reading from a RAID-5 array).

Quoted 1 line

Jeff

regards,

Signature · 4 lines
Paul Jakma	[email protected]	[email protected]	Key ID: 64A2FF6A
	warning: do not ever send email to [email protected]
Fortune:
A bird in the hand makes it awfully hard to blow your nose.
#6 lore raw
Quoted 12 lines · Paul Jakma
# cat /proc/scsi/scsi
Attached devices:
Host: scsi0 Channel: 00 Id: 00 Lun: 00
  Vendor: ATA      Model: WDC WD1600JD-00G Rev: 02.0
  Type:   Direct-Access                    ANSI SCSI revision: 05
Host: scsi1 Channel: 00 Id: 00 Lun: 00
  Vendor: ATA      Model: WDC WD1600JD-00G Rev: 02.0
  Type:   Direct-Access                    ANSI SCSI revision: 05
Host: scsi2 Channel: 00 Id: 00 Lun: 00
  Vendor: ATA      Model: WDC WD1600JD-00G Rev: 02.0
  Type:   Direct-Access                    ANSI SCSI revision: 05
Quoted 1 line

And am not having (touch wood) any stability problems using these disks with linux md RAID1 and RAID5. Though, they're in a K6-II 350, so performance is slow anyway. (i get about 25MB/s absolute max reading from a RAID-5 array).

Cool. Yeah, non-Seagate should be full speed and unaffected...

Jeff

#7 lore raw
Quoted 1 line · Jeff Garzik

Cool. Yeah, non-Seagate should be full speed and unaffected...

But I got the impression your patch enables mod15-quirk for all LBA48 drives, is that correct? If so, if I have to update kernels here, I think I'll reverse that one if it affects all LBA48, as I'd rather not suffer the performance hit (its slow enough already because of slow CPU/chipset/contended PCI bus thank you very much ;) ) - until such time as a better fix is known.

Quoted 1 line

Jeff

regards,

Signature · 5 lines
Paul Jakma	[email protected]	[email protected]	Key ID: 64A2FF6A
	warning: do not ever send email to [email protected]
Fortune:
It is so soon that I am done for, I wonder what I was begun for.
		-- Epitaph, Cheltenham Churchyard
#8 lore raw
Quoted 6 lines · Paul Jakma
Quoted 1 line · Jeff Garzik

Cool. Yeah, non-Seagate should be full speed and unaffected...

But I got the impression your patch enables mod15-quirk for all LBA48 drives, is that correct? If so, if I have to update kernels here, I think I'll reverse that one if it affects all LBA48, as I'd rather not suffer the performance hit (its slow enough already because of slow CPU/chipset/contended PCI bus thank you very much ;) ) - until such time as a better fix is known.

Nope, my patch only enables ATA_DFLAG_LOCK_SECTORS for devices flagged with SIL_QUIRK_MOD15WRITE:

        { "ST320012AS",         SIL_QUIRK_MOD15WRITE },
        { "ST330013AS",         SIL_QUIRK_MOD15WRITE },
        { "ST340017AS",         SIL_QUIRK_MOD15WRITE },
        { "ST360015AS",         SIL_QUIRK_MOD15WRITE },
        { "ST380023AS",         SIL_QUIRK_MOD15WRITE },
        { "ST3120023AS",        SIL_QUIRK_MOD15WRITE },
        { "ST340014ASL",        SIL_QUIRK_MOD15WRITE },
        { "ST360014ASL",        SIL_QUIRK_MOD15WRITE },
        { "ST380011ASL",        SIL_QUIRK_MOD15WRITE },
        { "ST3120022ASL",       SIL_QUIRK_MOD15WRITE },
        { "ST3160021ASL",       SIL_QUIRK_MOD15WRITE },
        { "Maxtor 4D060H3",     SIL_QUIRK_UDMA5MAX },
[...]
        if (quirks & SIL_QUIRK_MOD15WRITE) {
                printk(KERN_INFO "ata%u(%u): applying errata fix\n",
                       ap->id, dev->devno);
                ap->host->max_sectors = 15;
                ap->host->hostt->max_sectors = 15;
                dev->flags |= ATA_DFLAG_LOCK_SECTORS;
                return;
        }
#9 lore raw
Quoted 2 lines · Jeff Garzik

Nope, my patch only enables ATA_DFLAG_LOCK_SECTORS for devices flagged with SIL_QUIRK_MOD15WRITE:

...

That list needs a:

         { "ST3160023AS",        SIL_QUIRK_MOD15WRITE },

as well.

--Ricky

PS: I've completed 23 cycles of zeroing a 40G section of RAID0 space without
error.

#10 lore raw
Quoted 4 lines · Ricky Beam

That list needs a:

        { "ST3160023AS",        SIL_QUIRK_MOD15WRITE },

as well.

happens to be my drive, is there any way to tell a drive needs be in the quirk 15 list, other than it's Seagate and big writes block the dev?

// George

Signature · 3 lines
George Georgalis, Architect and administrator, Linux services. IXOYE
http://galis.org/george/  cell:646-331-2027  mailto:[email protected]
Key fingerprint = 5415 2738 61CF 6AE1 E9A7  9EF0 0186 503B 9831 1631
#11 lore raw
Quoted 9 lines · George Georgalis
Quoted 4 lines · Ricky Beam

That list needs a:

        { "ST3160023AS",        SIL_QUIRK_MOD15WRITE },

as well.

happens to be my drive, is there any way to tell a drive needs be in the quirk 15 list, other than it's Seagate and big writes block the dev?

Actually my problem with big writes was the bk kernel I downloaded was a rev too early (2.6.7-bk7 ?).

I tested the sata_sil.c version from June 25 (via bk checkout) extensively and had no problem... posted my results then, hdparm reported ~42 - 51 MB/sec, and never a write block.

Today I try putting 2.6.8.1 on the box and I'm getting ~14 MB/sec, the drive has been added to the sil_blacklist. Why? What did I miss?

I've taken my drive out of the black list, abused it with 5 continuous writes and "top id 0" no problem after 58Gb. I'm doing it again this time simultaneously with a massive rm -rf of backup directories. I really don't anticipate a problem.

So what's up with

         /*{ "ST3160023AS",      SIL_QUIRK_MOD15WRITE },*/

before it got in there, it was said the black list "would not grow" (for unexplained reasons).

// George

Signature · 2 lines
George Georgalis, systems architect, administrator Linux BSD IXOYE
http://galis.org/george/ cell:646-331-2027 mailto:[email protected]