On Thu, Jun 24, 2021 at 10:39:16AM +0300, Max Gurtovoy wrote:

On 6/24/2021 9:38 AM, Leon Romanovsky wrote:

quoted

On Thu, Jun 24, 2021 at 02:06:46AM +0300, Max Gurtovoy wrote:

quoted

On 6/9/2021 2:05 PM, Leon Romanovsky wrote:

quoted

From: Avihai Horon <redacted>

Relaxed Ordering is a capability that can only benefit users that support
it. All kernel ULPs should support Relaxed Ordering, as they are designed
to read data only after observing the CQE and use the DMA API correctly.

Hence, implicitly enable Relaxed Ordering by default for kernel ULPs.

Signed-off-by: Avihai Horon <redacted>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
Changelog:
v2:
   * Dropped IB/core patch and set RO implicitly in mlx5 exactly like in
     eth side of mlx5 driver.
v1: https://lore.kernel.org/lkml/cover.1621505111.git.leonro@nvidia.com (local)
   * Enabled by default RO in IB/core instead of changing all users
v0: https://lore.kernel.org/lkml/20210405052404.213889-1-leon@kernel.org (local)
   drivers/infiniband/hw/mlx5/mr.c | 10 ++++++----
   drivers/infiniband/hw/mlx5/wr.c |  5 ++++-
   2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index 3363cde85b14..2182e76ae734 100644
+++ b/drivers/infiniband/hw/mlx5/mr.c

@@ -69,6 +69,7 @@ static void set_mkc_access_pd_addr_fields(void *mkc, int acc, u64 start_addr,
   					  struct ib_pd *pd)
   {
   	struct mlx5_ib_dev *dev = to_mdev(pd->device);
+	bool ro_pci_enabled = pcie_relaxed_ordering_enabled(dev->mdev->pdev);
   	MLX5_SET(mkc, mkc, a, !!(acc & IB_ACCESS_REMOTE_ATOMIC));
   	MLX5_SET(mkc, mkc, rw, !!(acc & IB_ACCESS_REMOTE_WRITE));

@@ -78,10 +79,10 @@ static void set_mkc_access_pd_addr_fields(void *mkc, int acc, u64 start_addr,
   	if (MLX5_CAP_GEN(dev->mdev, relaxed_ordering_write))
   		MLX5_SET(mkc, mkc, relaxed_ordering_write,
-			 !!(acc & IB_ACCESS_RELAXED_ORDERING));
+			 acc & IB_ACCESS_RELAXED_ORDERING && ro_pci_enabled);
   	if (MLX5_CAP_GEN(dev->mdev, relaxed_ordering_read))
   		MLX5_SET(mkc, mkc, relaxed_ordering_read,
-			 !!(acc & IB_ACCESS_RELAXED_ORDERING));
+			 acc & IB_ACCESS_RELAXED_ORDERING && ro_pci_enabled);

Jason,

If it's still possible to add small change, it will be nice to avoid
calculating "acc & IB_ACCESS_RELAXED_ORDERING && ro_pci_enabled" twice.

The patch is part of for-next now, so feel free to send followup patch.

Thanks

diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index c1e70c99b70c..c4f246c90c4d 100644
+++ b/drivers/infiniband/hw/mlx5/mr.c

@@ -69,7 +69,8 @@ static void set_mkc_access_pd_addr_fields(void *mkc, int acc, u64 start_addr,
                                           struct ib_pd *pd)
  {
         struct mlx5_ib_dev *dev = to_mdev(pd->device);
-       bool ro_pci_enabled = pcie_relaxed_ordering_enabled(dev->mdev->pdev);
+       bool ro_pci_enabled = acc & IB_ACCESS_RELAXED_ORDERING &&
+                             pcie_relaxed_ordering_enabled(dev->mdev->pdev);

         MLX5_SET(mkc, mkc, a, !!(acc & IB_ACCESS_REMOTE_ATOMIC));
         MLX5_SET(mkc, mkc, rw, !!(acc & IB_ACCESS_REMOTE_WRITE));

@@ -78,11 +79,9 @@ static void set_mkc_access_pd_addr_fields(void *mkc, int acc, u64 start_addr,
         MLX5_SET(mkc, mkc, lr, 1);

         if (MLX5_CAP_GEN(dev->mdev, relaxed_ordering_write))
-               MLX5_SET(mkc, mkc, relaxed_ordering_write,
-                        (acc & IB_ACCESS_RELAXED_ORDERING) && ro_pci_enabled);
+               MLX5_SET(mkc, mkc, relaxed_ordering_write, ro_pci_enabled);
         if (MLX5_CAP_GEN(dev->mdev, relaxed_ordering_read))
-               MLX5_SET(mkc, mkc, relaxed_ordering_read,
-                        (acc & IB_ACCESS_RELAXED_ORDERING) && ro_pci_enabled);
+               MLX5_SET(mkc, mkc, relaxed_ordering_read, ro_pci_enabled);

         MLX5_SET(mkc, mkc, pd, to_mpd(pd)->pdn);
         MLX5_SET(mkc, mkc, qpn, 0xffffff);

(END)

Yes this looks good.

Can you/Avihai create a patch from this ? or I'll do it ?

I'd be surpised if it matters.. CSE and all

Jason