Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-07-27 Thread Boaz Harrosh
On 06/25/2014 04:17 AM, Vladislav Bolkhovitin wrote: Martin K. Petersen, on 06/23/2014 06:58 PM wrote: Mike == Mike Christie micha...@cs.wisc.edu writes: + unsigned int xfer_len = blk_rq_bytes(scmd-request); Mike Can you do bidi and dif/dix? Nope. Correction: at the moment. There is a

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-07-27 Thread Boaz Harrosh
On 06/25/2014 01:32 PM, Sagi Grimberg wrote: On 6/25/2014 11:48 AM, Sagi Grimberg wrote: SNIP I made the patch below which should fix both bidi support in iscsi and also WRITE_SAME (and similar commands) support. I'm a bit confused, for all commands (bidi or not) the iscsi header

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-07-27 Thread Christoph Hellwig
On Sun, Jul 27, 2014 at 12:11:11PM +0300, Boaz Harrosh wrote: If you have a tree that you want me to test I will be glad too. From this thread I'm confused as to what patches you want me to test? please point me to a tree you need testing. You can bug me any time, any tree. I will be happy to

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-07-13 Thread Christoph Hellwig
Sagi, can you send an update for the core-for-3.17 tree to pass a dma_direction or the scsi data buffer to scsi_transfer_length so we can make safe for bidi usage in the future? -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-07-13 Thread Martin K. Petersen
Christoph == Christoph Hellwig h...@infradead.org writes: Christoph Sagi, can you send an update for the core-for-3.17 tree to Christoph pass a dma_direction or the scsi data buffer to Christoph scsi_transfer_length so we can make safe for bidi usage in Christoph the future? I have cleaned this

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-26 Thread Bart Van Assche
On 06/11/14 11:09, Sagi Grimberg wrote: + return xfer_len + (xfer_len ilog2(sector_size)) * 8; Sorry that I just noticed this now, but why is a shift-right and ilog2() used in the above expression instead of just dividing the transfer length by the sector size ? Thanks, Bart. -- To

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-26 Thread James Bottomley
On Thu, 2014-06-26 at 16:53 +0200, Bart Van Assche wrote: On 06/11/14 11:09, Sagi Grimberg wrote: + return xfer_len + (xfer_len ilog2(sector_size)) * 8; Sorry that I just noticed this now, but why is a shift-right and ilog2() used in the above expression instead of just dividing the

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-26 Thread Atchley, Scott
On Jun 26, 2014, at 10:55 AM, James Bottomley james.bottom...@hansenpartnership.com wrote: On Thu, 2014-06-26 at 16:53 +0200, Bart Van Assche wrote: On 06/11/14 11:09, Sagi Grimberg wrote: + return xfer_len + (xfer_len ilog2(sector_size)) * 8; Sorry that I just noticed this now, but why

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-26 Thread James Bottomley
On June 26, 2014 11:41:48 AM EDT, Atchley, Scott atchle...@ornl.gov wrote: On Jun 26, 2014, at 10:55 AM, James Bottomley james.bottom...@hansenpartnership.com wrote: On Thu, 2014-06-26 at 16:53 +0200, Bart Van Assche wrote: On 06/11/14 11:09, Sagi Grimberg wrote: + return xfer_len +

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-26 Thread Atchley, Scott
On Jun 26, 2014, at 12:38 PM, James Bottomley james.bottom...@hansenpartnership.com wrote: On June 26, 2014 11:41:48 AM EDT, Atchley, Scott atchle...@ornl.gov wrote: On Jun 26, 2014, at 10:55 AM, James Bottomley james.bottom...@hansenpartnership.com wrote: On Thu, 2014-06-26 at 16:53

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-25 Thread Sagi Grimberg
On 6/25/2014 6:32 AM, Mike Christie wrote: On 06/24/2014 12:08 PM, Mike Christie wrote: On 06/24/2014 12:00 PM, Mike Christie wrote: On 06/24/2014 11:30 AM, Christoph Hellwig wrote: On Tue, Jun 24, 2014 at 07:27:46PM +0300, Sagi Grimberg wrote: This condition only matters in the bidi case,

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-25 Thread Christoph Hellwig
Mike, I'd prefer a fix on top of the core-for-3.16 branch in my scsi-queue tree, which already has the fix from Martin. I also really don't like these three confusing helpers: +static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd) +{ + return

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-25 Thread Christoph Hellwig
Sagi's patch was not correct because scsi_in was hardcoded to the transfer len when bidi was used. Right, should have condition that in the direction. something like: transfer_length = sc-sc_data_direction == DMA_TO_DEVICE ? scsi_out(sc)-length : scsi_in(sc)-length; would probably hit

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-25 Thread Sagi Grimberg
On 6/25/2014 11:48 AM, Sagi Grimberg wrote: SNIP I made the patch below which should fix both bidi support in iscsi and also WRITE_SAME (and similar commands) support. I'm a bit confused, for all commands (bidi or not) the iscsi header data_length should describe the scsi_data_buffer

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-25 Thread Martin K. Petersen
Christoph == Christoph Hellwig h...@infradead.org writes: Christoph So here we use blk_rq_bytes still, which is incorrect for Christoph WRITE SAME. Yeah, scsi_transfer_length() needs to go away completely if we go with the in and out variants. Christoph I think the easiest fix is to just pass

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-25 Thread Christoph Hellwig
On Wed, Jun 25, 2014 at 01:32:39PM +0300, Sagi Grimberg wrote: So I tested a bidirectional command using: $ sg_raw --infile=/root/filex --send=1024 --request=1024 --outfile=/root/filex /dev/bsg/7:0:0:0 53 00 00 00 00 00 00 00 02 00 And I see: kernel: session1: iscsi_prep_scsi_cmd_pdu iscsi

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-25 Thread Michael Christie
On Jun 25, 2014, at 6:35 AM, Christoph Hellwig h...@infradead.org wrote: On Wed, Jun 25, 2014 at 01:32:39PM +0300, Sagi Grimberg wrote: So I tested a bidirectional command using: $ sg_raw --infile=/root/filex --send=1024 --request=1024 --outfile=/root/filex /dev/bsg/7:0:0:0 53 00 00 00 00 00

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Mike Christie
On 06/11/2014 04:09 AM, Sagi Grimberg wrote: In case protection information exists on the wire scsi transports should include it in the transfer byte count (even if protection information does not exist in the host memory space). This helper will compute the total transfer length from the

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Martin K. Petersen
Mike == Mike Christie micha...@cs.wisc.edu writes: Mike The problem is WRITE_SAME requests are setup so that Mike req-__data_len is the value of the entire request when the setup Mike is completed but during the setup process it's value changes Oh, I see. So things break because iSCSI uses

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread sagi grimberg
On 6/24/2014 9:54 AM, Mike Christie wrote: On 06/11/2014 04:09 AM, Sagi Grimberg wrote: In case protection information exists on the wire scsi transports should include it in the transfer byte count (even if protection information does not exist in the host memory space). This helper will

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Sagi Grimberg
On 6/24/2014 3:53 PM, Martin K. Petersen wrote: Mike == Mike Christie micha...@cs.wisc.edu writes: Mike The problem is WRITE_SAME requests are setup so that Mike req-__data_len is the value of the entire request when the setup Mike is completed but during the setup process it's value changes

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Christoph Hellwig
On Tue, Jun 24, 2014 at 08:53:25AM -0400, Martin K. Petersen wrote: Oh, I see. So things break because iSCSI uses scsi_transfer_length() where the scatterlist length was used in the past. How about this? This fixes the regression for me. -- To unsubscribe from this list: send the line

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Christoph Hellwig
On Tue, Jun 24, 2014 at 04:08:25PM +0300, Sagi Grimberg wrote: Oh, I see. So things break because iSCSI uses scsi_transfer_length() where the scatterlist length was used in the past. Ohhh, didn't notice this one and sent out a duplicate... The patch looks good to me obviously. Can you

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread sagi grimberg
On 6/24/2014 3:53 PM, Martin K. Petersen wrote: SCSI: Use SCSI data buffer length to extract transfer size Commit 8846bab180fa introduced a helper that can be used to query the wire transfer size for a SCSI command taking protection information into account. However, some commands

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Michael Christie
On Jun 24, 2014, at 7:53 AM, Martin K. Petersen martin.peter...@oracle.com wrote: Mike == Mike Christie micha...@cs.wisc.edu writes: Mike The problem is WRITE_SAME requests are setup so that Mike req-__data_len is the value of the entire request when the setup Mike is completed but during

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Christoph Hellwig
On Tue, Jun 24, 2014 at 11:08:54AM -0500, Michael Christie wrote: static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd) { - unsigned int xfer_len = blk_rq_bytes(scmd-request); + unsigned int xfer_len = scsi_out(scmd)-length; unsigned int prot_op =

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Sagi Grimberg
On 6/24/2014 7:08 PM, Michael Christie wrote: On Jun 24, 2014, at 7:53 AM, Martin K. Petersen martin.peter...@oracle.com wrote: Mike == Mike Christie micha...@cs.wisc.edu writes: Mike The problem is WRITE_SAME requests are setup so that Mike req-__data_len is the value of the entire request

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Christoph Hellwig
On Tue, Jun 24, 2014 at 07:27:46PM +0300, Sagi Grimberg wrote: This condition only matters in the bidi case, which is not relevant for the PI case. I suggested to condition that in libiscsi (posted in the second thread, copy-paste below). Although I do agree that scsi_transfer_length() helper

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Martin K. Petersen
Mike == Michael Christie micha...@cs.wisc.edu writes: Mike Do we need to check for the data direction. Something like Mike if (scmd-sc_data_direction == DMA_TO_DEVICE) Mike xfer_len = scsi_out(scmnd)-length; Mike else Mike xfer_len = scsi_in(scmnd)-length; I guess that depends on the

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Mike Christie
On 06/24/2014 11:30 AM, Christoph Hellwig wrote: On Tue, Jun 24, 2014 at 07:27:46PM +0300, Sagi Grimberg wrote: This condition only matters in the bidi case, which is not relevant for the PI case. I suggested to condition that in libiscsi (posted in the second thread, copy-paste below).

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Martin K. Petersen
Mike == Mike Christie micha...@cs.wisc.edu writes: Mike It would be nice to just have one function to call and it just do Mike the right thing for the drivers. But what is the right thing when there are buffers for both directions? -- Martin K. Petersen Oracle Linux Engineering -- To

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Mike Christie
On 06/24/2014 11:31 AM, Martin K. Petersen wrote: Mike == Michael Christie micha...@cs.wisc.edu writes: Mike Do we need to check for the data direction. Something like Mike if (scmd-sc_data_direction == DMA_TO_DEVICE) Mike xfer_len = scsi_out(scmnd)-length; Mike else Mike

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-24 Thread Mike Christie
On 06/24/2014 12:00 PM, Mike Christie wrote: On 06/24/2014 11:30 AM, Christoph Hellwig wrote: On Tue, Jun 24, 2014 at 07:27:46PM +0300, Sagi Grimberg wrote: This condition only matters in the bidi case, which is not relevant for the PI case. I suggested to condition that in libiscsi (posted

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-23 Thread Mike Christie
On 06/11/2014 04:09 AM, Sagi Grimberg wrote: In case protection information exists on the wire scsi transports should include it in the transfer byte count (even if protection information does not exist in the host memory space). This helper will compute the total transfer length from the

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-23 Thread Martin K. Petersen
Mike == Mike Christie micha...@cs.wisc.edu writes: + unsigned int xfer_len = blk_rq_bytes(scmd-request); Mike Can you do bidi and dif/dix? Nope. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a

[PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-11 Thread Sagi Grimberg
In case protection information exists on the wire scsi transports should include it in the transfer byte count (even if protection information does not exist in the host memory space). This helper will compute the total transfer length from the scsi command data length and protection attributes.

Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

2014-06-11 Thread Martin K. Petersen
Sagi == Sagi Grimberg sa...@mellanox.com writes: Sagi In case protection information exists on the wire scsi transports Sagi should include it in the transfer byte count (even if protection Sagi information does not exist in the host memory space). This helper Sagi will compute the total