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...@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.bu...@oracle.com>
> > > Reviewed-tested-by: Zhu Yanjun <yanjun....@oracle.com>
> > > Signed-off-by: Hans Westgaard Ry <hans.westgaard...@oracle.com>
> > > Signed-off-by: Santosh Shilimkar <santosh.shilim...@oracle.com>
> > > 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.
>
> > 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
>
> > 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
Jason
