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

Christoph Hellwig
2021-05-04T09:53:50Z

On Mon, May 03, 2021 at 05:03:18PM +0200, Hannes Reinecke wrote:
> +struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
> + unsigned int op, blk_mq_req_flags_t flags)

Weird indentation - prototype continuations either use two tabs or
are aligned after the opening brace (I generally prefer the former).

> +{
> + struct request *rq;
> + struct scsi_cmnd *scmd;
> +
> + WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
> + ((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));

Woudn't a simple bool write command make more sense than passing the
actual op here?

> + 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;

Maybe a comment that explains what part of the scmd are initialized
and which not would be useful.