On 2014-11-10 08:56, Christoph Hellwig wrote:
The current SCSI handling suffers from large amounts of duplicate code, and a general confusion of multiple concepts of tagging.This series tries to reduce the amount of code, and introduce two separate clear concepts of tagging: a) a driver can request block-level tagging to always have a valid index assigned to cmd->request->tag by the block layer. This is already automatically done for the blk-mq case, but this gives the optional legacy request taggign similar semantics. b) the concept of a SCSI "tagged command" is now entirely separate from providing an actual tag, and always provided in a flag in the SCSI command. The driver does not need to opt-in this information, but it doesn't have to use it. While this series removes 750 lines of code and provides a much cleaner driver API it also opens up new questions: - how is the change_queue_type API supposed to be used for most drivers? It only changes the tag type from none to simple or back, but except for the special implementation in the 53c700 driver doesn't change the queue depth, which might cause it to issue multiple non-tagged command. Fortunately most drivers never look at this information, but then again changing the queue type is useless to start with. - for those drivers looking at the command tagged information we'd need to quiesce the LUN. No driver but the 53c700 driver does that, and the 53c700 does it at a target-level, which despite a comment claiming it's needed doesn't seem to make sense given the code. If we can make sure to quience all LUNs we could avoid the per-command flag for tagged commands and always look at the scsi_device flag. - similarly, do we need any synchronization when answering a tag message reject? - queue ramp down may drop to untagged mode, but queue ramp up never reverseѕ that. - the ->tagged_supported and ->simple_tags flags are bit-fields without clear locking protection. We probably want to move these and similar fields to a proper atomic bit flags member. - should the MSG_*_TAG fields move to the SPI transport class? That's where they are defined, and besides SPI drivers they are only (ab)used by the target code now. Changes since V1: - small fixes for various review comments
Looks good to me, nice cleanup and the diffstat is excellent :-) -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
