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.
