Re: [PATCH v2 05/20] scsi: core: Add support for internal commands

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux