On Fri, Sep 08, 2023 at 03:26:48PM +0900, Daisuke Matsuda wrote:
> +int rxe_odp_mr_atomic_op(struct rxe_mr *mr, u64 iova, int opcode,
> + u64 compare, u64 swap_add, u64 *orig_val)
> +{
> + int err;
> + int retry = 0;
> + struct ib_umem_odp *umem_odp = to_ib_umem_odp(mr->umem);
> +
> + mutex_lock(&umem_odp->umem_mutex);
> +
> + /* Atomic operations manipulate a single char. */
> + if (rxe_odp_check_pages(mr, iova, sizeof(char), 0))
> + goto need_fault;
> +
> + err = rxe_mr_do_atomic_op(mr, iova, opcode, compare,
> + swap_add, orig_val);
> +
> + mutex_unlock(&umem_odp->umem_mutex);
You should just use the xarray spinlock, the umem_mutex should only be
held around the faulting flow
> +
> + return err;
> +
> +need_fault:
> + /* allow max 3 tries for pagefault */
> + do {
Why a retry loop? We already have a retry loop in
ib_umem_odp_map_dma_and_lock,it doesn't need to be done externally. If
you reach here with the lock held then progress should be guarenteed
under the lock.
Jason
