>In article <E16BlnL-00080m-00@the-village.bc.nu>,
>Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>
>>You still need the scsi code. There are a whole sequence of common, quite
>>complex and generic functions that the scsi layer handles (in paticular
>>error handling).
>>
>
>Well, the preliminary patches already handle _some_ common things, like
>building the proper command request for reads and writes etc, and that
>will probably continue. We'll probably have to have all the old helpers
>for things like "this target only wants to be probed on lun 0" etc
>
Personally, I think it would be a mistake to have any of the low level
driver know a thing about the ll_rw_blk.c request structs or bio
structures or similar stuff. They simply don't *need* to know those
things. Low level drivers need a command that can go to the drive and
they need a sg array. Smart hosts, aka raid controllers and the like,
could theoretically use the higher level structs for reasonable things,
but I won't make any assumptions about which ones could or couldn't use
that information because I haven't looked into it. So, making the scsi
mid layer a helper library may be OK, but for the largest number of
drivers, it really means that the call chain would likely look something
like this:
blk_dev->request_fn()
scsi_dispatch_request() // map from the major/minor to driver
driver_queue_routine(bio *)
Scsi_Cmnd = scsi_make_cmnd(bio *);
send_command(cmnd); //put it in the hardware
driver_interrupt_routine()
if (cmnd = get_errored_command()) {
// Pick out the commands that had a transfer related error
// and rework or error them out here
if(retryable_error(cmnd))
requeue_command(cmnd);
else {
cmnd->bio->completion_handler(cmnd->bio);
scsi_free_command(cmnd);
}
}
if (cmnd = get_completed_command()) {
retryable = scsi_check_sense(cmnd);
switch(retryable) {
case TRANSIENT_ERROR:
requeue_command(cmnd);
break;
case FATAL_ERROR:
case NO_ERROR:
cmnd->bio->completion_handler(cmnd->bio);
scsi_free_command(cmnd);
break;
case BUSY:
requeue_command_with_delay(cmnd);
break;
case ...
}
}
Personally, I don't like it. The queueing stuff is somewhat OK (but
requires other things to be changed to accomodate and I don't think
those things should change), but I don't want to have to deal with
result parsing in every driver. Proper result parsing is huge and easy
to get wrong. The current code gets it wrong. There are a few ways in
which it could be fixed to do the right thing without disrupting the
world. Duplicating that in all the low level drivers will be a
nightmare though. I also don't like one aspect of putting the queueing
totally in the low level drivers. That way, all of the low level
drivers are going to have to maintain A) delayable queues with variable
delay times and undelay on completion semantics (my driver already has
this, but it's been a source of pain to maintain, it would be nice if it
didn't need it) and B) ordering requirements when multiple commands for
a device operating in untagged mode are in the queue (this one isn't so
hard, but getting it wrong means things like tape drives will store
garbage when you least expect it).
Anyway, I suspect the end result would be the same either way: drivers
would end up having control over their own flow of commands. The only
difference would be how that control would be achieved. A) by changing
the mid layer to accept more driver specific parameters (aka, even
though MegaRAID may take up to 60 seconds to complete a normal command,
the aic7xxx should never take more than 10 seconds, so the default
command timeout length would be driver specific) and to honor the low
level driver's idea of what to do on a timeout (let the low level driver
tell the mid layer what action to take, and that should be the limit of
the mid layers error handling "brains", performing those actions). Or
B) removing the mid layer from the picture all together (as much as
possible anyway, it (or a similar variant) will still likely be needed
on queueing just to map from major/minor to driver unless we change the
device allocation scheme or something) and then having the low level
driver call into helper routines.
>
>I disagree about the error handling, though.
>
>Traditionally, the timeouts and the reset handling was handled in the
>SCSI mid-layer, and it was a complete and utter disaster. Different
>hosts simply wanted so different behaviour that it's not even funny.
>
That's not really accurate. It was too opaque, sure. But for what it
was it did a decent job. The mid layer driver was trying to make
generic timeout decisions without the benefit of the low level driver's
knowledge of the current bus state. For example, 20 commands may time
out at once, but in reality, only one of those commands is probably
holding up the SCSI bus. The low level drivers can (usually) look at
their card to see which command is *really* the hold up. Then, they
could have all the other commands simply put back to sleep without doing
anything and take appropriate action against the holdup command. The
current mid layer (at least in the old error handling) couldn't do that.
The new_eh code attempts to allow drivers to tell them these things by
use of the strategy function. In reality, that's all you need. That's
the driver's ability to tell the mid layer *how* to proceed on any given
command.
>
>Timeouts for different commands were so different that people ended up
>making most timeouts so long that they no longer made sense for other
>commands etc.
>
Not accurate here either. For most commands across all controllers,
timeouts are pretty uniform (the timeout to read a CD-ROM for instance
is pretty constant). The timeouts *only* started to vary when you ran
across smart RAID controllers that were doing too much work rebuilding
things and didn't respond to your requests in a timely fashion. And
that only applys to their logical drives. It would be pretty easy to
just allow disk timeouts to be adjusted on a disk by disk basis to a
reasonable default for that controller and solve this problem.
>
>Other device drivers have been able to handle timeouts and errors on
>their own before, and have _not_ had the kinds of horrendous problems
>that the SCSI layer has had.
>
If you use the new eh strategy function (or so I understand it, if I'm
wrong here then I'll take the time to *make* myself right by changing
the code), then you are essentially allowing your driver to take control
of *what* happens, and the mid layer timeout code becomes nothing more
than a glorified timer creation, monitoring, and teardown framework that
happens to be plugged into the queueing and completion framework so it
can notice new commands and when old commands are done. Nothing more.
And that's all a SCSI driver that wants to do its own thing really
needs. Done properly, this could be a "good" thing. Done poorly,
everyone will hate it. But, the ability is there to make your driver do
what you want with the strategy call in.
>
>We'll see what the details will end up being, but I personally think
>that it is a major mistake to try to have generic error handling. The
>only true generic thing is "this request finished successfully / with an
>error", and _no_ high-level retries etc. It's up to the driver to decide
>if retries make sense.
>
Well, I disagree on this. I think asking all those drivers to deal with
sense data and keep up with changing standards and new sense returns on
new devices, etc. is just asking for a lot of out of date drivers that
do the wrong thing. Sense parsing and decision making is *device*
specific, not controller specific, and I don't think it has any place
being in the controller's responsibility list. You might as well start
asking eth drivers to not only check checksums on packets but also
protocol flags before passing the packet up to the IP or TCP layers and
to perform all the TCP retrans operations in themselves instead of in
the TCP layer.
>
>(Often retrying _doesn't_ make sense, because the firmware on the
>high-end card or disk itself may already have done retries on its own,
>and high-level error handling is nothing but a waste of time and causes
>the error notification to be even more delayed).
>
> Linus
>-
>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/
>
-
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/