diff mbox series

[v2] iser: explicitly set shost max_segment_size if non virtual boundary devices

Message ID 20190607012914.2328-1-sagi@grimberg.me (mailing list archive)
State Superseded
Headers show
Series [v2] iser: explicitly set shost max_segment_size if non virtual boundary devices | expand

Commit Message

Sagi Grimberg June 7, 2019, 1:29 a.m. UTC
if the rdma device supports sg gaps, we don't need to set a virtual
boundary but we then need to explicitly set the max_segment_size, otherwise
scsi takes BLK_MAX_SEGMENT_SIZE and sets it using dma_set_max_seg_size()
and this affects all the rdma device consumers.

Fix it by setting shost max_segment_size according to the device
capability if SG_GAPS are not supported.

Reported-by: Jason Gunthorpe <jgg@ziepe.ca>
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
Changes from v1:
- set max_segment_size only for non virtual boundary devices

 drivers/infiniband/ulp/iser/iscsi_iser.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Bart Van Assche June 7, 2019, 2 p.m. UTC | #1
On 6/6/19 6:29 PM, Sagi Grimberg wrote:
> if the rdma device supports sg gaps, we don't need to set a virtual
> boundary but we then need to explicitly set the max_segment_size, otherwise
> scsi takes BLK_MAX_SEGMENT_SIZE and sets it using dma_set_max_seg_size()
> and this affects all the rdma device consumers.
> 
> Fix it by setting shost max_segment_size according to the device
> capability if SG_GAPS are not supported.
> 
> Reported-by: Jason Gunthorpe <jgg@ziepe.ca>
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
> ---
> Changes from v1:
> - set max_segment_size only for non virtual boundary devices
> 
>   drivers/infiniband/ulp/iser/iscsi_iser.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
> index 841b66397a57..a3a4b956bbb9 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> @@ -653,7 +653,9 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
>   						   SHOST_DIX_GUARD_CRC);
>   		}
>   
> -		if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
> +		if (ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG)
> +			shost->max_segment_size = ib_dma_max_seg_size(ib_dev);
> +		else
>   			shost->virt_boundary_mask = ~MASK_4K;
>   
>   		if (iscsi_host_add(shost, ib_dev->dev.parent)) {

This is incomprehensible without a big comment that explains why 
max_segment_size is only set if the IB_DEVICE_SG_GAPS_REG feature is 
available.

Bart.
Jason Gunthorpe Oct. 29, 2019, 7:20 p.m. UTC | #2
On Thu, Jun 06, 2019 at 06:29:14PM -0700, Sagi Grimberg wrote:
> if the rdma device supports sg gaps, we don't need to set a virtual
> boundary but we then need to explicitly set the max_segment_size, otherwise
> scsi takes BLK_MAX_SEGMENT_SIZE and sets it using dma_set_max_seg_size()
> and this affects all the rdma device consumers.
> 
> Fix it by setting shost max_segment_size according to the device
> capability if SG_GAPS are not supported.
> 
> Reported-by: Jason Gunthorpe <jgg@ziepe.ca>
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
> ---
> Changes from v1:
> - set max_segment_size only for non virtual boundary devices
> 
>  drivers/infiniband/ulp/iser/iscsi_iser.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Sagi, are you respinning this or ??

https://patchwork.kernel.org/patch/10980657/

Jason
Sagi Grimberg Oct. 29, 2019, 7:25 p.m. UTC | #3
>> if the rdma device supports sg gaps, we don't need to set a virtual
>> boundary but we then need to explicitly set the max_segment_size, otherwise
>> scsi takes BLK_MAX_SEGMENT_SIZE and sets it using dma_set_max_seg_size()
>> and this affects all the rdma device consumers.
>>
>> Fix it by setting shost max_segment_size according to the device
>> capability if SG_GAPS are not supported.
>>
>> Reported-by: Jason Gunthorpe <jgg@ziepe.ca>
>> Suggested-by: Christoph Hellwig <hch@lst.de>
>> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
>> ---
>> Changes from v1:
>> - set max_segment_size only for non virtual boundary devices
>>
>>   drivers/infiniband/ulp/iser/iscsi_iser.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Sagi, are you respinning this or ??

I can take the change-log message and paste it in the code,
but is that something we want? Usually people look in the
change-log history...
Sagi Grimberg Oct. 29, 2019, 7:26 p.m. UTC | #4
>>> if the rdma device supports sg gaps, we don't need to set a virtual
>>> boundary but we then need to explicitly set the max_segment_size, 
>>> otherwise
>>> scsi takes BLK_MAX_SEGMENT_SIZE and sets it using dma_set_max_seg_size()
>>> and this affects all the rdma device consumers.
>>>
>>> Fix it by setting shost max_segment_size according to the device
>>> capability if SG_GAPS are not supported.
>>>
>>> Reported-by: Jason Gunthorpe <jgg@ziepe.ca>
>>> Suggested-by: Christoph Hellwig <hch@lst.de>
>>> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
>>> ---
>>> Changes from v1:
>>> - set max_segment_size only for non virtual boundary devices
>>>
>>>   drivers/infiniband/ulp/iser/iscsi_iser.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> Sagi, are you respinning this or ??
> 
> I can take the change-log message and paste it in the code,
> but is that something we want? Usually people look in the
> change-log history...

I actually thought it was already in...
diff mbox series

Patch

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 841b66397a57..a3a4b956bbb9 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -653,7 +653,9 @@  iscsi_iser_session_create(struct iscsi_endpoint *ep,
 						   SHOST_DIX_GUARD_CRC);
 		}
 
-		if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
+		if (ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG)
+			shost->max_segment_size = ib_dma_max_seg_size(ib_dev);
+		else
 			shost->virt_boundary_mask = ~MASK_4K;
 
 		if (iscsi_host_add(shost, ib_dev->dev.parent)) {