linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin LaHaise <bcrl@kvack.org>
To: Jeff Garzik <jgarzik@pobox.com>
Cc: linux-kernel@vger.kernel.org,
	"linux-ide@vger.kernel.org" <linux-ide@vger.kernel.org>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>
Subject: Re: [rfc/patch] libata -- port configurable delays
Date: Fri, 13 May 2005 16:03:12 -0400	[thread overview]
Message-ID: <20050513200312.GA6555@kvack.org> (raw)
In-Reply-To: <4284FC6E.7060300@pobox.com>

On Fri, May 13, 2005 at 03:13:50PM -0400, Jeff Garzik wrote:
> This delay is based on field experience, rather than spec.  I'm open to 
> making this optional, as you have done.  Some issues related to this 
> delay, to consider:

Do you have the original bug reports so we know which chipsets / drive 
combinations are problematic?

> 1) Nothing in life is free.  This delay is useful on some platforms, 
> because banging away at the Status register for extended periods of time 
> can cause an insane amount of PCI IO traffic.  Removing the delay just 
> moves the punishment from one area to another.

In this case the time to complete the task is reduced by 18%, so the 
delay is definately excessive.  That 18% is remarkably close to the 
19.5% hit for __delay in the profile results.

> 2) In a few controllers, the SATA<->FIS emulation can go kerflooey if 
> you bang the Status register 'too hard'.

The patch only enables it on one variant of ICH6 SATA so far.

> 3) IIRC some rare PATA devices don't like having their Status register 
> banged "too hard".  No data, just a vague memory.
> 
> 4) It may be worthwhile to rewrite the loop to check the Status register 
> _first_, then delay.
> 
> Finally, simply disabling the delay is IMO _far_ too dangerous on such a 
> popular driver (ata_piix).

Have you a better suggestion for blacklisting devices?

> I would be conservative, and create a module option for libata (not 
> ata_piix) which allows a user to globally disable the delay.  And make 
> sure that option defaults to 'delay', the current behavior.

This option is undesirable for hardware which behaves correctly.  The 
vast majority of users aren't going to be fiddling with hdparm options
(I certainly don't want to see modern SATA being a repeat of the magic 
handwaving the IDE driver and DMA went through for years).

At the very least the option should be introduced and new drivers should 
start life without these legacy delays in place.  If doing it on a per 
port interface isn't good enough, what is?  Globally seems worse to me 
as then any system with a mix of old and new hardware isn't going to do 
the right thing.

		-ben
-- 
"Time is what keeps everything from happening all at once." -- John Wheeler

  reply	other threads:[~2005-05-13 20:01 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-05-13 18:58 [rfc/patch] libata -- port configurable delays Benjamin LaHaise
2005-05-13 19:13 ` Jeff Garzik
2005-05-13 20:03   ` Benjamin LaHaise [this message]
2005-05-13 21:52     ` Alan Cox
2005-05-13 23:07       ` Jeff Garzik
2005-05-14  1:51   ` Mark Lord
2005-05-13 19:17 ` Jeff Garzik
2005-05-13 21:20 ` Alan Cox
2005-05-13 23:21   ` Jeff Garzik
2005-05-14 21:11     ` Alan Cox
2005-05-14 21:47       ` 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=20050513200312.GA6555@kvack.org \
    --to=bcrl@kvack.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.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).