On 11/23/21 9:46 AM, Bart Van Assche wrote:
On 11/23/21 12:13 AM, Hannes Reinecke wrote:
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.
There is no risk that the scsi_cmnd.device member will be dereferenced for
internal requests allocated by the UFS driver. But I understand from your
email that making sure that the scsi_cmnd.device member is not NULL is
important for other SCSI LLDs. I will look into the approach of associating
a stub SCSI device with internal requests.
(replying to my own email)
Hi Hannes,
Allocating a struct scsi_device for internal requests seems tricky to me. The
most straightforward approach would be to call scsi_alloc_sdev(). However, that
function accepts a scsi_target pointer and calls .slave_alloc(). So a
scsi_target structure would have to be set up before that function is called and
SCSI LLDs would have to be audited to verify that .slave_alloc() works fine for
the H:C:I:L tuple assigned to the fake SCSI device. Additionally, how should the
inquiry data be initialized that is filled in by scsi_add_lun()?
Since I do not use SCSI hardware that needs a scsi_device to be associated with
internal requests, I prefer that this functionality is implemented in a future
patch series. Changing the hba->host->internal_queue occurrences in the UFS
driver into something like hba->host->internal_sdev->request_queue once this
functionality is implemented should be easy.
Thanks,
Bart.
