Linux-RDMA Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH rdma-next v2 0/3] Optimize SGL registration
@ 2019-10-07 13:59 Leon Romanovsky
  2019-10-07 13:59 ` [PATCH mlx5-next v2 1/3] net/mlx5: Expose optimal performance scatter entries capability Leon Romanovsky
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Leon Romanovsky @ 2019-10-07 13:59 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe, Christoph Hellwig
  Cc: Leon Romanovsky, RDMA mailing list, Or Gerlitz, Yamin Friedman,
	Saeed Mahameed, linux-netdev

From: Leon Romanovsky <leonro@mellanox.com>

Changelog
v1->v2: https://lore.kernel.org/linux-rdma/20191007115819.9211-1-leon@kernel.org
 * Used Christoph's comment
 * Change patch code flow to have one DMA_FROM_DEVICE check
v0->v1: https://lore.kernel.org/linux-rdma/20191006155955.31445-1-leon@kernel.org
 * Reorganized patches to have IB/core changes separated from mlx5
 * Moved SGL check before rdma_rw_force_mr
 * Added and rephrased original comment.

-----------------------------------------------------------------------------
Hi,

This series from Yamin implements long standing "TODO" existed in rw.c.

Thanks

Yamin Friedman (3):
  net/mlx5: Expose optimal performance scatter entries capability
  RDMA/rw: Support threshold for registration vs scattering to local
    pages
  RDMA/mlx5: Add capability for max sge to get optimized performance

 drivers/infiniband/core/rw.c      | 25 +++++++++++++++----------
 drivers/infiniband/hw/mlx5/main.c |  2 ++
 include/linux/mlx5/mlx5_ifc.h     |  2 +-
 include/rdma/ib_verbs.h           |  2 ++
 4 files changed, 20 insertions(+), 11 deletions(-)

--
2.20.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH mlx5-next v2 1/3] net/mlx5: Expose optimal performance scatter entries capability
  2019-10-07 13:59 [PATCH rdma-next v2 0/3] Optimize SGL registration Leon Romanovsky
@ 2019-10-07 13:59 ` Leon Romanovsky
  2019-10-07 15:02   ` Bart Van Assche
  2019-10-08  5:55   ` Leon Romanovsky
  2019-10-07 13:59 ` [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages Leon Romanovsky
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: Leon Romanovsky @ 2019-10-07 13:59 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe, Christoph Hellwig
  Cc: Leon Romanovsky, RDMA mailing list, Or Gerlitz, Yamin Friedman,
	Saeed Mahameed, linux-netdev

From: Yamin Friedman <yaminf@mellanox.com>

Expose maximum scatter entries per RDMA READ for optimal performance.

Signed-off-by: Yamin Friedman <yaminf@mellanox.com>
Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 include/linux/mlx5/mlx5_ifc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index 138c50d5a353..c0bfb1d90dd2 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -1153,7 +1153,7 @@ struct mlx5_ifc_cmd_hca_cap_bits {
 	u8         log_max_srq[0x5];
 	u8         reserved_at_b0[0x10];

-	u8         reserved_at_c0[0x8];
+	u8         max_sgl_for_optimized_performance[0x8];
 	u8         log_max_cq_sz[0x8];
 	u8         reserved_at_d0[0xb];
 	u8         log_max_cq[0x5];
--
2.20.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages
  2019-10-07 13:59 [PATCH rdma-next v2 0/3] Optimize SGL registration Leon Romanovsky
  2019-10-07 13:59 ` [PATCH mlx5-next v2 1/3] net/mlx5: Expose optimal performance scatter entries capability Leon Romanovsky
