diff mbox

[dpdk-dev,2/2] net/mlx5: don't map doorbell register to write combining

Message ID 1503301622-14220-3-git-send-email-sagi@grimberg.me (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Sagi Grimberg Aug. 21, 2017, 7:47 a.m. UTC 
  From: Shahaf Shuler <shahafs@mellanox.com>

By default, Verbs maps the doorbell register to write combining.
Working with write combining is useful for drivers which use blue flame
for the doorbell write.

Since mlx5 PMD uses only doorbells and write combining mapping requires
an extra memory barrier to flush the doorbell after its write, setting
the mapping to un-cached by default.

Such change is reduces the max and average round trip
latency significantly.

Reported-by: Alexander Solganik <solganik@gmail.com>
Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Signed-off-by: Alexander Solganik <solganik@gmail.com>
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/net/mlx5/mlx5.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Ferruh Yigit Aug. 23, 2017, 11:03 a.m. UTC | #1 
On 8/21/2017 8:47 AM, Sagi Grimberg wrote:
> From: Shahaf Shuler <shahafs@mellanox.com>
> 
> By default, Verbs maps the doorbell register to write combining.
> Working with write combining is useful for drivers which use blue flame
> for the doorbell write.
> 
> Since mlx5 PMD uses only doorbells and write combining mapping requires
> an extra memory barrier to flush the doorbell after its write, setting
> the mapping to un-cached by default.
> 
> Such change is reduces the max and average round trip
> latency significantly.
> 
> Reported-by: Alexander Solganik <solganik@gmail.com>
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> Signed-off-by: Alexander Solganik <solganik@gmail.com>
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
> ---
>  drivers/net/mlx5/mlx5.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index b7e5046325c0..4c2a0b9652e3 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -920,6 +920,8 @@ rte_mlx5_pmd_init(void)
>  	 * using this PMD, which is not supported in forked processes.
>  	 */
>  	setenv("RDMAV_HUGEPAGES_SAFE", "1", 1);
> +	/* Don't map UAR to WC if BlueFlame is not used.*/
> +	setenv("MLX5_SHUT_UP_BF", "1", 1);

Although technically this is possible, I wonder how good idea it is a
driver communicating with other processes via system environment variable?

>  	ibv_fork_init();
>  	rte_pci_register(&mlx5_driver);
>  }
>
Shahaf Shuler Aug. 23, 2017, 11:58 a.m. UTC | #2 
Wednesday, August 23, 2017 2:04 PM, Ferruh Yigit:
> On 8/21/2017 8:47 AM, Sagi Grimberg wrote:

> > From: Shahaf Shuler <shahafs@mellanox.com>

> >

> > By default, Verbs maps the doorbell register to write combining.

> > Working with write combining is useful for drivers which use blue

> > flame for the doorbell write.

> >

> > Since mlx5 PMD uses only doorbells and write combining mapping

> > requires an extra memory barrier to flush the doorbell after its

> > write, setting the mapping to un-cached by default.

> >

> > Such change is reduces the max and average round trip latency

> > significantly.

> >

> > Reported-by: Alexander Solganik <solganik@gmail.com>

> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>

> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>

> > Signed-off-by: Alexander Solganik <solganik@gmail.com>

> > Signed-off-by: Sagi Grimberg <sagi@grimberg.me>

> > ---

> >  drivers/net/mlx5/mlx5.c | 2 ++

> >  1 file changed, 2 insertions(+)

> >

> > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index

> > b7e5046325c0..4c2a0b9652e3 100644

> > --- a/drivers/net/mlx5/mlx5.c

> > +++ b/drivers/net/mlx5/mlx5.c

> > @@ -920,6 +920,8 @@ rte_mlx5_pmd_init(void)

> >  	 * using this PMD, which is not supported in forked processes.

> >  	 */

> >  	setenv("RDMAV_HUGEPAGES_SAFE", "1", 1);

> > +	/* Don't map UAR to WC if BlueFlame is not used.*/

> > +	setenv("MLX5_SHUT_UP_BF", "1", 1);


Even if it is not clean, this is the only way to toggle the BF mapping mode on libmlx5/rdma-core.
We will try to propose changes on those libs to expose a proper API, in the meanwhile we have no other way.

> 

> Although technically this is possible, I wonder how good idea it is a driver

> communicating with other processes via system environment variable?

> 

> >  	ibv_fork_init();

> >  	rte_pci_register(&mlx5_driver);

> >  }

> >
Nélio Laranjeiro Aug. 23, 2017, 12:06 p.m. UTC | #3 
On Wed, Aug 23, 2017 at 12:03:39PM +0100, Ferruh Yigit wrote:
> On 8/21/2017 8:47 AM, Sagi Grimberg wrote:
> > From: Shahaf Shuler <shahafs@mellanox.com>
> > 
> > By default, Verbs maps the doorbell register to write combining.
> > Working with write combining is useful for drivers which use blue flame
> > for the doorbell write.
> > 
> > Since mlx5 PMD uses only doorbells and write combining mapping requires
> > an extra memory barrier to flush the doorbell after its write, setting
> > the mapping to un-cached by default.
> > 
> > Such change is reduces the max and average round trip
> > latency significantly.
> > 
> > Reported-by: Alexander Solganik <solganik@gmail.com>
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > Signed-off-by: Alexander Solganik <solganik@gmail.com>
> > Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
> > ---
> >  drivers/net/mlx5/mlx5.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> > index b7e5046325c0..4c2a0b9652e3 100644
> > --- a/drivers/net/mlx5/mlx5.c
> > +++ b/drivers/net/mlx5/mlx5.c
> > @@ -920,6 +920,8 @@ rte_mlx5_pmd_init(void)
> >  	 * using this PMD, which is not supported in forked processes.
> >  	 */
> >  	setenv("RDMAV_HUGEPAGES_SAFE", "1", 1);
> > +	/* Don't map UAR to WC if BlueFlame is not used.*/
> > +	setenv("MLX5_SHUT_UP_BF", "1", 1);
> 
> Although technically this is possible, I wonder how good idea it is a
> driver communicating with other processes via system environment variable?

We don't have such flexibility though verbs, those environment variable are
read by libmlx5 code and the PMD is linked though libibverbs which does not
provide a "clean" way tweak libmlx5.
It is not used to communicate with another process, it is the same process
executing the code which reads those environment variables.
Currently it is the only solution we have.

For the patch:

Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Thanks,
diff mbox

Patch

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index b7e5046325c0..4c2a0b9652e3 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -920,6 +920,8 @@  rte_mlx5_pmd_init(void)
 	 * using this PMD, which is not supported in forked processes.
 	 */
 	setenv("RDMAV_HUGEPAGES_SAFE", "1", 1);
+	/* Don't map UAR to WC if BlueFlame is not used.*/
+	setenv("MLX5_SHUT_UP_BF", "1", 1);
 	ibv_fork_init();
 	rte_pci_register(&mlx5_driver);
 }