From: Jeff Garzik <jeff@garzik.org>
To: Tejun Heo <tj@kernel.org>
Cc: "linux-ide@vger.kernel.org" <linux-ide@vger.kernel.org>,
Alan Cox <alan@lxorguk.ukuu.org.uk>,
Hans Werner <hwerner4@gmx.de>,
Sergei Shtylyov <sshtylyov@ru.mvista.com>
Subject: Re: [PATCH #upstream 2/2] libata: implement spurious irq handling for SFF and apply it to piix
Date: Thu, 21 Jan 2010 11:52:52 -0500 [thread overview]
Message-ID: <4B588664.4040602@garzik.org> (raw)
In-Reply-To: <4B579582.4050806@kernel.org>
On 01/20/2010 06:45 PM, Tejun Heo wrote:
> Hello,
>
> On 01/21/2010 04:33 AM, Jeff Garzik wrote:
>> Overall, as long as the drive is in Bus-Idle mode, it should be safe to
>> go ahead and read Status, for pretty much every controller and drive.
>
> Hmmm... I was a bit worried about the case Alan mentioned several
> times where access to AltStatus while data transfer is going on can
> lead to silent data corruption.
If a drive is in Bus-Idle, as I mentioned, then there is no active data
transfer.
>> I would make exception only for the new SATA FIS-based controllers,
>> where we know that hitting Status is likely both pointless and wasteful,
>> as well as being superfluous because the newer FIS-based controllers all
>> have irq status registers.
>
> FIS-based ones need their own interrupt handlers anyway so,
> fortunately, things like irq_check callback isn't necessary to begin
> with. :-)
Yep.
>> Additionally, I think we should have a "fast-timeout" and
>> "slow-timeout", whereby we check Status after a short period (5
>> seconds?) to make sure we did not lose an interrupt. If Status is !BSY,
>> then we can proceed with handling qc success/failure immediately.
>
> Does this happen often? What I find more common is just plain
> timeouts, so I think it would improve our exception latency if we
> apply different timeouts for each trial. ie. For the first RW try,
> set the timeout to 7 secs. For the second, 15 and then to 30. This
> wouldn't harm the correctness while allowing libata to react much
> faster to transient failures.
Lost interrupts do not happen often, but they do happen. Google finds
plenty of examples.
> Another thing is I can think of which can improve our robustness is
> dynamic irqpoll support such that when screaming IRQ happens, IRQ
> subsystem not only shuts down the IRQ line but also begins selectively
> irqpolling it.
Does this ever happen when data transfer is active? AFAIK this happens
during probe or reset or set-xfer or bus-idle or some other auxiliary
moment in time.
Jeff
next prev parent reply other threads:[~2010-01-21 16:52 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-19 1:46 [PATCH #upstream 1/2] libata: cleanup ata_sff_interrupt() Tejun Heo
2010-01-19 1:49 ` [PATCH #upstream 2/2] libata: implement spurious irq handling for SFF and apply it to piix Tejun Heo
2010-01-20 19:33 ` Jeff Garzik
2010-01-20 23:45 ` Tejun Heo
2010-01-21 12:14 ` Alan Cox
2010-01-21 16:52 ` Jeff Garzik [this message]
2010-01-22 0:36 ` Tejun Heo
2010-01-20 19:29 ` [PATCH #upstream 1/2] libata: cleanup ata_sff_interrupt() Jeff Garzik
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=4B588664.4040602@garzik.org \
--to=jeff@garzik.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=hwerner4@gmx.de \
--cc=linux-ide@vger.kernel.org \
--cc=sshtylyov@ru.mvista.com \
--cc=tj@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.