On 2020-06-25 07:01, Hannes Reinecke wrote:
> +/**
> + * scsi_get_internal_cmd - allocate an intenral SCSI command
^^^^^^^^
internal?
> + * @sdev: SCSI device from which to allocate the command
> + * @data_direction: Data direction for the allocated command
> + * @op_flags: request allocation flags
> + *
> + * Allocates a SCSI command for internal LLDD use.
> + */
> +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?
> +{
> + struct request *rq;
> + struct scsi_cmnd *scmd;
> + blk_mq_req_flags_t flags = 0;
> + unsigned int op = REQ_INTERNAL | op_flags;
> +
> + op |= (data_direction == DMA_TO_DEVICE) ?
> + REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN;
> + rq = blk_mq_alloc_request(sdev->request_queue, op, flags);
> + if (IS_ERR(rq))
> + return NULL;
> + scmd = blk_mq_rq_to_pdu(rq);
> + scmd->request = rq;
> + scmd->device = sdev;
> + return scmd;
> +}
> +EXPORT_SYMBOL_GPL(scsi_get_internal_cmd);
Since the 'flags' variable always has the value 0, can it be left out?
> +/**
> + * 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?
Thanks,
Bart.
