sc_data_direction and bidi commands

sc_data_direction and bidi commands

[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 ? I discovered this while debugging bidi support in a SCSI LLD.

Thanks,

Bart.

Re: sc_data_direction and bidi commands

[Nicholas A. Bellinger]

On Wed, 2015-01-21 at 17:18 +0100, Bart Van Assche wrote:
> 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

Re: sc_data_direction and bidi commands

[Bart Van Assche]

On 01/22/15 11:02, Nicholas A. Bellinger wrote:
> On Wed, 2015-01-21 at 17:18 +0100, Bart Van Assche wrote:
>> 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.

Re: sc_data_direction and bidi commands

[Nicholas A. Bellinger]

On Thu, 2015-01-22 at 11:14 +0100, Bart Van Assche wrote:
> On 01/22/15 11:02, Nicholas A. Bellinger wrote:
> > On Wed, 2015-01-21 at 17:18 +0100, Bart Van Assche wrote:
> >> 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.
> 

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

Re: sc_data_direction and bidi commands

[Bart Van Assche]

On 01/22/15 11:39, Nicholas A. Bellinger wrote:
> 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.