RDMA/odp: Remove broken debugging call to invalidate_range
authorJason Gunthorpe <jgg@mellanox.com>
Wed, 9 Oct 2019 16:09:35 +0000 (13:09 -0300)
committerJason Gunthorpe <jgg@mellanox.com>
Mon, 28 Oct 2019 19:41:14 +0000 (16:41 -0300)
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.ca
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
drivers/infiniband/core/umem_odp.c

index 163ff7ba92b7f136fda5c751357aa813f8d66733..d7d5fadf0899ad1e1bea0c2a94151329fd2a557f 100644 (file)
@@ -508,7 +508,6 @@ static int ib_umem_odp_map_dma_single_page(
 {
        struct ib_device *dev = umem_odp->umem.ibdev;
        dma_addr_t dma_addr;
-       int remove_existing_mapping = 0;
        int ret = 0;
 
        /*
@@ -534,28 +533,29 @@ static int ib_umem_odp_map_dma_single_page(
        } else if (umem_odp->page_list[page_index] == page) {
                umem_odp->dma_list[page_index] |= access_mask;
        } 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);
-               /* Better remove the mapping now, to prevent any further
-                * damage. */
-               remove_existing_mapping = 1;
+               /*
+                * This is a race here where we could have done:
+                *
+                *         CPU0                             CPU1
+                *   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:
        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;
 }