Skip to contentSkip to conversation

sc_data_direction and bidi commands

DISCUSSION Bart Van Assche 2 participants 5 messages Jan 21, 2015 – Jan 22, 2015 SCSI

lore
Share & tools
lore
#0 lore raw

Hello,

Although several SCSI LLDs compare sc_data_direction with the value DMA_BIDIRECTIONAL apparently scsi_setup_cmnd() never sets sc_data_direction to the value DMA_BIDIRECTIONAL. Is this behavior on purpose ? I discovered this while debugging bidi support in a SCSI LLD.

Thanks,

Bart.

#1 lore raw
Quoted 7 lines · Bart Van Assche

Hello,

Although several SCSI LLDs compare sc_data_direction with the value DMA_BIDIRECTIONAL apparently scsi_setup_cmnd() never sets sc_data_direction to the value DMA_BIDIRECTIONAL. Is this behavior on purpose ?

Yes. Bi-directional CDBs are signaled using scsi_bidi_cmnd(), and IIRC ->sc_data_direction = DMA_TO_DEVICE is set for those cases.

There is an example in drivers/target/loopback/tcm_loop.c, btw.

--nab

#2 lore raw
Quoted 8 lines · Nicholas A. Bellinger
Quoted 4 lines · Bart Van Assche

Although several SCSI LLDs compare sc_data_direction with the value DMA_BIDIRECTIONAL apparently scsi_setup_cmnd() never sets sc_data_direction to the value DMA_BIDIRECTIONAL. Is this behavior on purpose ?

Yes. Bi-directional CDBs are signaled using scsi_bidi_cmnd(), and IIRC ->sc_data_direction = DMA_TO_DEVICE is set for those cases.

But why is sc_data_direction set to DMA_BIDIRECTIONAL in scsi_ioctl_reset() in drivers/scsi/scsi_error.c ? I think scsi_bidi_cmnd() will return false for the SCSI command submitted by scsi_ioctl_reset() since in this case scmd->request->next_rq == NULL.

Bart.

#3 lore raw
Quoted 7 lines · Bart Van Assche
Quoted 6 lines · Nicholas A. Bellinger
Quoted 4 lines · Bart Van Assche

Although several SCSI LLDs compare sc_data_direction with the value DMA_BIDIRECTIONAL apparently scsi_setup_cmnd() never sets sc_data_direction to the value DMA_BIDIRECTIONAL. Is this behavior on purpose ?

Quoted 8 lines
Quoted 2 lines

Yes. Bi-directional CDBs are signaled using scsi_bidi_cmnd(), and IIRC ->sc_data_direction = DMA_TO_DEVICE is set for those cases.

But why is sc_data_direction set to DMA_BIDIRECTIONAL in scsi_ioctl_reset() in drivers/scsi/scsi_error.c ? I think scsi_bidi_cmnd() will return false for the SCSI command submitted by scsi_ioctl_reset() since in this case scmd->request->next_rq == NULL.

Well, according to DMA-API.txt it's classified as:

DMA_BIDIRECTIONAL direction isn't known

and is unrelated to any actual SCSI bidi commands containing both WRITE and READ payload buffers.

scsi_ioctl_reset() is only generating TMFs btw, and can't inject any SCSI bidi commands.

--nab

#4 lore raw
Quoted 6 lines · Nicholas A. Bellinger

Well, according to DMA-API.txt it's classified as:

DMA_BIDIRECTIONAL direction isn't known

and is unrelated to any actual SCSI bidi commands containing both WRITE and READ payload buffers.

I think I figured out why several SCSI LLDs are comparing sc_data_direction with DMA_BIDIRECTIONAL. In code that has been removed eight years ago I found the following construct (see also
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit?id=21b2f0c803adaf00fce1b606c50b49ae8b106773):

               if (inlen == 0) {
                       data_direction = DMA_FROM_DEVICE;
               } else if (outlen == 0 ) {
                       data_direction = DMA_TO_DEVICE;
               } else {
                       /*
                        * Can this ever happen?
                        */
                       data_direction = DMA_BIDIRECTIONAL;
               }

Here is some more removed code from the same timeframe
(https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit?id=beb40487508290f5d6565598c60a3f44261beef2):

struct scsi_request *scsi_allocate_request(struct scsi_device *sdev,
gfp_t gfp_mask)

{
       const int offset = ALIGN(sizeof(struct scsi_request), 4);
       const int size = offset + sizeof(struct request);
       struct scsi_request *sreq;

       sreq = kzalloc(size, gfp_mask);
       if (likely(sreq != NULL)) {
               sreq->sr_request = (struct request *)(((char *)sreq)
                                  + offset);
               sreq->sr_device = sdev;
               sreq->sr_host = sdev->host;
               sreq->sr_magic = SCSI_REQ_MAGIC;
               sreq->sr_data_direction = DMA_BIDIRECTIONAL;
       }

       return sreq;
}

Bart.