netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@ziepe.ca>
To: "santosh.shilimkar@oracle.com" <santosh.shilimkar@oracle.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net,
	Hans Westgaard Ry <hans.westgaard.ry@oracle.com>
Subject: Re: [net-next][PATCH v2 1/2] rds: handle unsupported rdma request to fs dax memory
Date: Fri, 10 May 2019 16:20:46 -0300	[thread overview]
Message-ID: <20190510192046.GH13038@ziepe.ca> (raw)
In-Reply-To: <d25910f3-51f1-9214-3479-150b1d320e43@oracle.com>

On Fri, May 10, 2019 at 11:58:42AM -0700, santosh.shilimkar@oracle.com wrote:
> On 5/10/19 11:07 AM, Jason Gunthorpe wrote:
> > 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:
> > > > On Fri, May 10, 2019 at 09:11:24AM -0700, Santosh Shilimkar wrote:
> > > > > On 5/10/2019 5:54 AM, Jason Gunthorpe wrote:
> > > > > > On Mon, Apr 29, 2019 at 04:37:19PM -0700, Santosh Shilimkar wrote:
> > > > > > > From: Hans Westgaard Ry <hans.westgaard.ry@oracle.com>
> > > > > > > 
> > > > > > > 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..
> > > > > > 
> > > > > > > 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 <haakon.bugge@oracle.com>
> > > > > > > Reviewed-tested-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> > > > > > > Signed-off-by: Hans Westgaard Ry <hans.westgaard.ry@oracle.com>
> > > > > > > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> > > > > > >     net/rds/rdma.c | 5 +++--
> > > > > > >     1 file changed, 3 insertions(+), 2 deletions(-)
> > > > > > > 
> [...]
> 
> > 
> > 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.
> > 
> Am not saying there is any issue with GUP. Let me try to explain the
> issue first. You are aware of various discussions about doing DMA
> or RDMA on FS DAX memory. e.g [1] [2] [3]
> 
> One of the proposal to do safely RDMA on FS DAX memory is/was ODP

It is not about safety. ODP is required in all places that would have
used gup_longterm because ODP avoids th gup_longterm entirely.

> Currently RDS doesn't have support for ODP MR registration
> and hence we don't want user application to do RDMA using
> fastreg/fmr on FS DAX memory which isn't safe.

No, it is safe.

The only issue is you need to determine if this use of GUP is longterm
or short term. Longterm means userspace is in control of how long the
GUP lasts, short term means the kernel is in control.

ie posting a fastreg, sending the data, then un-GUP'ing on completion
is a short term GUP and it is fine on any type of memory.

So if it is a long term pin then it needs to be corrected and the only
thing the comment needs to explain is that it is a long term pin.

Jason

  reply	other threads:[~2019-05-10 19:20 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-29 23:37 [net-next][PATCH v2 0/2] rds: handle unsupported rdma request to fs dax memory Santosh Shilimkar
2019-04-29 23:37 ` [net-next][PATCH v2 1/2] " Santosh Shilimkar
2019-05-01  7:44   ` Leon Romanovsky
2019-05-01 17:54     ` Santosh Shilimkar
2019-05-02  6:21       ` Leon Romanovsky
2019-05-02 17:52         ` Santosh Shilimkar
2019-05-05  6:28           ` Leon Romanovsky
2019-05-06 16:39             ` Santosh Shilimkar
2019-05-10 12:54   ` Jason Gunthorpe
2019-05-10 16:11     ` Santosh Shilimkar
2019-05-10 17:55       ` Jason Gunthorpe
2019-05-10 18:02         ` santosh.shilimkar
2019-05-10 18:07           ` Jason Gunthorpe
2019-05-10 18:58             ` santosh.shilimkar
2019-05-10 19:20               ` Jason Gunthorpe [this message]
2019-05-10 19:38                 ` Santosh Shilimkar
2019-05-10 19:47                   ` Jason Gunthorpe
2019-05-10 20:12                     ` Santosh Shilimkar
2019-05-10 20:39                       ` Jason Gunthorpe
2019-04-29 23:37 ` [net-next][PATCH v2 2/2] rds: add sysctl for rds support of On-Demand-Paging Santosh Shilimkar
2019-05-01  7:45   ` Leon Romanovsky
2019-05-01 17:54     ` Santosh Shilimkar
2019-05-02  6:18       ` Leon Romanovsky
2019-05-02 17:59         ` Santosh Shilimkar
2019-05-05  6:22           ` Leon Romanovsky
2019-05-06 16:37             ` Santosh Shilimkar
2019-05-10 13:02   ` Jason Gunthorpe
2019-05-10 16:13     ` Santosh Shilimkar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190510192046.GH13038@ziepe.ca \
    --to=jgg@ziepe.ca \
    --cc=davem@davemloft.net \
    --cc=hans.westgaard.ry@oracle.com \
    --cc=netdev@vger.kernel.org \
    --cc=santosh.shilimkar@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).