Re: [PATCH 01/84] scsi: core: Use a member variable to track the SCSI command submitter

[Bart Van Assche <bvanassche@acm.org>]

On 9/28/21 9:54 AM, Benjamin Block wrote:
> On Fri, Sep 17, 2021 at 05:04:44PM -0700, Bart Van Assche wrote:
>> +enum scsi_cmnd_submitter {
>> +	BLOCK_LAYER = 0,
>> +	SCSI_ERROR_HANDLER = 1,
>> +	SCSI_RESET_IOCTL = 2,
>> +} __packed;
>> +
> 
> Might be prudent to not make them as generic, especially `BLOCK_LAYER`
> might easily clash without namespace. `SUBMITTED_BY_...`?

Hi Benjamin,

I will insert the prefix SUBMITTED_BY_.

> 
>>   struct scsi_cmnd {
>>   	struct scsi_request req;
>>   	struct scsi_device *device;
>> @@ -90,6 +96,7 @@ struct scsi_cmnd {
>>   	unsigned char prot_op;
>>   	unsigned char prot_type;
>>   	unsigned char prot_flags;
>> +	enum scsi_cmnd_submitter submitter;
> 
> Do you think it'd make much of a difference, if you initialized this in
> scsi_init_command(), or somewhere around there, explicitly to
> `BLOCK_LAYER`? Makes it easier to maintain, and to not forget, that it
> needs to be done, if the memset() to 0 ever changes... after the
> memset() the memory should be hot.
> 
> I just had to search a bit where this gets set to 0, as I didn't
> remember exactly where it was.

Performance-wise this probably won't make much difference. I'd like to add
that assignment in scsi_queue_rq() where the "cmd->scsi_done = scsi_done"
code was. That should be the approach that has the lowest probability of
introducing functional changes in the SCSI core.

Thanks,

Bart.