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

Hannes Reinecke
2021-05-04T12:55:01Z

On 5/4/21 11:53 AM, Christoph Hellwig wrote:
> 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).
>
Yeah, I'm never sure how one should be indenting the second line for a
function declaration. But I'll fix it.

>> +{
>> + 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?
>
Yep, can do.

>> + 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.
>
Okay.

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer