* [PATCH 0/1] net/rds: Track user mapped pages through special API @ 2020-02-12 3:03 John Hubbard 2020-02-12 3:03 ` [PATCH 1/1] " John Hubbard 0 siblings, 1 reply; 6+ messages in thread From: John Hubbard @ 2020-02-12 3:03 UTC (permalink / raw) To: Andrew Morton Cc: Santosh Shilimkar, Hans Westgaard Ry, Leon Romanovsky, David S . Miller, Jakub Kicinski, netdev, rds-devel, linux-rdma, linux-mm, LKML, John Hubbard Hi Andrew and all, Here's another gup-to-pup (get_user_pages() to pin_user_pages()) conversion patch, this time from Leon Romanovsky, that we agreed is better suited for the linux-mm tree than for linux-rdma. And it also couldn't be merged until now (5.6-rc1) because it relies on stuff from a few different git trees. (Leon: I added my standard blurb about "this changes set_page_dirty() to set_page_dirty_lock()", to the commit description.) I've reviewed this, and done some basic checks (cross-compiles, and a subset of an LTP run on x86), but I have not personally done directed tests that would provide coverage of this change. For that, could we please get some Tested-by tags, and any other tags (reviews, acks) from those of you who have reportedly tested this? That would be Hans or Santosh (on Cc below), I'm told: Cc: Hans Westgaard Ry <hans.westgaard.ry@oracle.com> Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com> Leon Romanovsky (1): net/rds: Track user mapped pages through special API net/rds/rdma.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) base-commit: 359c92c02bfae1a6f1e8e37c298e518fd256642c -- 2.25.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] net/rds: Track user mapped pages through special API 2020-02-12 3:03 [PATCH 0/1] net/rds: Track user mapped pages through special API John Hubbard @ 2020-02-12 3:03 ` John Hubbard 2020-02-12 17:31 ` santosh.shilimkar 2020-02-17 2:37 ` David Miller 0 siblings, 2 replies; 6+ messages in thread From: John Hubbard @ 2020-02-12 3:03 UTC (permalink / raw) To: Andrew Morton Cc: Santosh Shilimkar, Hans Westgaard Ry, Leon Romanovsky, David S . Miller, Jakub Kicinski, netdev, rds-devel, linux-rdma, linux-mm, LKML, John Hubbard From: Leon Romanovsky <leonro@mellanox.com> Convert net/rds to use the newly introduces pin_user_pages() API, which properly sets FOLL_PIN. Setting FOLL_PIN is now required for code that requires tracking of pinned pages. Note that this effectively changes the code's behavior: it now ultimately calls set_page_dirty_lock(), instead of set_page_dirty(). This is probably more accurate. As Christoph Hellwig put it, "set_page_dirty() is only safe if we are dealing with a file backed page where we have reference on the inode it hangs off." [1] [1] https://lore.kernel.org/r/20190723153640.GB720@lst.de Cc: Hans Westgaard Ry <hans.westgaard.ry@oracle.com> Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com> Signed-off-by: Leon Romanovsky <leonro@mellanox.com> Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- net/rds/rdma.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/net/rds/rdma.c b/net/rds/rdma.c index 3341eee87bf9..585e6b3b69ce 100644 --- a/net/rds/rdma.c +++ b/net/rds/rdma.c @@ -162,10 +162,9 @@ static int rds_pin_pages(unsigned long user_addr, unsigned int nr_pages, if (write) gup_flags |= FOLL_WRITE; - ret = get_user_pages_fast(user_addr, nr_pages, gup_flags, pages); + ret = pin_user_pages_fast(user_addr, nr_pages, gup_flags, pages); if (ret >= 0 && ret < nr_pages) { - while (ret--) - put_page(pages[ret]); + unpin_user_pages(pages, ret); ret = -EFAULT; } @@ -300,8 +299,7 @@ static int __rds_rdma_map(struct rds_sock *rs, struct rds_get_mr_args *args, * to release anything. */ if (!need_odp) { - for (i = 0 ; i < nents; i++) - put_page(sg_page(&sg[i])); + unpin_user_pages(pages, nr_pages); kfree(sg); } ret = PTR_ERR(trans_private); @@ -325,7 +323,12 @@ static int __rds_rdma_map(struct rds_sock *rs, struct rds_get_mr_args *args, if (cookie_ret) *cookie_ret = cookie; - if (args->cookie_addr && put_user(cookie, (u64 __user *)(unsigned long) args->cookie_addr)) { + if (args->cookie_addr && + put_user(cookie, (u64 __user *)(unsigned long)args->cookie_addr)) { + if (!need_odp) { + unpin_user_pages(pages, nr_pages); + kfree(sg); + } ret = -EFAULT; goto out; } @@ -496,9 +499,7 @@ void rds_rdma_free_op(struct rm_rdma_op *ro) * is the case for a RDMA_READ which copies from remote * to local memory */ - if (!ro->op_write) - set_page_dirty(page); - put_page(page); + unpin_user_pages_dirty_lock(&page, 1, !ro->op_write); } } @@ -515,8 +516,7 @@ void rds_atomic_free_op(struct rm_atomic_op *ao) /* Mark page dirty if it was possibly modified, which * is the case for a RDMA_READ which copies from remote * to local memory */ - set_page_dirty(page); - put_page(page); + unpin_user_pages_dirty_lock(&page, 1, true); kfree(ao->op_notifier); ao->op_notifier = NULL; @@ -944,7 +944,7 @@ int rds_cmsg_atomic(struct rds_sock *rs, struct rds_message *rm, return ret; err: if (page) - put_page(page); + unpin_user_page(page); rm->atomic.op_active = 0; kfree(rm->atomic.op_notifier); -- 2.25.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] net/rds: Track user mapped pages through special API 2020-02-12 3:03 ` [PATCH 1/1] " John Hubbard @ 2020-02-12 17:31 ` santosh.shilimkar 2020-02-12 17:51 ` Leon Romanovsky 2020-02-17 2:37 ` David Miller 1 sibling, 1 reply; 6+ messages in thread From: santosh.shilimkar @ 2020-02-12 17:31 UTC (permalink / raw) To: John Hubbard, Andrew Morton Cc: Hans Westgaard Ry, Leon Romanovsky, David S . Miller, Jakub Kicinski, netdev, rds-devel, linux-rdma, linux-mm, LKML On 2/11/20 7:03 PM, John Hubbard wrote: > From: Leon Romanovsky <leonro@mellanox.com> > > Convert net/rds to use the newly introduces pin_user_pages() API, > which properly sets FOLL_PIN. Setting FOLL_PIN is now required for > code that requires tracking of pinned pages. > > Note that this effectively changes the code's behavior: it now > ultimately calls set_page_dirty_lock(), instead of set_page_dirty(). > This is probably more accurate. > > As Christoph Hellwig put it, "set_page_dirty() is only safe if we are > dealing with a file backed page where we have reference on the inode it > hangs off." [1] > > [1] https://urldefense.com/v3/__https://lore.kernel.org/r/20190723153640.GB720@lst.de__;!!GqivPVa7Brio!OJHuecs9Iup5ig3kQBi_423uMMuskWhBQAdOICrY3UQ_ZfEaxt9ySY7E8y32Q7pk5tByyA$ > > Cc: Hans Westgaard Ry <hans.westgaard.ry@oracle.com> > Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- Change looks fine to me. Just on safer side, we will try to test this change with regression suite to make sure it works as expected. For patch itself, Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] net/rds: Track user mapped pages through special API 2020-02-12 17:31 ` santosh.shilimkar @ 2020-02-12 17:51 ` Leon Romanovsky 2020-02-12 17:55 ` santosh.shilimkar 0 siblings, 1 reply; 6+ messages in thread From: Leon Romanovsky @ 2020-02-12 17:51 UTC (permalink / raw) To: santosh.shilimkar Cc: John Hubbard, Andrew Morton, Hans Westgaard Ry, David S . Miller, Jakub Kicinski, netdev, rds-devel, linux-rdma, linux-mm, LKML On Wed, Feb 12, 2020 at 09:31:51AM -0800, santosh.shilimkar@oracle.com wrote: > On 2/11/20 7:03 PM, John Hubbard wrote: > > From: Leon Romanovsky <leonro@mellanox.com> > > > > Convert net/rds to use the newly introduces pin_user_pages() API, > > which properly sets FOLL_PIN. Setting FOLL_PIN is now required for > > code that requires tracking of pinned pages. > > > > Note that this effectively changes the code's behavior: it now > > ultimately calls set_page_dirty_lock(), instead of set_page_dirty(). > > This is probably more accurate. > > > > As Christoph Hellwig put it, "set_page_dirty() is only safe if we are > > dealing with a file backed page where we have reference on the inode it > > hangs off." [1] > > > > [1] https://urldefense.com/v3/__https://lore.kernel.org/r/20190723153640.GB720@lst.de__;!!GqivPVa7Brio!OJHuecs9Iup5ig3kQBi_423uMMuskWhBQAdOICrY3UQ_ZfEaxt9ySY7E8y32Q7pk5tByyA$ > > > > Cc: Hans Westgaard Ry <hans.westgaard.ry@oracle.com> > > Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > > --- > Change looks fine to me. Just on safer side, we will try > to test this change with regression suite to make sure it > works as expected. Thanks Santosh, I wrote this patch before John's series was merged into the tree, but back then, Hans tested it and it worked, hope that it still works. :) > > For patch itself, > > Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] net/rds: Track user mapped pages through special API 2020-02-12 17:51 ` Leon Romanovsky @ 2020-02-12 17:55 ` santosh.shilimkar 0 siblings, 0 replies; 6+ messages in thread From: santosh.shilimkar @ 2020-02-12 17:55 UTC (permalink / raw) To: Leon Romanovsky Cc: John Hubbard, Andrew Morton, Hans Westgaard Ry, David S . Miller, Jakub Kicinski, netdev, rds-devel, linux-rdma, linux-mm, LKML On 2/12/20 9:51 AM, Leon Romanovsky wrote: > On Wed, Feb 12, 2020 at 09:31:51AM -0800, santosh.shilimkar@oracle.com wrote: >> On 2/11/20 7:03 PM, John Hubbard wrote: >>> From: Leon Romanovsky <leonro@mellanox.com> >>> >>> Convert net/rds to use the newly introduces pin_user_pages() API, >>> which properly sets FOLL_PIN. Setting FOLL_PIN is now required for >>> code that requires tracking of pinned pages. >>> >>> Note that this effectively changes the code's behavior: it now >>> ultimately calls set_page_dirty_lock(), instead of set_page_dirty(). >>> This is probably more accurate. >>> >>> As Christoph Hellwig put it, "set_page_dirty() is only safe if we are >>> dealing with a file backed page where we have reference on the inode it >>> hangs off." [1] >>> >>> [1] https://urldefense.com/v3/__https://lore.kernel.org/r/20190723153640.GB720@lst.de__;!!GqivPVa7Brio!OJHuecs9Iup5ig3kQBi_423uMMuskWhBQAdOICrY3UQ_ZfEaxt9ySY7E8y32Q7pk5tByyA$ >>> >>> Cc: Hans Westgaard Ry <hans.westgaard.ry@oracle.com> >>> Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com> >>> Signed-off-by: Leon Romanovsky <leonro@mellanox.com> >>> Signed-off-by: John Hubbard <jhubbard@nvidia.com> >>> --- >> Change looks fine to me. Just on safer side, we will try >> to test this change with regression suite to make sure it >> works as expected. > > Thanks Santosh, > I wrote this patch before John's series was merged into the tree, > but back then, Hans tested it and it worked, hope that it still works. :) > I see. Wasn't aware of it. In that case, its should be fine. Regards, Santosh ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] net/rds: Track user mapped pages through special API 2020-02-12 3:03 ` [PATCH 1/1] " John Hubbard 2020-02-12 17:31 ` santosh.shilimkar @ 2020-02-17 2:37 ` David Miller 1 sibling, 0 replies; 6+ messages in thread From: David Miller @ 2020-02-17 2:37 UTC (permalink / raw) To: jhubbard Cc: akpm, santosh.shilimkar, hans.westgaard.ry, leonro, kuba, netdev, rds-devel, linux-rdma, linux-mm, linux-kernel From: John Hubbard <jhubbard@nvidia.com> Date: Tue, 11 Feb 2020 19:03:55 -0800 > From: Leon Romanovsky <leonro@mellanox.com> > > Convert net/rds to use the newly introduces pin_user_pages() API, > which properly sets FOLL_PIN. Setting FOLL_PIN is now required for > code that requires tracking of pinned pages. > > Note that this effectively changes the code's behavior: it now > ultimately calls set_page_dirty_lock(), instead of set_page_dirty(). > This is probably more accurate. > > As Christoph Hellwig put it, "set_page_dirty() is only safe if we are > dealing with a file backed page where we have reference on the inode it > hangs off." [1] > > [1] https://lore.kernel.org/r/20190723153640.GB720@lst.de > > Cc: Hans Westgaard Ry <hans.westgaard.ry@oracle.com> > Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > Signed-off-by: John Hubbard <jhubbard@nvidia.com> Applied, thank you. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-17 2:37 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-12 3:03 [PATCH 0/1] net/rds: Track user mapped pages through special API John Hubbard 2020-02-12 3:03 ` [PATCH 1/1] " John Hubbard 2020-02-12 17:31 ` santosh.shilimkar 2020-02-12 17:51 ` Leon Romanovsky 2020-02-12 17:55 ` santosh.shilimkar 2020-02-17 2:37 ` David Miller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.