Re: [PATCH 02/31] scsi: add scsi_{get,put}_internal_cmd() helper

[John Garry <john.garry@huawei.com>]

On 22/02/2021 13:23, Hannes Reinecke wrote:
> Add helper functions to allow LLDDs to allocate and free
> internal commands.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>

same comments as v6 apply :)

As does RB tag

> ---
>   drivers/scsi/scsi_lib.c    | 45 ++++++++++++++++++++++++++++++++++++++
>   include/scsi/scsi_device.h |  4 ++++
>   2 files changed, 49 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index d0ae586565f8..5cb464972682 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1934,6 +1934,51 @@ void scsi_mq_destroy_tags(struct Scsi_Host *shost)
>   	blk_mq_free_tag_set(&shost->tag_set);
>   }
>   
> +/**
> + * scsi_get_internal_cmd - allocate an internal SCSI command
> + * @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,
> +	enum dma_data_direction data_direction, int 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);
> +
> +/**
> + * scsi_put_internal_cmd - free an internal SCSI command
> + * @scmd: SCSI command to be freed
> + *
> + * Check if @scmd is an internal command, and call
> + * blk_mq_free_request() if true.
> + */
> +void scsi_put_internal_cmd(struct scsi_cmnd *scmd)
> +{
> +	struct request *rq = blk_mq_rq_from_pdu(scmd);
> +
> +	if (WARN_ON(!blk_rq_is_internal(rq)))
> +		return;

Still not sure if we need this.... best cc jens on series in future for 
view on complete changes