Skip to content

Commit 46870b2

Browse files
committed
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/[email protected] Signed-off-by: Jason Gunthorpe <[email protected]>
1 parent 0968970 commit 46870b2

File tree

1 file changed

+19
-19
lines changed

1 file changed

+19
-19
lines changed

drivers/infiniband/core/umem_odp.c

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,6 @@ static int ib_umem_odp_map_dma_single_page(
508508
{
509509
struct ib_device *dev = umem_odp->umem.ibdev;
510510
dma_addr_t dma_addr;
511-
int remove_existing_mapping = 0;
512511
int ret = 0;
513512

514513
/*
@@ -534,28 +533,29 @@ static int ib_umem_odp_map_dma_single_page(
534533
} else if (umem_odp->page_list[page_index] == page) {
535534
umem_odp->dma_list[page_index] |= access_mask;
536535
} else {
537-
pr_err("error: got different pages in IB device and from get_user_pages. IB device page: %p, gup page: %p\n",
538-
umem_odp->page_list[page_index], page);
539-
/* Better remove the mapping now, to prevent any further
540-
* damage. */
541-
remove_existing_mapping = 1;
536+
/*
537+
* This is a race here where we could have done:
538+
*
539+
* CPU0 CPU1
540+
* get_user_pages()
541+
* invalidate()
542+
* page_fault()
543+
* mutex_lock(umem_mutex)
544+
* page from GUP != page in ODP
545+
*
546+
* It should be prevented by the retry test above as reading
547+
* the seq number should be reliable under the
548+
* umem_mutex. Thus something is really not working right if
549+
* things get here.
550+
*/
551+
WARN(true,
552+
"Got different pages in IB device and from get_user_pages. IB device page: %p, gup page: %p\n",
553+
umem_odp->page_list[page_index], page);
554+
ret = -EAGAIN;
542555
}
543556

544557
out:
545558
put_user_page(page);
546-
547-
if (remove_existing_mapping) {
548-
ib_umem_notifier_start_account(umem_odp);
549-
dev->ops.invalidate_range(
550-
umem_odp,
551-
ib_umem_start(umem_odp) +
552-
(page_index << umem_odp->page_shift),
553-
ib_umem_start(umem_odp) +
554-
((page_index + 1) << umem_odp->page_shift));
555-
ib_umem_notifier_end_account(umem_odp);
556-
ret = -EAGAIN;
557-
}
558-
559559
return ret;
560560
}
561561

0 commit comments

Comments
 (0)