LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Robert Hancock <hancockr@shaw.ca>
To: Jeff Garzik <jeff@garzik.org>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
	linux-ide@vger.kernel.org, Allen Martin <AMartin@nvidia.com>,
	nas@arctrix.com, B.Steinbrink@gmx.de
Subject: Re: sata_nv ADMA controller lockup investigation
Date: Thu, 15 Feb 2007 22:52:46 -0600	[thread overview]
Message-ID: <45D5389E.70300@shaw.ca> (raw)
In-Reply-To: <45D439F7.3030604@garzik.org>

Jeff Garzik wrote:
> Robert Hancock wrote:
>> It's curious that only the post-cache-flush command is having issues,
>> and normal NCQ operation seems fine. Maybe it's related to that tag 0
>> being reused repeatedly?
> 
> 
> If you take cache flush out of the equation, what happens when NCQ is 
> enabled with a queue depth of 1 (to reproduce tag-0-used-repeatedly 
> condition)?
> 
>     Jeff

I was able to reproduce it in my same test case with NCQ depth set to 1.
Of course, there were still some cache flushes going on there, so I'm not
certain that test really told us anything new. I'm rather doutbful that it's
related to reusing the same tag now, though, based on the tests I've been
doing. It may be something to do with switching between NCQ and non-NCQ
commands, maybe the controller isn't able to handle doing that too rapidly.
This patch seems to fix the problem - or at least it hasn't failed the tests that
I've run so far. It's not really ideal though, so I'd like to do some more
investigation/testing before proclaiming it as a fix. Experimentally, it
appears that 10 microseconds is not enough delay, but 20 seems to work better.

Hints from the peanut gallery remain welcome.

--- linux-2.6.20-git6edit/drivers/ata/sata_nv.c.before_hacking	2007-02-15 18:19:13.000000000 -0600
+++ linux-2.6.20-git6edit/drivers/ata/sata_nv.c	2007-02-15 22:36:02.000000000 -0600
@@ -219,6 +219,7 @@
 	void __iomem *		gen_block;
 	void __iomem *		notifier_clear_block;
 	u8			flags;
+	int			last_issue_ncq;
 };
 
 struct nv_host_priv {
@@ -1260,6 +1261,7 @@
 {
 	struct nv_adma_port_priv *pp = qc->ap->private_data;
 	void __iomem *mmio = pp->ctl_block;
+	int curr_ncq = (qc->tf.protocol == ATA_PROT_NCQ);
 
 	VPRINTK("ENTER\n");
 
@@ -1274,6 +1276,14 @@
 	/* write append register, command tag in lower 8 bits
 	   and (number of cpbs to append -1) in top 8 bits */
 	wmb();
+
+	if(curr_ncq != pp->last_issue_ncq) {
+	   	/* Seems to need some delay before switching between NCQ and non-NCQ
+		   commands, else we get command timeouts and such. */
+		udelay(20);
+		pp->last_issue_ncq = curr_ncq;
+	}
+
 	writew(qc->tag, mmio + NV_ADMA_APPEND);
 
 	DPRINTK("Issued tag %u\n",qc->tag);

  reply	other threads:[~2007-02-16  4:52 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-15  4:30 Robert Hancock
2007-02-15 10:46 ` Jeff Garzik
2007-02-16  4:52   ` Robert Hancock [this message]
2007-03-20 22:01     ` Neil Schemenauer
2007-03-20 23:24       ` Robert Hancock

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=45D5389E.70300@shaw.ca \
    --to=hancockr@shaw.ca \
    --cc=AMartin@nvidia.com \
    --cc=B.Steinbrink@gmx.de \
    --cc=jeff@garzik.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nas@arctrix.com \
    --subject='Re: sata_nv ADMA controller lockup investigation' \
    /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

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).