On 2020-06-28 02:02, Hannes Reinecke wrote:
> On 6/28/20 5:48 AM, Bart Van Assche wrote:
>> On 2020-06-25 07:01, Hannes Reinecke wrote:
>>> +struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
>>> + int data_direction, int op_flags)
>>
>> How about using enum dma_data_direction for data_direction and unsigned
>> int, or even better, a new __bitwise type for op_flags?
>>
> Okay for data direction, but converting op_flags into __bitwise (or even
> a new type) should be relegated to a different patchset.
OK.
>>> +/**
>>> + * scsi_put_internal_cmd - free an internal SCSI command
>>> + * @scmd: SCSI command to be freed
>>> + */
>>> +void scsi_put_internal_cmd(struct scsi_cmnd *scmd)
>>> +{
>>> + struct request *rq = blk_mq_rq_from_pdu(scmd);
>>> +
>>> + if (blk_rq_is_internal(rq))
>>> + blk_mq_free_request(rq);
>>> +}
>>> +EXPORT_SYMBOL_GPL(scsi_put_internal_cmd);
>>
>> How about triggering a warning for the !blk_rq_is_internal(rq) case
>> instead of silently ignoring regular SCSI commands?
>>
> That's by design.
> Some drivers have a common routine for freeing up commands, so it'd be
> quite tricky to separate these two cases out at the driver level.
> So it's far easier to call the common routine for all commands, and
> let this function do the right thing for all commands.
That sounds fair to me, but is an example available in this patch series
of a call to scsi_put_internal_cmd() from such a common routine? It
seems to me that all calls to scsi_put_internal_cmd() introduced in this
patch series happen from code paths that handle internal commands only?
Thanks,
Bart.
