Re: [PATCH] Re: Linux v2.6.22-rc3

[Tejun Heo <htejun@gmail.com>]

Hello,

Jeff Garzik wrote:
> On Thu, Jun 07, 2007 at 01:56:11PM -0700, Linus Torvalds wrote:
>> On Thu, 7 Jun 2007, Tejun Heo wrote:
>>> Ah.. okay.  Now I see what's going on.  Jeff, this is another device
>>> which doesn't set nsect and lbal to 1 after reset.  Gregor, please try
>>> the attached patch.
> 
>> Tejun, since Jeff is apparently traveling this week, and Gregor tested the 
>> patch successfully (and it looks sane anyway - why in Gods name _would_ we 
>> care what the initial setting of nsect/lbal is?), can you send this in 
>> with the changelog and sign-off?
> 
> Ack'ing the sata_promise change was easy, but with this one it would
> be nice to wait a bit before changing the core probe code that [now]
> every ATA setup goes through, based on a single bug report.

Yeap, I'm not so sure about the change either.  The posted patch is more
of proof-of-concept.

Currently we have three related reports.

* this one
* IT8212 raid mode Alan is working on
* (likely) pata_pcmcia http://thread.gmane.org/gmane.linux.kernel/530099

> The values assist in detecting ghost devices (same device appearing
> on both master and slave) and TF register malfunctions, and I would
> appreciate not breaking _that_ so late in 2.6.22-rc for a single
> report.  Thankfully we have -some- ghost device prevention code
> elsewhere, but this is part of it.

Upto 2.6.21, if the same condition triggers, it delays 30secs and just
continue, so I don't think it was a worthy protection against ghost
devices or TF malfunction.  The only protection it offers is preventing
libata from accessing slave's status register too early.  SRST sequence
looks like the following.

1. SRST raised, delay, and cleared

2. libata waits for master device readiness after 150ms pause.  After
finishing reset, if slave is present, master waits for PDIAG- assert.

3. slave asserts PDIAG-, master asserts readiness.  libata goes on to
check for the slave device nsect/lbal.

4. slave sets nsect/lbal, libata goes on to check readiness

5. slave asserts readiness, SRST complete.

So, the nsect/lbal check is meaningful iff 1. slave lies about device
readiness after releasing PDIAG- but before setting nsect/lbal, or 2.
master/slave register selection is flimsy.

I don't think the first one is probable considering BSY is supposed to
set when SRST is received.  This is pretty fundamental in the protocol
and necessary for the device to work properly as master, so I think this
is one of few things we can rely upon.

To me, the second one sounds pretty far fetched too but, well, it's ATA.
 Who knows?

How about limiting nsect/lbal wait duration?  Say, 100ms or 500ms?  That
can somewhat ease our paranoia and should show acceptable behavior for
braindead devices too.

Thanks.

-- 
tejun