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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/28/20 5:48 AM, Bart Van Assche wrote:
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?

Okay for data direction, but converting op_flags into __bitwise (or even a new type) should be relegated to a different patchset.

+{
+	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?

Yep, true.

+/**
+ * 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.

Cheers,

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



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux