Re: [PATCH V6 1/9] RDMA/iser: Limit sg tablesize and max_sectors to device fastreg max depth

2015-07-26 Thread Sagi Grimberg

On 7/24/2015 10:14 PM, Jason Gunthorpe wrote:

On Fri, Jul 24, 2015 at 01:40:17PM -0500, Steve Wise wrote:

Huh. How does this relate to the max_page_list_len argument:

  struct ib_mr *ib_alloc_fast_reg_mr(struct ib_pd *pd, int max_page_list_len)

Shouldn't max_fast_reg_page_list_len be checked during the above?

Ie does this still make sense:

drivers/infiniband/ulp/iser/iser_verbs.c:   desc-data_mr = 
ib_alloc_fast_reg_mr(pd, ISCSI_ISER_SG_TABLESIZE + 1);

?

The only ULP that checks this is SRP, so basically, all our ULPs are
probably quietly broken? cxgb3 has a limit of 10 (!?!?!!)



Yea seems like some drivers need to enforce this in ib_alloc_fast_reg_mr() as 
well as ib_alloc_fast_reg_page_list(), and ULPs need
to not exceed the device max.


Great, Sagi, can you incorporate that in your series so that
ib_alloc_mr's max_entires is checked against
max_fast_reg_page_list_len and EINVAL's if it is too great?


Yes. I'll take care of that.
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V6 1/9] RDMA/iser: Limit sg tablesize and max_sectors to device fastreg max depth

2015-07-24 Thread Jason Gunthorpe
On Fri, Jul 24, 2015 at 01:40:17PM -0500, Steve Wise wrote:
  Huh. How does this relate to the max_page_list_len argument:
  
   struct ib_mr *ib_alloc_fast_reg_mr(struct ib_pd *pd, int max_page_list_len)
  
  Shouldn't max_fast_reg_page_list_len be checked during the above?
  
  Ie does this still make sense:
  
  drivers/infiniband/ulp/iser/iser_verbs.c:   desc-data_mr = 
  ib_alloc_fast_reg_mr(pd, ISCSI_ISER_SG_TABLESIZE + 1);
  
  ?
  
  The only ULP that checks this is SRP, so basically, all our ULPs are
  probably quietly broken? cxgb3 has a limit of 10 (!?!?!!)
  
 
 Yea seems like some drivers need to enforce this in ib_alloc_fast_reg_mr() as 
 well as ib_alloc_fast_reg_page_list(), and ULPs need
 to not exceed the device max.

Great, Sagi, can you incorporate that in your series so that
ib_alloc_mr's max_entires is checked against
max_fast_reg_page_list_len and EINVAL's if it is too great?

Also, after your series can we drop ib_alloc_fast_reg_page_list, and
then also the associated WR stuff?

Thanks,
Jason
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH V6 1/9] RDMA/iser: Limit sg tablesize and max_sectors to device fastreg max depth

2015-07-24 Thread Steve Wise


 -Original Message-
 From: Jason Gunthorpe [mailto:jguntho...@obsidianresearch.com]
 Sent: Friday, July 24, 2015 11:41 AM
 To: Steve Wise
 Cc: dledf...@redhat.com; infinip...@intel.com; sa...@mellanox.com; 
 ogerl...@mellanox.com; r...@mellanox.com; linux-
 r...@vger.kernel.org; e...@mellanox.com; target-de...@vger.kernel.org; 
 linux-...@vger.kernel.org; bfie...@fieldses.org
 Subject: Re: [PATCH V6 1/9] RDMA/iser: Limit sg tablesize and max_sectors to 
 device fastreg max depth
 
 On Fri, Jul 24, 2015 at 11:18:21AM -0500, Steve Wise wrote:
  Currently the sg tablesize, which dictates fast register page list
  depth to use, does not take into account the limits of the rdma device.
  So adjust it once we discover the device fastreg max depth limit.  Also
  adjust the max_sectors based on the resulting sg tablesize.
 
 Huh. How does this relate to the max_page_list_len argument:
 
  struct ib_mr *ib_alloc_fast_reg_mr(struct ib_pd *pd, int max_page_list_len)
 
 Shouldn't max_fast_reg_page_list_len be checked during the above?
 
 Ie does this still make sense:
 
 drivers/infiniband/ulp/iser/iser_verbs.c:   desc-data_mr = 
 ib_alloc_fast_reg_mr(pd, ISCSI_ISER_SG_TABLESIZE + 1);
 
 ?
 
 The only ULP that checks this is SRP, so basically, all our ULPs are
 probably quietly broken? cxgb3 has a limit of 10 (!?!?!!)
 

Yea seems like some drivers need to enforce this in ib_alloc_fast_reg_mr() as 
well as ib_alloc_fast_reg_page_list(), and ULPs need
to not exceed the device max.

I will fix iser to limit the mr and page_list allocation based on the device 
max.

Steve.

--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V6 1/9] RDMA/iser: Limit sg tablesize and max_sectors to device fastreg max depth

2015-07-24 Thread Jason Gunthorpe
On Fri, Jul 24, 2015 at 11:18:21AM -0500, Steve Wise wrote:
 Currently the sg tablesize, which dictates fast register page list
 depth to use, does not take into account the limits of the rdma device.
 So adjust it once we discover the device fastreg max depth limit.  Also
 adjust the max_sectors based on the resulting sg tablesize.

Huh. How does this relate to the max_page_list_len argument:

 struct ib_mr *ib_alloc_fast_reg_mr(struct ib_pd *pd, int max_page_list_len)

Shouldn't max_fast_reg_page_list_len be checked during the above?

Ie does this still make sense:

drivers/infiniband/ulp/iser/iser_verbs.c:   desc-data_mr = 
ib_alloc_fast_reg_mr(pd, ISCSI_ISER_SG_TABLESIZE + 1);

?

The only ULP that checks this is SRP, so basically, all our ULPs are
probably quietly broken? cxgb3 has a limit of 10 (!?!?!!)

Jason
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html