On Sat, 20 Sep 2008, James Bottomley wrote: > What I mean is that I can't find an error case that's currently shown > scsi_end_request(cmd, -EIO, this_count, 1) where requeuing after > completing only the currently attempted transfer is valid. If this had > all been done as a single transaction, we'd have killed everything at > this point. Just because we split the request into multiple > transactions doesn't mean we should go back around and try a new > transaction after we hit an error. Yes, that makes sense. > > They end up doing this: > > > > scsi_end_request(cmd, -EIO, this_count, 1); > > > > when in fact they should do this: > > > > scsi_end_request(cmd, -EIO, 0, 1); > > > > (where the -EIO value is ignored). > > Actually, no ... that's just equivalent to scsi_requeue_command(q, cmd) > which is done at several places in the code (correctly) for sense errors > that imply the whole lot should be retried (actually, it's assuming > good_bytes is zero). Except that those places don't do the blk_noretry_request test. Should they? And even if they do, what's to prevent an infinite retry loop? > OK, so look at the current code in scsi_io_completion where we call > scsi_end_request(cmd, -EIO, this_count, 1): > > UNIT ATTENTION for removable medium (means medium changed) > ILLEGAL REQUEST where there's no command resize fallback > NOT READY for unknown (non retryable) reasons > VOLUME OVERFLOW > > In none of these cases do we want any form of requeuing, we want to kill > the entire request. I take your point. Which leads to the question: Why was the code ever calling scsi_end_request(cmd, -EIO, this_count, 1) in the first place? Apparently all of these paths should be setting the last argument to 0, always. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
