lkml.org 
[lkml]   [2003]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Silicon Image 3112A SATA trouble
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:
> >>>
> >>>
> >>>>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.

I don't see why, it's exactly 2 x non-errata case.

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

Of course, you would always complete these fully. But having partial
completions at the lowest layer gives it to you for free. non-errata
case uses the exact same path, it just happens to complete 100% of the
request all the time.

> >>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.

Right. But as you mention, error handling is a partial completion by
nature (almost always).

> >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 :)

That's good :). But even given that, error handling is usually the less
tested path (by far). I do commend your 'keep it simple', I think that's
key there.

> >>>>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.

Precisely. But there are more than one workable way to fix it :)

--
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:59    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site