I'm on windows for the week in Minnesota without access to my Linux system
It will be next weekend before I can get to this.
Bob
-----Original Message-----
From: Jason Gunthorpe <jgg@xxxxxxxxxx>
Sent: Monday, June 6, 2022 12:10 PM
To: Bob Pearson <rpearsonhpe@xxxxxxxxx>
Cc: zyjzyj2000@xxxxxxxxx; bvanassche@xxxxxxx; linux-rdma@xxxxxxxxxxxxxxx; Zago, Frank <frank.zago@xxxxxxx>; Hack, Jenny (Ft. Collins) <jhack@xxxxxxx>
Subject: Re: [PATCH for-next v15 1/2] RDMA/rxe: Stop lookup of partially built objects
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
Sure.
> + } 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?
One of the tasklets takes a reference on the AH when building a packet.
It mostly seems to not hold it for long so the answer is likely no but I'm not 100% sure.
> +/**
> + * 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?
Nope. That fragment got lost and here it is.
Jason
