2015-03-10 19:20 GMT+09:00 Gilad Broner <gbroner@xxxxxxxxxxxxxx>: >>> +static bool inject_cmd_hang_tr(struct ufs_hba *hba) >>> +{ >>> + int tag; >>> + >>> + tag = find_first_bit(&hba->outstanding_reqs, hba->nutrs); >>> + if (tag == hba->nutrs) >>> + return 0; >>> + >>> + __clear_bit(tag, &hba->outstanding_reqs); >>> + hba->lrb[tag].cmd = NULL; >>> + __clear_bit(tag, &hba->lrb_in_use); >> >> hba->lrb_in_use is a bitmap set by test_and_set_bit_lock(). So >> this should be cleared by clear_bit_unlock(). > > You are correct. Thanks. > >> >> And as soon as the bit corresponds to this slot in hba->lrb_in_use is >> cleared, this slot could be reused. But if the request corresponds >> to the slot is not completed yet, it could sacrifice the new request, >> too. So should we only inject into the commands which have been >> completed like this? > > Please note that we only clear the bit in hba->lrb_in_use. scsi_done is > not called for this request. Therefore, the tag is not yet free in the > block layer and next calls for queuecommand will not pass down this tag to > be used in the UFS driver. So there is no danger of a new request being > sacrificed. OK, I see there is no danger as far as the commands are comming through queuecommand(). But what about the query requests? PATCH 1/4 in this series has added ioctl interface for query request which also acquires a tag in hba->lrb_in_use through ufshcd_get_dev_cmd_tag(). Although it is very difficult to happen, is it possible for new query requests to be clashed by inject_cmd_hang_tr() in the same way I described earlier? -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html