* [PATCH] re-fix sata_sil quirk (w/ description)
@ 2004-06-23 0:49 Jeff Garzik
2004-06-23 1:05 ` Ricky Beam
2004-06-23 1:11 ` Ricky Beam
0 siblings, 2 replies; 4+ messages in thread
From: Jeff Garzik @ 2004-06-23 0:49 UTC (permalink / raw)
To: linux-ide; +Cc: Bartlomiej Zolnierkiewicz, Ricky Beam
[-- Attachment #1: Type: text/plain, Size: 452 bytes --]
The patch itself is unchanged, but I have checked it in with a detailed
technical description, including my rationale.
Bart, I'm not going to bother with a 'better fix'. Since this only
affects 3112 controllers, I will be adding code to make sure that the
mod15write quirk does not activate for the other controllers (3512, 3114).
See the changeset description accompanying the patch for more info.
Pushing to Andrew and Linus right now...
[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2908 bytes --]
# 2004/06/22 20:42:16-04:00 jgarzik@pobox.com
# [libata sata_sil] Re-fix mod15write bug
#
# Certain early SATA drives have problems with write requests whose
# length satisfy the equation "sectors % 15 == 1", on the SiI 3112.
# Other drives, and other SiI controllers, are not affected.
#
# The fix for this problem is to avoid such requests, in one of three
# ways, for the affect drive+controller combos:
# 1) Limit all writes to 15 sectors
# 2) Use block layer features to avoid creating requests whose
# length satisfies the above equation.
# 3) When a request satisfies the above equation, split the request
# into two writes, neither of which satisfies the equation.
#
# I chose fix #1, the most simple to implement. After discussion with
# Silicon Image and others regarding the impact of this fix, I have
# decided to remain with fix #1, and will not be implementing a
# "better fix". This means that the affected SATA drives will see
# decreased performance, but set of affected drives is small and will
# never grow larger.
#
# Further, the complexity of implementing solution #2 or
# solution #3 is rather large.
#
# When implementing lba48 'large request' support, I unintentionally
# broke the fix for these affected drives. Kudos to Ricky Beam for
# noticing this.
#
# This change restores the fix, by adding a flag ATA_DFLAG_LOCK_SECTORS
# to indicate that the max_sectors value set by the low-level driver
# should never be changed.
#
diff -Nru a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
--- a/drivers/scsi/libata-scsi.c 2004-06-22 20:44:51 -04:00
+++ b/drivers/scsi/libata-scsi.c 2004-06-22 20:44:51 -04:00
@@ -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);
}
diff -Nru a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
--- a/drivers/scsi/sata_sil.c 2004-06-22 20:44:51 -04:00
+++ b/drivers/scsi/sata_sil.c 2004-06-22 20:44:51 -04:00
@@ -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;
}
diff -Nru a/include/linux/libata.h b/include/linux/libata.h
--- a/include/linux/libata.h 2004-06-22 20:44:51 -04:00
+++ b/include/linux/libata.h 2004-06-22 20:44:51 -04:00
@@ -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 */
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] re-fix sata_sil quirk (w/ description)
2004-06-23 0:49 [PATCH] re-fix sata_sil quirk (w/ description) Jeff Garzik
@ 2004-06-23 1:05 ` Ricky Beam
2004-06-23 1:11 ` Ricky Beam
1 sibling, 0 replies; 4+ messages in thread
From: Ricky Beam @ 2004-06-23 1:05 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-ide, Bartlomiej Zolnierkiewicz
On Tue, 22 Jun 2004, Jeff Garzik wrote:
>Bart, I'm not going to bother with a 'better fix'. Since this only
>affects 3112 controllers, I will be adding code to make sure that the
>mod15write quirk does not activate for the other controllers (3512, 3114).
Actually, it does effect other controllers ... I'm using a 3114. And it's
currently only known to effect Seagate drives. It may still happen to
other drives and my not effect Seagate drives with some future firmware
revision.
Current activity using buffered writes:
Device: tps kB_read/s kB_wrtn/s kB_read kB_wrtn
sda 1694.95 0.00 10438.68 0 626112
sdb 1694.33 0.00 10438.68 0 626112
sdc 1643.23 0.00 10438.68 0 626112
sdd 1631.01 0.00 10438.68 0 626112
md_d0 26096.70 0.00 46974.06 0 2817504
md_d0p2 20877.36 0.00 41754.72 0 2504448
That's certainly acceptable -- and in fact right at the edge of what the
3114 can do with all four port lit up.
--Ricky
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] re-fix sata_sil quirk (w/ description)
2004-06-23 0:49 [PATCH] re-fix sata_sil quirk (w/ description) Jeff Garzik
2004-06-23 1:05 ` Ricky Beam
@ 2004-06-23 1:11 ` Ricky Beam
2004-06-23 2:11 ` Jeff Garzik
1 sibling, 1 reply; 4+ messages in thread
From: Ricky Beam @ 2004-06-23 1:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-ide, Bartlomiej Zolnierkiewicz
[-- Attachment #1: Type: TEXT/PLAIN, Size: 706 bytes --]
On Tue, 22 Jun 2004, Jeff Garzik wrote:
>The patch itself is unchanged, but I have checked it in with a detailed
>technical description, including my rationale.
...
# ... This means that the affected SATA drives will see
# decreased performance, but set of affected drives is small and will
# never grow larger.
Actually, the set of affected drives is about 1/3rd of the SATA drive
market: all current model Seagate drives. (Maxtor and WD being the other
thirds.) My money says their new 400G monster will have the exact same
problems.
I'm using ST3160023AS drives (3 FW 3.18, 1 FW 3.05.) I'd include their
manufacture date, but they're rather busy right now so I cannot pull them
out.
--Ricky
[-- Attachment #2: Type: TEXT/PLAIN, Size: 2908 bytes --]
# 2004/06/22 20:42:16-04:00 jgarzik@pobox.com
# [libata sata_sil] Re-fix mod15write bug
#
# Certain early SATA drives have problems with write requests whose
# length satisfy the equation "sectors % 15 == 1", on the SiI 3112.
# Other drives, and other SiI controllers, are not affected.
#
# The fix for this problem is to avoid such requests, in one of three
# ways, for the affect drive+controller combos:
# 1) Limit all writes to 15 sectors
# 2) Use block layer features to avoid creating requests whose
# length satisfies the above equation.
# 3) When a request satisfies the above equation, split the request
# into two writes, neither of which satisfies the equation.
#
# I chose fix #1, the most simple to implement. After discussion with
# Silicon Image and others regarding the impact of this fix, I have
# decided to remain with fix #1, and will not be implementing a
# "better fix". This means that the affected SATA drives will see
# decreased performance, but set of affected drives is small and will
# never grow larger.
#
# Further, the complexity of implementing solution #2 or
# solution #3 is rather large.
#
# When implementing lba48 'large request' support, I unintentionally
# broke the fix for these affected drives. Kudos to Ricky Beam for
# noticing this.
#
# This change restores the fix, by adding a flag ATA_DFLAG_LOCK_SECTORS
# to indicate that the max_sectors value set by the low-level driver
# should never be changed.
#
diff -Nru a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
--- a/drivers/scsi/libata-scsi.c 2004-06-22 20:44:51 -04:00
+++ b/drivers/scsi/libata-scsi.c 2004-06-22 20:44:51 -04:00
@@ -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);
}
diff -Nru a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
--- a/drivers/scsi/sata_sil.c 2004-06-22 20:44:51 -04:00
+++ b/drivers/scsi/sata_sil.c 2004-06-22 20:44:51 -04:00
@@ -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;
}
diff -Nru a/include/linux/libata.h b/include/linux/libata.h
--- a/include/linux/libata.h 2004-06-22 20:44:51 -04:00
+++ b/include/linux/libata.h 2004-06-22 20:44:51 -04:00
@@ -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 */
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] re-fix sata_sil quirk (w/ description)
2004-06-23 1:11 ` Ricky Beam
@ 2004-06-23 2:11 ` Jeff Garzik
0 siblings, 0 replies; 4+ messages in thread
From: Jeff Garzik @ 2004-06-23 2:11 UTC (permalink / raw)
To: Ricky Beam; +Cc: linux-ide, Bartlomiej Zolnierkiewicz
Ricky Beam wrote:
> On Tue, 22 Jun 2004, Jeff Garzik wrote:
>
>> The patch itself is unchanged, but I have checked it in with a detailed
>> technical description, including my rationale.
>
> ...
>
> # ... This means that the affected SATA drives will see
> # decreased performance, but set of affected drives is small and will
> # never grow larger.
>
> Actually, the set of affected drives is about 1/3rd of the SATA drive
> market: all current model Seagate drives. (Maxtor and WD being the other
> thirds.) My money says their new 400G monster will have the exact same
> problems.
>
> I'm using ST3160023AS drives (3 FW 3.18, 1 FW 3.05.) I'd include their
> manufacture date, but they're rather busy right now so I cannot pull them
> out.
Seagate claims they fixed the problem, so I doubt it...
Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2004-06-23 2:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-06-23 0:49 [PATCH] re-fix sata_sil quirk (w/ description) Jeff Garzik
2004-06-23 1:05 ` Ricky Beam
2004-06-23 1:11 ` Ricky Beam
2004-06-23 2:11 ` Jeff Garzik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).