Re: Silicon Image 3112A SATA trouble

Jeff Garzik
2003-11-30T20:35:47Z

Jens Axboe wrote:
> On Sun, Nov 30 2003, Jeff Garzik wrote:
>>Since the hardware request API is (and must be) completely decoupled
>>from struct request API, I can achieve 1.5 x non-errata case.
>
> Hmm I don't follow that... Being a bit clever, you could even send off
> both A and B parts of the request in one go. Probably not worth it
> though, that would add some complexity (things like not spanning a page,
> stuff you probably don't want to bother the driver with).
[...]
> Indeed. The partial completions only exist at the driver -> block layer
> (or -> scsi) layer, not talking to the hardware. The hardware always
> gets 'a request', if that just happens to be only a part of a struct
> request so be it.
[...]
> Sure yes, the fewer completions the better. Where do you get the 1.5
> from? You need to split the request handling no matter what for the
> errata path, I would count that as 2 completions.

Taskfile completion and struct request completion are separate. That
results in

* struct request received by libata
* libata detects errata
* libata creates 2 struct ata_queued_cmd's
* libata calls ata_qc_push() 2 times
* Time passes
* ata_qc_complete called 2 times
Option 1: {scsi|block} complete called 2 times, once for each taskfile
Option 2: {scsi|block} complete called 1 time, when both taskfiles are done

one way: 2 h/w completions, 1 struct request completion == 1.5
another way: 2 h/w completions, 2 struct request completions == 2.0

Maybe another way of looking at it:
It's a question of where the state is stored -- in ata_queued_cmd or
entirely in struct request -- and what are the benefits/downsides of each.

When a single struct request causes the initiation of multiple
ata_queued_cmd's, libata must be capable of knowing when multiple
ata_queued_cmds forming a whole have completed. struct request must
also know this. _But_. The key distinction is that libata must handle
multiple requests might not be based on sector progress.

For this SII errata, I _could_ do this at the block layer:
ata_qc_complete() -> blk_end_io(first half of sectors)
ata_qc_complete() -> blk_end_io(some more sectors)

And the request would be completed by the block layer (right?).

But under the hood, libata has to handle these situations:
* One or more ATA commands must complete in succession, before the
struct request may be end_io'd.
* One or more ATA commands must complete asynchronously, before the
struct request may be end_io'd.
* These ATA commands might not be sector based: sometimes aggressive
power management means that libata must issue and complete a PM-related
taskfile, before issuing the {READ|WRITE} DMA passed to it in the struct
request.

I'm already storing and handling this stuff at the hardware-queue level.
(remember hardware queues often bottleneck at the host and/or bus
levels, not necessarily the request_queue level)

So what all this hopefully boils down to is: if I have to do "internal
completions" anyway, it's just more work for libata to separate out the
2 taskfiles into 2 block layer completions. For both errata and
non-errata paths, I can just say "the last taskfile is done, clean up"

Yet another way of looking at it:
In order for all state to be kept at the block layer level, you would
need this check:

if ((rq->expected_taskfiles == rq->completed_taskfiles) &&
(rq->expected_sectors == rq->completed_sectors))
the struct request is "complete"

and each call to end_io would require both a taskfile count and a sector
count, which would increment ->completed_taskfiles and ->completed_sectors.

Note1: s/taskfile/cdb/ if that's your fancy :)
Note2: ->completed_sectors exists today under another name, yes, I know :)

Jeff