On Tue, 2008-09-30 at 10:11 -0700, Mike Anderson wrote: > James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > > On Tue, 2008-09-30 at 11:08 -0400, Alan Stern wrote: > > > On Tue, 30 Sep 2008, James Bottomley wrote: > > > > > > > I don't really think this is the right approach, since the retry case > > > > needs to be split apart again. > > > > > > > > The only time scsi_requeue_command() needs to be called is if the > > > > request completes successfully but has leftovers. The reason is that > > > > the command will be different next time around, so it has to be > > > > re-prepared. In all the other cases, the same command can be reused. > > > > This will have the knock on effect of not resetting the timers or the > > > > counters, so it has to be done carefully. > > > > > > All right. (Incidentally, do you happen to know the derivation of > > > "knock on effect"? The American form, "side effect", seems more > > > self-explanatory.) > > > > The etymology is probably from Rugby: a knock on takes the ball further > > than allowed by the rules, usually as an unintended consequence of some > > other action. > > > > > > Of the three requeue cases: > > > > > > > > UNIT_ATTENTION needs immediate retry > > > > NOT_READY needs delayed retry > > > > ILLEGAL_REQUEST with cmd switch (assuming we still do it) needs > > > > immediate retry > > > > > > If the command is switched from 10-byte to 6-byte form, won't it need > > > to be re-prepared? > > > > Yes, sorry, that one needs a re-prepare requeue. > > > > > > DID_RESET is arguable either way, but probably needs delayed. > > > > > > > > immediate requeue is done by: > > > > > > > > scsi_queue_insert(cmd, SCSI_MLQUEUE_EH_RETRY); > > > > > > > > And delayed by > > > > > > > > scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY); > > > > > > Easily fixed. And it looks like neither of these needs a call to > > > scsi_next_command(), right? > > > > Right, which is a nice side effect. > > scsi_finish_command is only called from scsi_eh_flush_done_q (newer > patches moves this to scsi_attempt_requeue_command) and scsi_softirq_done. > scsi_io_completion is only called from scsi_finish_command. In > scsi_softirq_done we just called scsi_decide_disposition to map the > result. Could some (or all) of the sense mapping be moved to > scsi_decide_disposition? It seems incorrect to decode this same data in > more than one location plus in some cases could prevent device handlers > from the full control they need. Is there some path or behavior that I am > missing? Well, it already is done in scsi_decide_disposition() with a call to scsi_check_sense(). However, there's a slight problem in that some senses need to be interpreted differently depending on what the ULD is which leads to the current 3 tier system (before in decide disposition, during in ULD done and after in scsi_io_completion). The discriminators in the done functions do look to be duplicates, so I suppose they could be collapsed. > Also since previous mid retry changes are related to retry behavior borrowing > this thread for a related request.... > > James, It would be good if you have time to look at the repost of mid > retry changes and if they are ok would you consider applying them plus > these changes. They're on my list ... it's just there's rather a lot of them and I was hoping someone else would get there first ... > It would be good to also have a refresh of > scsi-post-merge-2.6 tree with Jens tree. I am running testing now, but my > tree needed a lot of merging assistance and it would be good to know what > others are testing is the same. > http://thread.gmane.org/gmane.linux.scsi/44715 Um, it shouldn't need any merging assistance ... it rebases into linux-next just fine. Are you sure you're doing the right thing. To get this tree, you need to set it up as a remote git remote add -f scsi-postmerge git://git.kernel.org//pub/scm/linux/kernel/git/jejb/scsi-post-merge-2.6.git in a tree you want to use it in (must contain at least scsi-misc and Jens block#for-2.6.28) and rebase the postmerge tree into your current git rebase --onto master scsi-postmerge/merge-base scsi-postmerge/master This will create a temporary branch for you to play with (git branch will show * (no branch) master ). If you play around with it and find it to be OK, you can create a new branch for it git branch <new branch name> git checkout <new branch name> And the temporary branch will become permanent (you can even force your temporary to become master with git branch -f master) > Since this change plus Mike C's and mine effect retry / completion > behavior it would be good to test these changes all together. James -- 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
