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. hosted at Digital Ocean