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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
