Re: [PATCH for-next v6 7/7] RDMA/rxe: Add support for the traditional Atomic operations with ODP

[Jason Gunthorpe <jgg@ziepe.ca>]

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