On 11/10/22 12:40 PM, Mike Christie wrote:
> On 11/10/22 5:15 AM, John Garry wrote:
>> On 04/11/2022 23:18, Mike Christie wrote:
>>> - req_flags_t rq_flags, int *resid)
>>> +int __scsi_exec_req(const struct scsi_exec_args *args)
>>
>> nit: I would expect a req to be passed based on the name, like blk_execute_rq()
>>
>
> We have scsi_exeucute_req now which works like scsi_exec_req. I carried it over
> because it seemed nice that it reflects we are executing a request vs something
> like a TMF. I don't care either way if people have a preference.
>
>
>>> {
>>> struct request *req;
>>> struct scsi_cmnd *scmd;
>>> int ret;
>>> - req = scsi_alloc_request(sdev->request_queue,
>>> - data_direction == DMA_TO_DEVICE ?
>>> - REQ_OP_DRV_OUT : REQ_OP_DRV_IN,
>>> - rq_flags & RQF_PM ? BLK_MQ_REQ_PM : 0);
>>> + req = scsi_alloc_request(args->sdev->request_queue,
>>> + args->data_dir == DMA_TO_DEVICE ?
>>> + REQ_OP_DRV_OUT : REQ_OP_DRV_IN,
>>
>> Did you ever consider just putting the scsi_alloc_request() opf arg in struct scsi_exec_args (rather than data_dir), i.e. have the caller evaluate? We already do it in other callers to scsi_alloc_request().
>
> I did, but this part of the patches just convert us to the args struct
> use. I tried to not change the types to keep the patchset down.
>
> I'll change the types in another patchset,
Oh wait, I could probably handle your comments in this set if we
wanted to.
scsi_execute could already take the block layer flags, so we already
are doing that type of thing so when I was thinking the scsi_execute
interface should be less block layer'y I was wrong. So, I could convert
the users to use the REQ_OP_DRV instead of DMA values when I do the args
conversion since we normally do just pass it in the directly (vs libata
where we do some extra work).
One thing that is weird to me is that scsi_execute_req is the more scsi'ish
interface. I would have thought since it took the req flag since it has the
req naming in it.
I don't care either way.