@ 2019-10-07 13:59 ` Leon Romanovsky
  2019-10-07 15:00   ` Christoph Hellwig
                     ` (2 more replies)
  2019-10-07 13:59 ` [PATCH rdma-next v2 3/3] RDMA/mlx5: Add capability for max sge to get optimized performance Leon Romanovsky
  2019-10-15  8:15 ` [PATCH rdma-next v2 0/3] Optimize SGL registration Leon Romanovsky
  3 siblings, 3 replies; 16+ messages in thread
From: Leon Romanovsky @ 2019-10-07 13:59 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe, Christoph Hellwig
  Cc: Leon Romanovsky, RDMA mailing list, Or Gerlitz, Yamin Friedman,
	Saeed Mahameed, linux-netdev

From: Yamin Friedman <yaminf@mellanox.com>

If there are more scatter entries than the recommended limit provided by
the ib device, UMR registration is used. This will provide optimal
performance when performing large RDMA READs over devices that advertise
the threshold capability.

With ConnectX-5 running NVMeoF RDMA with FIO single QP 128KB writes:
Without use of cap: 70Gb/sec
With use of cap: 84Gb/sec

Signed-off-by: Yamin Friedman <yaminf@mellanox.com>
Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/rw.c | 25 +++++++++++++++----------
 include/rdma/ib_verbs.h      |  2 ++
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c
index 5337393d4dfe..c27a543b58ef 100644
--- a/drivers/infiniband/core/rw.c
+++ b/drivers/infiniband/core/rw.c
@@ -20,14 +20,16 @@ module_param_named(force_mr, rdma_rw_force_mr, bool, 0);
 MODULE_PARM_DESC(force_mr, "Force usage of MRs for RDMA READ/WRITE operations");

 /*
- * Check if the device might use memory registration.  This is currently only
- * true for iWarp devices. In the future we can hopefully fine tune this based
- * on HCA driver input.
+ * Check if the device might use memory registration. This is currently
+ * true for iWarp devices and devices that have optimized SGL registration
+ * logic.
  */
 static inline bool rdma_rw_can_use_mr(struct ib_device *dev, u8 port_num)
 {
 	if (rdma_protocol_iwarp(dev, port_num))
 		return true;
+	if (dev->attrs.max_sgl_rd)
+		return true;
 	if (unlikely(rdma_rw_force_mr))
 		return true;
 	return false;
@@ -35,17 +37,20 @@ static inline bool rdma_rw_can_use_mr(struct ib_device *dev, u8 port_num)

 /*
  * Check if the device will use memory registration for this RW operation.
- * We currently always use memory registrations for iWarp RDMA READs, and
- * have a debug option to force usage of MRs.
- *
- * XXX: In the future we can hopefully fine tune this based on HCA driver
- * input.
+ * For RDMA READs we must use MRs on iWarp and can optionaly use them as an
+ * optimaztion otherwise.  Additionally we have a debug option to force usage
+ * of MRs to help testing this code path.
  */
+
 static inline bool rdma_rw_io_needs_mr(struct ib_device *dev, u8 port_num,
 		enum dma_data_direction dir, int dma_nents)
 {
-	if (rdma_protocol_iwarp(dev, port_num) && dir == DMA_FROM_DEVICE)
-		return true;
+	if (dir == DMA_FROM_DEVICE) {
+		if (rdma_protocol_iwarp(dev, port_num))
+			return true;
+		if (dev->attrs.max_sgl_rd && dma_nents > dev->attrs.max_sgl_rd)
+			return true;
+	}
 	if (unlikely(rdma_rw_force_mr))
 		return true;
 	return false;
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 4f671378dbfc..60fd98a9b7e8 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -445,6 +445,8 @@ struct ib_device_attr {
 	struct ib_tm_caps	tm_caps;
 	struct ib_cq_caps       cq_caps;
 	u64			max_dm_size;
+	/* Max entries for sgl for optimized performance per READ */
+	u32			max_sgl_rd;
 };

 enum ib_mtu {
--
2.20.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH rdma-next v2 3/3] RDMA/mlx5: Add capability for max sge to get optimized performance
  2019-10-07 13:59 [PATCH rdma-next v2 0/3] Optimize SGL registration Leon Romanovsky
  2019-10-07 13:59 ` [PATCH mlx5-next v2 1/3] net/mlx5: Expose optimal performance scatter entries capability Leon Romanovsky
  2019-10-07 13:59 ` [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages Leon Romanovsky
@ 2019-10-07 13:59 ` Leon Romanovsky
  2019-10-15  8:15 ` [PATCH rdma-next v2 0/3] Optimize SGL registration Leon Romanovsky
  3 siblings, 0 replies; 16+ messages in thread
From: Leon Romanovsky @ 2019-10-07 13:59 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe, Christoph Hellwig
  Cc: Leon Romanovsky, RDMA mailing list, Or Gerlitz, Yamin Friedman,
	Saeed Mahameed, linux-netdev

From: Yamin Friedman <yaminf@mellanox.com>

Allows the IB device to provide a value of maximum scatter gather entries
per RDMA READ.

In certain cases it may be preferable for a device to perform UMR memory
registration rather than have many scatter entries in a single RDMA READ.
This provides a significant performance increase in devices capable of
using different memory registration schemes based on the number of scatter
gather entries. This general capability allows each device vendor to fine
tune when it is better to use memory registration.

Signed-off-by: Yamin Friedman <yaminf@mellanox.com>
Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx5/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index fa23c8e7043b..39d54e285ae9 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -1012,6 +1012,8 @@ static int mlx5_ib_query_device(struct ib_device *ibdev,
 		1 << MLX5_CAP_GEN(mdev, log_max_klm_list_size);
 	props->max_pi_fast_reg_page_list_len =
 		props->max_fast_reg_page_list_len / 2;
+	props->max_sgl_rd =
+		MLX5_CAP_GEN(mdev, max_sgl_for_optimized_performance);
 	get_atomic_caps_qp(dev, props);
 	props->masked_atomic_cap   = IB_ATOMIC_NONE;
 	props->max_mcast_grp	   = 1 << MLX5_CAP_GEN(mdev, log_max_mcg);
--
2.20.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages
  2019-10-07 13:59 ` [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages Leon Romanovsky
@ 2019-10-07 15:00   ` Christoph Hellwig
  2019-10-07 15:57     ` Leon Romanovsky
  2019-10-07 15:07   ` Bart Van Assche
  2019-10-07 15:12   ` Bart Van Assche
  2 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2019-10-07 15:00 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Jason Gunthorpe, Christoph Hellwig,
	Leon Romanovsky, RDMA mailing list, Or Gerlitz, Yamin Friedman,
	Saeed Mahameed, linux-netdev

>   */
> +
>  static inline bool rdma_rw_io_needs_mr(struct ib_device *dev, u8 port_num,

Same like an empty line sneaked in here.  Except for that the whole
series looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH mlx5-next v2 1/3] net/mlx5: Expose optimal performance scatter entries capability
  2019-10-07 13:59 ` [PATCH mlx5-next v2 1/3] net/mlx5: Expose optimal performance scatter entries capability Leon Romanovsky
@ 2019-10-07 15:02   ` Bart Van Assche
  2019-10-07 15:54     ` Leon Romanovsky
  2019-10-08  5:55   ` Leon Romanovsky
  1 sibling, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2019-10-07 15:02 UTC (permalink / raw)
  To: Leon Romanovsky, Doug Ledford, Jason Gunthorpe, Christoph Hellwig
  Cc: Leon Romanovsky, RDMA mailing list, Or Gerlitz, Yamin Friedman,
	Saeed Mahameed, linux-netdev

On 10/7/19 6:59 AM, Leon Romanovsky wrote:
> -	u8         reserved_at_c0[0x8];
> +	u8         max_sgl_for_optimized_performance[0x8];

Should the name of this member variable perhaps be changed into 
"max_sgl_for_optimal_performance"?

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages
  2019-10-07 13:59 ` [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages Leon Romanovsky
  2019-10-07 15:00   ` Christoph Hellwig
@ 2019-10-07 15:07   ` Bart Van Assche
  2019-10-07 16:03     ` Leon Romanovsky
  2019-10-07 15:12   ` Bart Van Assche
  2 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2019-10-07 15:07 UTC (permalink / raw)
  To: Leon Romanovsky, Doug Ledford, Jason Gunthorpe, Christoph Hellwig
  Cc: Leon Romanovsky, RDMA mailing list, Or Gerlitz, Yamin Friedman,
	Saeed Mahameed, linux-netdev

On 10/7/19 6:59 AM, Leon Romanovsky wrote:
>   /*
> - * Check if the device might use memory registration.  This is currently only
> - * true for iWarp devices. In the future we can hopefully fine tune this based
> - * on HCA driver input.
> + * Check if the device might use memory registration. This is currently
> + * true for iWarp devices and devices that have optimized SGL registration
> + * logic.
>    */

The following sentence in the above comment looks confusing to me: 
"Check if the device might use memory registration." That sentence 
suggests that the HCA decides whether or not to use memory registration. 
Isn't it the RDMA R/W code that decides whether or not to use memory 
registration?

> + * For RDMA READs we must use MRs on iWarp and can optionaly use them as an
> + * optimaztion otherwise.  Additionally we have a debug option to force usage
> + * of MRs to help testing this code path.

You may want to change "optionaly" into "optionally" and "optimaztion" 
into "optimization".

>   static inline bool rdma_rw_io_needs_mr(struct ib_device *dev, u8 port_num,
>   		enum dma_data_direction dir, int dma_nents)
>   {
> -	if (rdma_protocol_iwarp(dev, port_num) && dir == DMA_FROM_DEVICE)
> -		return true;
> +	if (dir == DMA_FROM_DEVICE) {
> +		if (rdma_protocol_iwarp(dev, port_num))
> +			return true;
> +		if (dev->attrs.max_sgl_rd && dma_nents > dev->attrs.max_sgl_rd)
> +			return true;
> +	}
>   	if (unlikely(rdma_rw_force_mr))
>   		return true;
>   	return false;

Should this function be renamed? The function name suggests if this 
function returns 'true' that using memory registration is mandatory. My 
understanding is if this function returns true for the mlx5 HCA that 
using memory registration improves performance but is not mandatory.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages
  2019-10-07 13:59 ` [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages Leon Romanovsky
  2019-10-07 15:00   ` Christoph Hellwig
  2019-10-07 15:07   ` Bart Van Assche
@ 2019-10-07 15:12   ` Bart Van Assche
  2019-10-07 15:58     ` Leon Romanovsky
  2 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2019-10-07 15:12 UTC (permalink / raw)
  To: Leon Romanovsky, Doug Ledford, Jason Gunthorpe, Christoph Hellwig
  Cc: Leon Romanovsky, RDMA mailing list, Or Gerlitz, Yamin Friedman,
	Saeed Mahameed, linux-netdev

On 10/7/19 6:59 AM, Leon Romanovsky wrote:
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 4f671378dbfc..60fd98a9b7e8 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -445,6 +445,8 @@ struct ib_device_attr {
>   	struct ib_tm_caps	tm_caps;
>   	struct ib_cq_caps       cq_caps;
>   	u64			max_dm_size;
> +	/* Max entries for sgl for optimized performance per READ */
                                    ^^^^^^^^^
                                    optimal?

Should it be mentioned that zero means that the HCA has not set this 
parameter?

> +	u32			max_sgl_rd;

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH mlx5-next v2 1/3] net/mlx5: Expose optimal performance scatter entries capability
  2019-10-07 15:02   ` Bart Van Assche
@ 2019-10-07 15:54     ` Leon Romanovsky
  0 siblings, 0 replies; 16+ messages in thread
From: Leon Romanovsky @ 2019-10-07 15:54 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Doug Ledford, Jason Gunthorpe, Christoph Hellwig,
	RDMA mailing list, Or Gerlitz, Yamin Friedman, Saeed Mahameed,
	linux-netdev

On Mon, Oct 07, 2019 at 08:02:50AM -0700, Bart Van Assche wrote:
> On 10/7/19 6:59 AM, Leon Romanovsky wrote:
> > -	u8         reserved_at_c0[0x8];
> > +	u8         max_sgl_for_optimized_performance[0x8];
>
> Should the name of this member variable perhaps be changed into
> "max_sgl_for_optimal_performance"?

We don't want to force our internal HW/FW names on all uverbs users
and drivers. So, the answer is no, it is used for optimized performance,
but it is not the proper name for uverbs.

Thanks

>
> Thanks,
>
> Bart.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages
  2019-10-07 15:00   ` Christoph Hellwig
@ 2019-10-07 15:57     ` Leon Romanovsky
  0 siblings, 0 replies; 16+ messages in thread
From: Leon Romanovsky @ 2019-10-07 15:57 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Doug Ledford, Jason Gunthorpe, RDMA mailing list, Or Gerlitz,
	Yamin Friedman, Saeed Mahameed, linux-netdev

On Mon, Oct 07, 2019 at 08:00:41AM -0700, Christoph Hellwig wrote:
> >   */
> > +
> >  static inline bool rdma_rw_io_needs_mr(struct ib_device *dev, u8 port_num,
>
> Same like an empty line sneaked in here.  Except for that the whole
> series looks fine:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages
  2019-10-07 15:12   ` Bart Van Assche
@ 2019-10-07 15:58     ` Leon Romanovsky
  0 siblings, 0 replies; 16+ messages in thread
From: Leon Romanovsky @ 2019-10-07 15:58 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Doug Ledford, Jason Gunthorpe, Christoph Hellwig,
	RDMA mailing list, Or Gerlitz, Yamin Friedman, Saeed Mahameed,
	linux-netdev

On Mon, Oct 07, 2019 at 08:12:37AM -0700, Bart Van Assche wrote:
> On 10/7/19 6:59 AM, Leon Romanovsky wrote:
> > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index 4f671378dbfc..60fd98a9b7e8 100644
> > --- a/include/rdma/ib_verbs.h
> > +++ b/include/rdma/ib_verbs.h
> > @@ -445,6 +445,8 @@ struct ib_device_attr {
> >   	struct ib_tm_caps	tm_caps;
> >   	struct ib_cq_caps       cq_caps;
> >   	u64			max_dm_size;
> > +	/* Max entries for sgl for optimized performance per READ */
>                                    ^^^^^^^^^
>                                    optimal?
>
> Should it be mentioned that zero means that the HCA has not set this
> parameter?

It is always the case for other max_* values.

>
> > +	u32			max_sgl_rd;
>
> Thanks,
>
> Bart.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages
  2019-10-07 15:07   ` Bart Van Assche
@ 2019-10-07 16:03     ` Leon Romanovsky
  2019-10-07 22:22       ` Bart Van Assche
  0 siblings, 1 reply; 16+ messages in thread
From: Leon Romanovsky @ 2019-10-07 16:03 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Doug Ledford, Jason Gunthorpe, Christoph Hellwig,
	RDMA mailing list, Or Gerlitz, Yamin Friedman, Saeed Mahameed,
	linux-netdev

On Mon, Oct 07, 2019 at 08:07:55AM -0700, Bart Van Assche wrote:
> On 10/7/19 6:59 AM, Leon Romanovsky wrote:
> >   /*
> > - * Check if the device might use memory registration.  This is currently only
> > - * true for iWarp devices. In the future we can hopefully fine tune this based
> > - * on HCA driver input.
> > + * Check if the device might use memory registration. This is currently
> > + * true for iWarp devices and devices that have optimized SGL registration
> > + * logic.
> >    */
>
> The following sentence in the above comment looks confusing to me: "Check if
> the device might use memory registration." That sentence suggests that the
> HCA decides whether or not to use memory registration. Isn't it the RDMA R/W
> code that decides whether or not to use memory registration?

I'm open for any reasonable text, what do you expect to be written there?

>
> > + * For RDMA READs we must use MRs on iWarp and can optionaly use them as an
> > + * optimaztion otherwise.  Additionally we have a debug option to force usage
> > + * of MRs to help testing this code path.
>
> You may want to change "optionaly" into "optionally" and "optimaztion" into
> "optimization".

Thanks

>
> >   static inline bool rdma_rw_io_needs_mr(struct ib_device *dev, u8 port_num,
> >   		enum dma_data_direction dir, int dma_nents)
> >   {
> > -	if (rdma_protocol_iwarp(dev, port_num) && dir == DMA_FROM_DEVICE)
> > -		return true;
> > +	if (dir == DMA_FROM_DEVICE) {
> > +		if (rdma_protocol_iwarp(dev, port_num))
> > +			return true;
> > +		if (dev->attrs.max_sgl_rd && dma_nents > dev->attrs.max_sgl_rd)
> > +			return true;
> > +	}
> >   	if (unlikely(rdma_rw_force_mr))
> >   		return true;
> >   	return false;
>
> Should this function be renamed? The function name suggests if this function
> returns 'true' that using memory registration is mandatory. My understanding
> is if this function returns true for the mlx5 HCA that using memory
> registration improves performance but is not mandatory.

The end result the same, better to work with MR while working with mlx5 for "dma_nents >
dev->attrs.max_sgl_rd",

Thanks

>
> Thanks,
>
> Bart.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages
  2019-10-07 16:03     ` Leon Romanovsky
@ 2019-10-07 22:22       ` Bart Van Assche
  2019-10-08  5:53         ` Leon Romanovsky
  0 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2019-10-07 22:22 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Jason Gunthorpe, Christoph Hellwig,
	RDMA mailing list, Or Gerlitz, Yamin Friedman, Saeed Mahameed,
	linux-netdev

On 10/7/19 9:03 AM, Leon Romanovsky wrote:
> On Mon, Oct 07, 2019 at 08:07:55AM -0700, Bart Van Assche wrote:
>> On 10/7/19 6:59 AM, Leon Romanovsky wrote:
>>>    /*
>>> - * Check if the device might use memory registration.  This is currently only
>>> - * true for iWarp devices. In the future we can hopefully fine tune this based
>>> - * on HCA driver input.
>>> + * Check if the device might use memory registration. This is currently
>>> + * true for iWarp devices and devices that have optimized SGL registration
>>> + * logic.
>>>     */
>>
>> The following sentence in the above comment looks confusing to me: "Check if
>> the device might use memory registration." That sentence suggests that the
>> HCA decides whether or not to use memory registration. Isn't it the RDMA R/W
>> code that decides whether or not to use memory registration?
> 
> I'm open for any reasonable text, what do you expect to be written there?

Hi Leon,

How about the following (not sure whether this is correct)?

/*
  * Report whether memory registration should be used. Memory
  * registration must be used for iWarp devices because of
  * iWARP-specific limitations. Memory registration is also enabled if
  * registering memory will yield better performance than using multiple
  * SGE entries.
  */

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages
  2019-10-07 22:22       ` Bart Van Assche
@ 2019-10-08  5:53         ` Leon Romanovsky
  0 siblings, 0 replies; 16+ messages in thread
From: Leon Romanovsky @ 2019-10-08  5:53 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Doug Ledford, Jason Gunthorpe, Christoph Hellwig,
	RDMA mailing list, Or Gerlitz, Yamin Friedman, Saeed Mahameed,
	linux-netdev

On Mon, Oct 07, 2019 at 03:22:30PM -0700, Bart Van Assche wrote:
> On 10/7/19 9:03 AM, Leon Romanovsky wrote:
> > On Mon, Oct 07, 2019 at 08:07:55AM -0700, Bart Van Assche wrote:
> > > On 10/7/19 6:59 AM, Leon Romanovsky wrote:
> > > >    /*
> > > > - * Check if the device might use memory registration.  This is currently only
> > > > - * true for iWarp devices. In the future we can hopefully fine tune this based
> > > > - * on HCA driver input.
> > > > + * Check if the device might use memory registration. This is currently
> > > > + * true for iWarp devices and devices that have optimized SGL registration
> > > > + * logic.
> > > >     */
> > >
> > > The following sentence in the above comment looks confusing to me: "Check if
> > > the device might use memory registration." That sentence suggests that the
> > > HCA decides whether or not to use memory registration. Isn't it the RDMA R/W
> > > code that decides whether or not to use memory registration?
> >
> > I'm open for any reasonable text, what do you expect to be written there?
>
> Hi Leon,
>
> How about the following (not sure whether this is correct)?
>
> /*
>  * Report whether memory registration should be used. Memory
>  * registration must be used for iWarp devices because of
>  * iWARP-specific limitations. Memory registration is also enabled if
>  * registering memory will yield better performance than using multiple
>  * SGE entries.
>  */

"Better performance" is relevant for mlx5 only, maybe others will use
this max_.. field to overcome their HW limitations.

Thanks

>
> Thanks,
>
> Bart.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH mlx5-next v2 1/3] net/mlx5: Expose optimal performance scatter entries capability
  2019-10-07 13:59 ` [PATCH mlx5-next v2 1/3] net/mlx5: Expose optimal performance scatter entries capability Leon Romanovsky
  2019-10-07 15:02   ` Bart Van Assche
@ 2019-10-08  5:55   ` Leon Romanovsky
  1 sibling, 0 replies; 16+ messages in thread
From: Leon Romanovsky @ 2019-10-08  5:55 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe, Christoph Hellwig
  Cc: RDMA mailing list, Or Gerlitz, Yamin Friedman, Saeed Mahameed,
	linux-netdev

On Mon, Oct 07, 2019 at 04:59:31PM +0300, Leon Romanovsky wrote:
> From: Yamin Friedman <yaminf@mellanox.com>
>
> Expose maximum scatter entries per RDMA READ for optimal performance.
>
> Signed-off-by: Yamin Friedman <yaminf@mellanox.com>
> Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
>  include/linux/mlx5/mlx5_ifc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

I'm taking this patch to mlx5-next, since it is not controversial.

Thanks

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH rdma-next v2 0/3] Optimize SGL registration
  2019-10-07 13:59 [PATCH rdma-next v2 0/3] Optimize SGL registration Leon Romanovsky
                   ` (2 preceding siblings ...)
  2019-10-07 13:59 ` [PATCH rdma-next v2 3/3] RDMA/mlx5: Add capability for max sge to get optimized performance Leon Romanovsky
@ 2019-10-15  8:15 ` Leon Romanovsky
  3 siblings, 0 replies; 16+ messages in thread
From: Leon Romanovsky @ 2019-10-15  8:15 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe, Christoph Hellwig
  Cc: RDMA mailing list, Or Gerlitz, Yamin Friedman, Saeed Mahameed,
	linux-netdev

On Mon, Oct 07, 2019 at 04:59:30PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Changelog
> v1->v2: https://lore.kernel.org/linux-rdma/20191007115819.9211-1-leon@kernel.org
>  * Used Christoph's comment
>  * Change patch code flow to have one DMA_FROM_DEVICE check
> v0->v1: https://lore.kernel.org/linux-rdma/20191006155955.31445-1-leon@kernel.org
>  * Reorganized patches to have IB/core changes separated from mlx5
>  * Moved SGL check before rdma_rw_force_mr
>  * Added and rephrased original comment.
>
> -----------------------------------------------------------------------------
> Hi,
>
> This series from Yamin implements long standing "TODO" existed in rw.c.
>
> Thanks

Jason, Doug

Do you expect anything from me in regards to this series?

Thanks

>
> Yamin Friedman (3):
>   net/mlx5: Expose optimal performance scatter entries capability
>   RDMA/rw: Support threshold for registration vs scattering to local
>     pages
>   RDMA/mlx5: Add capability for max sge to get optimized performance
>
>  drivers/infiniband/core/rw.c      | 25 +++++++++++++++----------
>  drivers/infiniband/hw/mlx5/main.c |  2 ++
>  include/linux/mlx5/mlx5_ifc.h     |  2 +-
>  include/rdma/ib_verbs.h           |  2 ++
>  4 files changed, 20 insertions(+), 11 deletions(-)
>
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, back to index

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-07 13:59 [PATCH rdma-next v2 0/3] Optimize SGL registration Leon Romanovsky
2019-10-07 13:59 ` [PATCH mlx5-next v2 1/3] net/mlx5: Expose optimal performance scatter entries capability Leon Romanovsky
2019-10-07 15:02   ` Bart Van Assche
2019-10-07 15:54     ` Leon Romanovsky
2019-10-08  5:55   ` Leon Romanovsky
2019-10-07 13:59 ` [PATCH rdma-next v2 2/3] RDMA/rw: Support threshold for registration vs scattering to local pages Leon Romanovsky
2019-10-07 15:00   ` Christoph Hellwig
2019-10-07 15:57     ` Leon Romanovsky
2019-10-07 15:07   ` Bart Van Assche
2019-10-07 16:03     ` Leon Romanovsky
2019-10-07 22:22       ` Bart Van Assche
2019-10-08  5:53         ` Leon Romanovsky
2019-10-07 15:12   ` Bart Van Assche
2019-10-07 15:58     ` Leon Romanovsky
2019-10-07 13:59 ` [PATCH rdma-next v2 3/3] RDMA/mlx5: Add capability for max sge to get optimized performance Leon Romanovsky
2019-10-15  8:15 ` [PATCH rdma-next v2 0/3] Optimize SGL registration Leon Romanovsky

Linux-RDMA Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rdma/0 linux-rdma/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rdma linux-rdma/ https://lore.kernel.org/linux-rdma \
		linux-rdma@vger.kernel.org linux-rdma@archiver.kernel.org
	public-inbox-index linux-rdma

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rdma


AGPL code for this site: git clone https://public-inbox.org/ public-inbox