Re: Silicon Image 3112A SATA trouble

[Jeff Garzik <jgarzik@pobox.com>]

Jens Axboe wrote:
> On Sun, Nov 30 2003, Jeff Garzik wrote:
> 
>>Jens Axboe wrote:
>>
>>>On Sun, Nov 30 2003, Jeff Garzik wrote:
>>>
>>>
>>>>Jens Axboe wrote:
>>>>
>>>>
>>>>>On Sun, Nov 30 2003, Jeff Garzik wrote:
>>>>>
>>>>>
>>>>>>fond of partial completions, as I feel they add complexity, 
>>>>>>particularly so in my case:  I can simply use the same error paths for 
>>>>>>both the single-sector taskfile and the "everything else" taskfile, 
>>>>>>regardless of which taskfile throws the error.
>>>>>
>>>>>
>>>>>It's just a questions of maintaining the proper request state so you
>>>>>know how much and what part of a request is pending. Requests have been
>>>>>handled this way ever since clustered requests, that is why
>>>>>current_nr_sectors differs from nr_sectors. And with hard_* duplicates,
>>>>>it's pretty easy to extend this a bit. I don't see this as something
>>>>>complex, and if the alternative you are suggesting (your implementation
>>>>>idea is not clear to me...) is to fork another request then I think it's
>>>>>a lot better.
>>>>
>>>>[snip howto]
>>>>
>>>>Yeah, I know how to do partial completions.  The increased complexity 
>>>>arises in my driver.  It's simply less code in my driver to treat each 
>>>>transaction as an "all or none" affair.
>>>>
>>>>For the vastly common case, it's less i-cache and less interrupts to do 
>>>>all-or-none.  In the future I'll probably want to put partial 
>>>>completions in the error path...
>>>
>>>
>>>Oh come one, i-cache? We're doing IO here, a cache line more or less in
>>>request handling is absolutely so much in the noise. 
>>>
>>>What are the "increased complexity" involved with doing partial
>>>completions? You don't even have to know it's a partial request in the
>>>error handling, it's "just the request" state. Honestly, I don't see a
>>>problem there. You'll have to expand on what exactly you see as added
>>>complexity. To me it still seems like the fastest and most elegant way
>>>to handle it. It requires no special attention on request buildup, it
>>>requires no extra request and ugly split-code in the request handling.
>>>And the partial-completions come for free with the block layer code.
>>
>>libata, drivers/ide, and SCSI all must provide internal "submit this 
>>taskfile/cdb" API that is decoupled from struct request.  Therefore, 
> 
> 
> Yes
> 
> 
>>submitting a transaction pair, or for ATAPI submitting the internal 
>>REQUEST SENSE, is quite simple and only a few lines of code.
> 
> 
> SCSI already does these partial completions...
> 
> 
>>Any extra diddling of the hardware, and struct request, to provide 
>>partial completions is extra code.  The hardware is currently set up to 
>>provide only "it's done" or "it failed" information.  Logically, then, 
>>partial completions must be more code than the current <none> ;-)
> 
> 
> That's not a valid argument. Whatever you do, you have to add some lines
> of code.

Right.  But the point with mentioning "decouple[...]" above was that the 
most simple path is to submit two requests to hardware, and then a 
single function call into {scsi|block} to complete the transaction.

Current non-errata case:  1 taskfile, 1 completion func call
Upcoming errata solution:  2 taskfiles, 1 completion func call
Your errata suggestion seems to be:  2 taskfiles, 2 completion func calls

That's obviously more work and more code for the errata case.

And for the non-errata case, partial completions don't make any sense at 
all.


>>WRT error handling, according to ATA specs I can look at the error
>>information to determine how much of the request, if any, completed
>>successfully.  (dunno if this is also doable on ATAPI)  That's why
>>partial completions in the error path make sense to me.
> 
> 
> ... so if you do partial completions in the normal paths (or rather
> allow them), error handling will be simpler. And we all know where the

In the common non-errata case, there is never a partial completion.


> hard and stupid bugs are - the basically never tested error handling.

I have :)  libata error handling is stupid and simple, but it's also 
solid and easy to verify.  Yet another path to be honed, of course :)


>>>>see.  I'll implement whichever is easier first, which will certainly
>>>>be better than the current sledgehammer limit.  Any improvement over
>>>>the 
>>>
>>>
>>>Definitely, the current static limit completely sucks...
>>>
>>>
>>>
>>>>current code will provide dramatic performance increases, and we can
>>>>tune after that...
>>>
>>>
>>>A path needs to be chosen first, though.
>>
>>The path has been chosen:  the "it works" solution first, then tune.
>>:)
> 
> 
> Since one path excludes the other, you must choose a path first. Tuning
> is honing a path, not rewriting that code.

The first depends on the second.  The "it works" solution creates the 
path to be honed.

	Jeff