Re: [RFC PATCH rdma-next 05/10] RDMA/rxe: Allow registering persistent flag for pmem MR only

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/28/2021 3:07 AM, Li Zhijian wrote:
Memory region should support 2 placement types: IB_ACCESS_FLUSH_PERSISTENT
and IB_ACCESS_FLUSH_GLOBAL_VISIBILITY, and only pmem/nvdimm has ability to
persist data(IB_ACCESS_FLUSH_PERSISTENT).

Signed-off-by: Li Zhijian <lizhijian@xxxxxxxxxxxxxx>
---
  drivers/infiniband/sw/rxe/rxe_mr.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index bcd5e7afa475..21616d058f29 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -206,6 +206,11 @@ static bool iova_in_pmem(struct rxe_mr *mr, u64 iova, int length)
  	return page_in_dev_pagemap(page);
  }
+static bool ib_check_flush_access_flags(struct ib_mr *mr, u32 flags)
+{
+	return mr->is_pmem || !(flags & IB_ACCESS_FLUSH_PERSISTENT);
+}

It is perfectly allowed to flush ordinary memory, persistence is
another matter entirely. Is this subroutine checking for flush,
or persistence? Its name is confusing and needs to be clarified.

+
  int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova,
  		     int access, struct rxe_mr *mr)
  {
@@ -282,6 +287,13 @@ int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova,
// iova_in_pmem must be called after set is updated
  	mr->ibmr.is_pmem = iova_in_pmem(mr, iova, length);
+	if (!ib_check_flush_access_flags(&mr->ibmr, access)) {
+		pr_err("Cannot set IB_ACCESS_FLUSH_PERSISTENT for non-pmem memory\n");
+		mr->state = RXE_MR_STATE_INVALID;
+		mr->umem = NULL;
+		err = -EINVAL;
+		goto err_release_umem;
+	}

Setting is_pmem is reasonable, but again, this is confusing with respect
to the region being flushable. In general, all memory is flushable,
provided the platform supports any kind of cache flush (i.e. all of them).

Tom.



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux