Re: [PATCH v2 01/15] scsi: Add struct for args to execution functions

[Mike Christie <michael.christie@oracle.com>]

On 12/12/22 1:45 PM, Bart Van Assche wrote:
> On 12/8/22 22:13, Mike Christie wrote:
>> This begins to move the SCSI execution functions to use a struct for
>> passing in optional args. This patch adds the new struct, temporarily
>> converts scsi_execute and scsi_execute_req and add two helpers:
>> 1. scsi_execute_args which takes the scsi_exec_args struct.
>> 2. scsi_execute_cmd does not support the scsi_exec_args struct.
>                     ^^^
>                     which?
> 
>> @@ -232,8 +222,7 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
>>       memcpy(scmd->cmnd, cmd, scmd->cmd_len);
>>       scmd->allowed = retries;
>>       req->timeout = timeout;
>> -    req->cmd_flags |= flags;
>> -    req->rq_flags |= rq_flags | RQF_QUIET;
>> +    req->rq_flags |= RQF_QUIET;
> 
> My understanding is that neither scsi_alloc_request() nor any of the
> functions it calls copies its 'flags' argument into req->rq_flags. I
> think this is a behavior change that has not been described in the
> patch description. I'm not saying that this change is wrong but that
> careful review is required if this change is retained.


It's not directly copied but we only used the one flag RQF_PM.

The new code has us pass in the BLK_MQ flag which is used by
blk_mq_alloc_request. Those flags we pass blk_mq_alloc_request
eventually get set to blk_mq_alloc_data->flags so when blk_mq_rq_ctx_init
is called it checks for its BLK_MQ flags and does BLK_MQ_REQ_PM:


        if (data->flags & BLK_MQ_REQ_PM)
                data->rq_flags |= RQF_PM;
...
        rq->rq_flags = data->rq_flags;


Note that if you are scanning the code and see the scsi_dh module's
req_flags, the variable name was misleading as they were really the
blk_opf_t flags.