On 11/22/21 6:46 PM, Bart Van Assche wrote: > On 11/22/21 12:58 AM, John Garry wrote: >> On 19/11/2021 19:57, Bart Van Assche wrote: >>> +/** >>> + * scsi_get_internal_cmd - Allocate an internal SCSI command >>> + * @q: request queue from which to allocate the command. This >>> request queue may >>> + * but does not have to be associated with a SCSI device. This >>> request >>> + * queue must be associated with a SCSI tag set. See also >>> + * scsi_mq_setup_tags(). >>> + * @data_direction: Data direction for the allocated command. >>> + * @flags: Zero or more BLK_MQ_REQ_* flags. >>> + * >>> + * Allocates a request for driver-internal use. The tag of the >>> returned SCSI >>> + * command is guaranteed to be unique. >>> + */ >>> +struct scsi_cmnd *scsi_get_internal_cmd(struct request_queue *q, >>> + enum dma_data_direction data_direction, >>> + blk_mq_req_flags_t flags) >> >> I'd pass the Scsi_Host or scsi_device rather than a request q, so maybe: >> >> struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev, ..) >> struct scsi_cmnd *scsi_host_get_internal_cmd(struct Scsi_Host *shost, ..) > > Passing a request queue pointer as first argument instead of a struct > scsi_device is a deliberate choice. In the UFS driver (and probably also > in other SCSI LLDs) we want to allocate internal requests without these > requests being visible in any existing SCSI device statistics. Creating > a new SCSI device for the allocation of internal requests is not a good > choice because that new SCSI device would have to be assigned a LUN > number and would be visible in sysfs. Hence the choice to allocate > internal requests from a request queue that is not associated with any > SCSI device. > It's actually a bit more involved. The biggest issue is that the SCSI layer is littered with the assumption that there _will_ be a ->device pointer in struct scsi_cmnd. If we make up a scsi_cmnd structure _without_ that we'll have to audit the entire stack to ensure we're not tripping over a NULL device pointer. And to make matters worse, we also need to audit the completion path in the driver, which typically have the same 'issue'. Case in point: # git grep -- '->device' drivers/scsi | wc --lines 2712 Which was the primary reason for adding a stub device to the SCSI Host; simply to avoid all the pointless churn and have a valid device for all commands. The only way I can see how to avoid getting dragged down into that rat-hole is to _not_ returning a scsi_cmnd, but rather something else entirely; that's the avenue I've exploited with my last patchset which would just return a tag number. But as there are drivers which really need a scsi_cmnd I can't se how we can get away with not having a stub scsi_device for the scsi host. And that won't even show up in sysfs if we assign it a LUN number beyond the addressable range; 'max_id':0 tends to be a safe choice here. Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@xxxxxxx +49 911 74053 688 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
