On 2/26/19 5:10 PM, Ira Weiny wrote:
> John,
>
> With your put_user_page patchset, Is there a reason you don't call
> put_user_page() in the 2 spots shown in the diff below?
>
> And Jason, in the second case I find it odd that the driver needs to clear the
> invalidate_range() callback prior to the final ib_umem_odp_unmap_dma_pages()
> call such that put_user_page() is called. I am correct that this is how the
> pages finally get put in the end?
>
> Ira
>
> diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c
> index 730b2fa0942e..a63f5eda02ca 100644
> --- a/drivers/infiniband/core/umem_odp.c
> +++ b/drivers/infiniband/core/umem_odp.c
> @@ -687,7 +687,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem_odp *umem_odp, u64 user_virt,
> if (ret < 0) {
> /* Release left over pages when handling errors. */
> for (++j; j < npages; ++j)
> - put_page(local_page_list[j]);
> + put_user_page(local_page_list[j]);
Hi Ira,
This one I somehow just overlooked. It definitely should have been a put_user_page()
call.
> break;
> }
> }
> @@ -750,7 +750,7 @@ void ib_umem_odp_unmap_dma_pages(struct ib_umem_odp *umem_odp, u64 virt,
> }
> /* on demand pinning support */
> if (!umem->context->invalidate_range)
> - put_page(page);
> + put_user_page(page);
...and this one, I didn't recognize as being part of the pool of pages that
were acquired via get_user_pages(). But assuming that it is acquired via
get_user_pages(), then of course it also should be changed to put_user_page(),
you're right.
Glad you're taking a hard look at this!
thanks,
--
John Hubbard
NVIDIA
