From: Tejun Heo <htejun@gmail.com>
To: stable@kernel.org, IDE/ATA development list <linux-ide@vger.kernel.org>
Cc: Jeff Garzik <jeff@garzik.org>, Michael Tokarev <mjt@tls.msk.ru>,
Diego Torres <dtorres@coral.dnsalias.org>
Subject: [PATCH -stable] libata: kill spurious NCQ completion detection
Date: Sat, 08 Dec 2007 09:25:31 +0900 [thread overview]
Message-ID: <4759E47B.8000108@gmail.com> (raw)
Spurious NCQ completion detection implemented in ahci was incorrect.
On AHCI receving and processing FISes and raising interrupts are not
interlocked and spurious interrupts are expected.
For example, if an interrupt occurs while interrupt handler is running
and the running interrupt handler handles the event the new IRQ
indicated, after IRQ handler finishes, it will be executed again
because IRQ pending bit is set by the new interrupt but there won't be
anything to process.
Please read the following message for more information.
http://article.gmane.org/gmane.linux.ide/26012
This patch...
* Removes all spurious IRQ whining from ahci. Spurious NCQ completion
detection was completely wrong. Spurious D2H Register FIS taught us
that some early drives send spurious D2H Register FIS with I bit set
while NCQ commands are in progress but none of recent drives does
that and even the ones which show such behavior can do NCQ fine.
* Kills all NCQ blacklist entries which were added because of spurious
NCQ completions. I tracked down each commit and verified all
removed ones are actually added because of spurious completions.
WD740ADFD-00NLR1 wasn't deleted but moved upward because the drive
not only had spurious NCQ completions but also is slow on sequential
data transfers if NCQ is enabled.
Maxtor 7V300F0 was added by 0e3dbc01d53940fe10e5a5cfec15ede3e929c918
from Alan Cox. I can only find evidences that the drive only had
troubles with spuruious completions by searching the mailing list.
This entry needs to be verified and removed if it doesn't have other
NCQ related problems.
Signed-off-by: Tejun Heo <htejun@gmail.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
drivers/ata/ahci.c | 65 ----------------------------------------------
drivers/ata/libata-core.c | 17 ------------
2 files changed, 2 insertions(+), 80 deletions(-)
Index: tree0/drivers/ata/ahci.c
===================================================================
--- tree0.orig/drivers/ata/ahci.c
+++ tree0/drivers/ata/ahci.c
@@ -1432,7 +1432,7 @@ static void ahci_port_intr(struct ata_po
struct ata_eh_info *ehi = &ap->eh_info;
struct ahci_port_priv *pp = ap->private_data;
u32 status, qc_active;
- int rc, known_irq = 0;
+ int rc;
status = readl(port_mmio + PORT_IRQ_STAT);
writel(status, port_mmio + PORT_IRQ_STAT);
@@ -1448,74 +1448,11 @@ static void ahci_port_intr(struct ata_po
qc_active = readl(port_mmio + PORT_CMD_ISSUE);
rc = ata_qc_complete_multiple(ap, qc_active, NULL);
- if (rc > 0)
- return;
if (rc < 0) {
ehi->err_mask |= AC_ERR_HSM;
ehi->action |= ATA_EH_SOFTRESET;
ata_port_freeze(ap);
- return;
- }
-
- /* hmmm... a spurious interupt */
-
- /* if !NCQ, ignore. No modern ATA device has broken HSM
- * implementation for non-NCQ commands.
- */
- if (!ap->sactive)
- return;
-
- if (status & PORT_IRQ_D2H_REG_FIS) {
- if (!pp->ncq_saw_d2h)
- ata_port_printk(ap, KERN_INFO,
- "D2H reg with I during NCQ, "
- "this message won't be printed again\n");
- pp->ncq_saw_d2h = 1;
- known_irq = 1;
- }
-
- if (status & PORT_IRQ_DMAS_FIS) {
- if (!pp->ncq_saw_dmas)
- ata_port_printk(ap, KERN_INFO,
- "DMAS FIS during NCQ, "
- "this message won't be printed again\n");
- pp->ncq_saw_dmas = 1;
- known_irq = 1;
- }
-
- if (status & PORT_IRQ_SDB_FIS) {
- const __le32 *f = pp->rx_fis + RX_FIS_SDB;
-
- if (le32_to_cpu(f[1])) {
- /* SDB FIS containing spurious completions
- * might be dangerous, whine and fail commands
- * with HSM violation. EH will turn off NCQ
- * after several such failures.
- */
- ata_ehi_push_desc(ehi,
- "spurious completions during NCQ "
- "issue=0x%x SAct=0x%x FIS=%08x:%08x",
- readl(port_mmio + PORT_CMD_ISSUE),
- readl(port_mmio + PORT_SCR_ACT),
- le32_to_cpu(f[0]), le32_to_cpu(f[1]));
- ehi->err_mask |= AC_ERR_HSM;
- ehi->action |= ATA_EH_SOFTRESET;
- ata_port_freeze(ap);
- } else {
- if (!pp->ncq_saw_sdb)
- ata_port_printk(ap, KERN_INFO,
- "spurious SDB FIS %08x:%08x during NCQ, "
- "this message won't be printed again\n",
- le32_to_cpu(f[0]), le32_to_cpu(f[1]));
- pp->ncq_saw_sdb = 1;
- }
- known_irq = 1;
}
-
- if (!known_irq)
- ata_port_printk(ap, KERN_INFO, "spurious interrupt "
- "(irq_stat 0x%x active_tag 0x%x sactive 0x%x)\n",
- status, ap->active_tag, ap->sactive);
}
static void ahci_irq_clear(struct ata_port *ap)
Index: tree0/drivers/ata/libata-core.c
===================================================================
--- tree0.orig/drivers/ata/libata-core.c
+++ tree0/drivers/ata/libata-core.c
@@ -3772,6 +3772,7 @@ static const struct ata_blacklist_entry
/* Devices where NCQ should be avoided */
/* NCQ is slow */
{ "WDC WD740ADFD-00", NULL, ATA_HORKAGE_NONCQ },
+ { "WDC WD740ADFD-00NLR1", NULL, ATA_HORKAGE_NONCQ, },
/* http://thread.gmane.org/gmane.linux.ide/14907 */
{ "FUJITSU MHT2060BH", NULL, ATA_HORKAGE_NONCQ },
/* NCQ is broken */
@@ -3790,22 +3791,6 @@ static const struct ata_blacklist_entry
{ "HTS541060G9SA00", "MB3OC60D", ATA_HORKAGE_NONCQ, },
{ "HTS541080G9SA00", "MB4OC60D", ATA_HORKAGE_NONCQ, },
{ "HTS541010G9SA00", "MBZOC60D", ATA_HORKAGE_NONCQ, },
- /* Drives which do spurious command completion */
- { "HTS541680J9SA00", "SB2IC7EP", ATA_HORKAGE_NONCQ, },
- { "HTS541612J9SA00", "SBDIC7JP", ATA_HORKAGE_NONCQ, },
- { "HDT722516DLA380", "V43OA96A", ATA_HORKAGE_NONCQ, },
- { "Hitachi HTS541616J9SA00", "SB4OC70P", ATA_HORKAGE_NONCQ, },
- { "Hitachi HTS542525K9SA00", "BBFOC31P", ATA_HORKAGE_NONCQ, },
- { "WDC WD740ADFD-00NLR1", NULL, ATA_HORKAGE_NONCQ, },
- { "WDC WD3200AAJS-00RYA0", "12.01B01", ATA_HORKAGE_NONCQ, },
- { "FUJITSU MHV2080BH", "00840028", ATA_HORKAGE_NONCQ, },
- { "ST9120822AS", "3.CLF", ATA_HORKAGE_NONCQ, },
- { "ST9160821AS", "3.CLF", ATA_HORKAGE_NONCQ, },
- { "ST9160821AS", "3.ALD", ATA_HORKAGE_NONCQ, },
- { "ST9160821AS", "3.CCD", ATA_HORKAGE_NONCQ, },
- { "ST3160812AS", "3.ADJ", ATA_HORKAGE_NONCQ, },
- { "ST980813AS", "3.ADB", ATA_HORKAGE_NONCQ, },
- { "SAMSUNG HD401LJ", "ZZ100-15", ATA_HORKAGE_NONCQ, },
/* devices which puke on READ_NATIVE_MAX */
{ "HDS724040KLSA80", "KFAOA20N", ATA_HORKAGE_BROKEN_HPA, },
next reply other threads:[~2007-12-08 0:25 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-12-08 0:25 Tejun Heo [this message]
2007-12-11 22:51 ` [stable] [PATCH -stable] libata: kill spurious NCQ completion detection Greg KH
2007-12-11 23:30 ` Tejun Heo
2007-12-13 6:03 ` patch libata-kill-spurious-ncq-completion-detection.patch queued to -stable tree gregkh
2007-12-13 6:11 ` gregkh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4759E47B.8000108@gmail.com \
--to=htejun@gmail.com \
--cc=dtorres@coral.dnsalias.org \
--cc=jeff@garzik.org \
--cc=linux-ide@vger.kernel.org \
--cc=mjt@tls.msk.ru \
--cc=stable@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).