Commit 46870b23 authored by Jason Gunthorpe's avatar Jason Gunthorpe

RDMA/odp: Remove broken debugging call to invalidate_range

invalidate_range() also obtains the umem_mutex which is being held at this
point, so if this path were was ever called it would deadlock. Thus
conclude the debugging never triggers and rework it into a simple WARN_ON
and leave things as they are.

While here add a note to explain how we could possibly get inconsistent
page pointers.

Link: https://lore.kernel.org/r/20191009160934.3143-16-jgg@ziepe.caSigned-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
parent 09689703
...@@ -508,7 +508,6 @@ static int ib_umem_odp_map_dma_single_page( ...@@ -508,7 +508,6 @@ static int ib_umem_odp_map_dma_single_page(
{ {
struct ib_device *dev = umem_odp->umem.ibdev; struct ib_device *dev = umem_odp->umem.ibdev;
dma_addr_t dma_addr; dma_addr_t dma_addr;
int remove_existing_mapping = 0;
int ret = 0; int ret = 0;
/* /*
...@@ -534,28 +533,29 @@ static int ib_umem_odp_map_dma_single_page( ...@@ -534,28 +533,29 @@ static int ib_umem_odp_map_dma_single_page(
} else if (umem_odp->page_list[page_index] == page) { } else if (umem_odp->page_list[page_index] == page) {
umem_odp->dma_list[page_index] |= access_mask; umem_odp->dma_list[page_index] |= access_mask;
} else { } else {
pr_err("error: got different pages in IB device and from get_user_pages. IB device page: %p, gup page: %p\n", /*
umem_odp->page_list[page_index], page); * This is a race here where we could have done:
/* Better remove the mapping now, to prevent any further *
* damage. */ * CPU0 CPU1
remove_existing_mapping = 1; * get_user_pages()
* invalidate()
* page_fault()
* mutex_lock(umem_mutex)
* page from GUP != page in ODP
*
* It should be prevented by the retry test above as reading
* the seq number should be reliable under the
* umem_mutex. Thus something is really not working right if
* things get here.
*/
WARN(true,
"Got different pages in IB device and from get_user_pages. IB device page: %p, gup page: %p\n",
umem_odp->page_list[page_index], page);
ret = -EAGAIN;
} }
out: out:
put_user_page(page); put_user_page(page);
if (remove_existing_mapping) {
ib_umem_notifier_start_account(umem_odp);
dev->ops.invalidate_range(
umem_odp,
ib_umem_start(umem_odp) +
(page_index << umem_odp->page_shift),
ib_umem_start(umem_odp) +
((page_index + 1) << umem_odp->page_shift));
ib_umem_notifier_end_account(umem_odp);
ret = -EAGAIN;
}
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment