Re: [PATCH for-next v15 1/2] RDMA/rxe: Stop lookup of partially built objects

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Jun 06, 2022 at 09:18:03AM -0500, Bob Pearson wrote:
> @@ -164,9 +171,16 @@ int __rxe_add_to_pool(struct rxe_pool *pool, struct rxe_pool_elem *elem)
>  	elem->pool = pool;
>  	elem->obj = (u8 *)elem - pool->elem_offset;
>  	kref_init(&elem->ref_cnt);
> +	init_completion(&elem->complete);
>  
> -	err = xa_alloc_cyclic(&pool->xa, &elem->index, elem, pool->limit,
> -			      &pool->next, GFP_KERNEL);
> +	/* AH objects are unique in that the create_ah verb
> +	 * can be called in atomic context. If the create_ah
> +	 * call is not sleepable use GFP_ATOMIC.
> +	 */
> +	gfp_flags = sleepable ? GFP_KERNEL : GFP_ATOMIC;

I would add a 'if (sleepable) might_sleep()' here too

> +	} else {
> +		unsigned long until = jiffies + timeout;
> +
> +		/* AH objects are unique in that the destroy_ah verb
> +		 * can be called in atomic context. This delay
> +		 * replaces the wait_for_completion call above
> +		 * when the destroy_ah call is not sleepable
> +		 */
> +		while (!completion_done(&elem->complete) &&
> +				time_before(jiffies, until))
> +			mdelay(1);

Is it even possible that a transient AH reference can exist?

> +/**
> + * rxe_wqe_is_fenced - check if next wqe is fenced
> + * @qp: the queue pair
> + * @wqe: the next wqe
> + *
> + * Returns: 1 if wqe needs to wait
> + *	    0 if wqe is ready to go
> + */
> +static int rxe_wqe_is_fenced(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
> +{
> +	/* Local invalidate fence (LIF) see IBA 10.6.5.1
> +	 * Requires ALL previous operations on the send queue
> +	 * are complete. Make mandatory for the rxe driver.
> +	 */
> +	if (wqe->wr.opcode == IB_WR_LOCAL_INV)
> +		return qp->req.wqe_index != queue_get_consumer(qp->sq.queue,
> +						QUEUE_TYPE_FROM_CLIENT);
> +
> +	/* Fence see IBA 10.8.3.3
> +	 * Requires that all previous read and atomic operations
> +	 * are complete.
> +	 */
> +	return (wqe->wr.send_flags & IB_SEND_FENCE) &&
> +		atomic_read(&qp->req.rd_atomic) != qp->attr.max_rd_atomic;
> +}

Does this belong in this patch?

Jason