linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Cox <alan@linux.intel.com>
To: Tejun Heo <tj@kernel.org>
Cc: Jeff Garzik <jgarzik@pobox.com>,
	linux-ide@vger.kernel.org, tom.leiming@gmail.com,
	seth.heasley@intel.com
Subject: Re: [RFC PATCH #upstream-fixes] libata: don't use 32bit PIO for small transfers
Date: Tue, 6 Sep 2011 10:49:52 +0100	[thread overview]
Message-ID: <20110906104952.0c930a93@bob.linux.org.uk> (raw)
In-Reply-To: <20110906042101.GE18425@mtj.dyndns.org>

On Tue, 6 Sep 2011 13:21:01 +0900
Tejun Heo <tj@kernel.org> wrote:

> Some configurations have problems with using 32bit PIO on ATAPI cdb
> transfers.  The most likely cause is different division of payload
> across FISes but hasn't been confirmed.  There isn't much to be gained
> by using 32bit PIO for small transfers.  Play it safe and use 16bit
> PIO for transfers smaller than 512 bytes.

This seems to me to be the wrong approach as it hurts lots of other
commands unnecessarily and may cause regressions on other setups
because their behaviour is unnecessarily changed and performance
reduced.

The problem report is specific to Sandybridge in IDE mode, which is a
recent controller so it should be chased up with the Intel folks who
submitted the device ID as an apparent hardware problem - that way if
it is a chipset problem (and I have no idea if it is) it might get a
proper workaround, or fixed in future.

Secondly it's a SATA specific problem.

Thirdly we've been doing 32bit PIO on old IDE since it was invented.

This seems an extreme reaction for a report of a single specific
problem on a single specific device and controller.

So NACK.

Add a piix_sata_data_xfer32_maybe() function to the ata_piix driver
specifically for this chipset.


I've added Seth who submitted the relevant PCI identifers to the email.
I'm a bit at a loss to understand why none of the previous discussion
or patch was Cc'd in his direction ?

Seth - the bug report is
https://bugzilla.kernel.org/show_bug.cgi?id=40592

and I guess the underlying question is "Are there any known 32bit PIO
errata that would explain this ?"

Before the changes go in it might also be worth testing CD burning and
audio ripping etc to see if the problem is one dependent on transfer
size or the fact the transfer is the cdb on a SATA transfer, so that
any fixing is done right.

Alan

  reply	other threads:[~2011-09-06  9:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-06  4:21 [RFC PATCH #upstream-fixes] libata: don't use 32bit PIO for small transfers Tejun Heo
2011-09-06  9:49 ` Alan Cox [this message]
2011-09-14  7:44   ` Ming Lei
2011-09-14 18:51     ` Alan Cox
2011-09-07 10:41 ` Alan Cox

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=20110906104952.0c930a93@bob.linux.org.uk \
    --to=alan@linux.intel.com \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=seth.heasley@intel.com \
    --cc=tj@kernel.org \
    --cc=tom.leiming@gmail.com \
    /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).