On 5/4/2019 11:28 PM, Leon Romanovsky wrote:

On Thu, May 02, 2019 at 10:52:23AM -0700, Santosh Shilimkar wrote:

quoted

On 5/1/2019 11:21 PM, Leon Romanovsky wrote:

quoted

On Wed, May 01, 2019 at 10:54:00AM -0700, Santosh Shilimkar wrote:

quoted

On 5/1/2019 12:44 AM, Leon Romanovsky wrote:

quoted

On Mon, Apr 29, 2019 at 04:37:19PM -0700, Santosh Shilimkar wrote:

quoted

From: Hans Westgaard Ry <redacted>

RDS doesn't support RDMA on memory apertures that require On Demand
Paging (ODP), such as FS DAX memory. User applications can try to use
RDS to perform RDMA over such memories and since it doesn't report any
failure, it can lead to unexpected issues like memory corruption when
a couple of out of sync file system operations like ftruncate etc. are
performed.

The patch adds a check so that such an attempt to RDMA to/from memory
apertures requiring ODP will fail.

Reviewed-by: H??kon Bugge <redacted>
Reviewed-tested-by: Zhu Yanjun [off-list ref]
Signed-off-by: Hans Westgaard Ry <redacted>
Signed-off-by: Santosh Shilimkar <redacted>
---
    net/rds/rdma.c | 5 +++--
    1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/rds/rdma.c b/net/rds/rdma.c
index 182ab84..e0a6b72 100644
--- a/net/rds/rdma.c
+++ b/net/rds/rdma.c

@@ -158,8 +158,9 @@ static int rds_pin_pages(unsigned long user_addr, unsigned int nr_pages,
    {
    	int ret;

-	ret = get_user_pages_fast(user_addr, nr_pages, write, pages);
-
+	/* get_user_pages return -EOPNOTSUPP for fs_dax memory */
+	ret = get_user_pages_longterm(user_addr, nr_pages,
+				      write, pages, NULL);

I'm not RDS expert, but from what I see in net/rds/rdma.c and this code,
you tried to mimic ib_umem_get() without protection, checks and native
ODP, FS and DAX supports.

The real way to solve your ODP problem will require to extend
ib_umem_get() to work for kernel ULPs too and use it instead of
get_user_pages(). We are working on that and it is in internal review now.

Yes am aware of it. For FS_DAX like memory,  get_user_pages_longterm()
fails and then using ib_reg_user_mr() the memory is registered as
ODP regsion. This work is not ready yet and without above check,
one can do RDMA on FS DAX memory with Fast Reg or FMR memory
registration which is not safe and hence need to fail the operation.

Once the support is added to RDS, this code path will make that
registration go through.

Hope it clarifies.

Only partial, why don't you check if user asked ODP through verbs
interface and return EOPNOTSUPP in such case?

I think you are mixing two separate things. ODP is just one way of
supporting RDMA on FS DAX memory. Tomorrow, some other mechanism
can be used as well. RDS is just using inbuilt kernel mm API
to find out if its FS DAX memory(get_user_pages_longterm).
Current code will make RDS get_mr fail if RDS application issues
memory registration request on FS DAX memory and in future when
support gets added, it will do the ODP registration and return
the key.

But we are talking about kernel code only, right?
Future support will be added if it exists.

yes kernel code only.

quoted

quoted

It will ensure that once your code will support ODP properly written
applications will work with/without ODP natively.

Application shouldn't care if RDS ULP internally uses ODP
or some other mechanism to support RDMA on FS DAX memory.
This makes it transparent it to RDS application.

ODP checks need to be internal to kernel, user won't see those ODP
checks.

Correct. The check is within RDS kernel module.