On Fri, May 10, 2019 at 11:02:35AM -0700, santosh.shilimkar@oracle.com wrote:

On 5/10/19 10:55 AM, Jason Gunthorpe wrote:

quoted

On Fri, May 10, 2019 at 09:11:24AM -0700, Santosh Shilimkar wrote:

quoted

On 5/10/2019 5:54 AM, Jason Gunthorpe 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.

This comment doesn't make any sense..

quoted

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
+++ 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);

GUP is supposed to fully work on DAX filesystems.

Above comment has typo. Should have been
get_user_pages_longterm return -EOPNOTSUPP.

quoted

You only need to switch to the long term version if the duration of
the GUP is under control of user space - ie it may last forever.

Short duration pins in the kernel do not need long term.

Thats true but the intention here is to use the long term version
which does check for the FS DAX memory. Instead of calling direct
accessor to check DAX memory region, longterm version of the API
is used

quoted

At a minimum the commit message needs re-writing to properly explain
the motivation here.

Commit is actually trying to describe the motivation describing more of
issues of not making the call fail. The code comment typo was
misleading.

Every single sentence in the commit message is wrong

I will rewrite commit message but can you please comment on other
questions above. GUP long term was used to detect whether its
fs_dax memory which could be misleading since the RDS MRs are
short lived. Do you want us to use accessor instead to check
if its FS DAX memory?

Why would you need to detect FS DAX memory? GUP users are not supposed
to care.

GUP is supposed to work just 'fine' - other than the usual bugs we
have with GUP and any FS backed memory.

Jason