-----Original Message-----
From: Jason Gunthorpe <jgg@xxxxxxxxxx>
Sent: Monday, June 6, 2022 12:07 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 2/2] RDMA/rxe: Convert read side locking to rcu
On Mon, Jun 06, 2022 at 09:18:04AM -0500, Bob Pearson wrote:
> Use rcu_read_lock() for protecting read side operations in rxe_pool.c.
>
> Signed-off-by: Bob Pearson <rpearsonhpe@xxxxxxxxx>
> drivers/infiniband/sw/rxe/rxe_pool.c | 26 ++++++++++----------------
> 1 file changed, 10 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c
> b/drivers/infiniband/sw/rxe/rxe_pool.c
> index 7a5685f0713e..103bf0c03441 100644
> +++ b/drivers/infiniband/sw/rxe/rxe_pool.c
> @@ -195,16 +195,15 @@ void *rxe_pool_get_index(struct rxe_pool *pool,
> u32 index) {
> struct rxe_pool_elem *elem;
> struct xarray *xa = &pool->xa;
> - unsigned long flags;
> void *obj;
>
> - xa_lock_irqsave(xa, flags);
> + rcu_read_lock();
> elem = xa_load(xa, index);
> if (elem && kref_get_unless_zero(&elem->ref_cnt))
> obj = elem->obj;
> else
> obj = NULL;
> - xa_unlock_irqrestore(xa, flags);
> + rcu_read_unlock();
>
> return obj;
> }
> @@ -221,16 +220,15 @@ int __rxe_cleanup(struct rxe_pool_elem *elem, bool sleepable)
> struct rxe_pool *pool = elem->pool;
> struct xarray *xa = &pool->xa;
> static int timeout = RXE_POOL_TIMEOUT;
> - unsigned long flags;
> int ret, err = 0;
> void *xa_ret;
>
> + WARN_ON(!in_task());
I think this should just be
if (sleepable)
might_sleep();
?
It all seems OK, any chance the AH bit can be split to its own patch or is it all interconnected? It would be easier for rc this way.
I can't not do anything about AH because it throws kernel splats. I'd rather not break the driver
even for one patch. If one says this won't work unless you apply all of them you may as well just have
one patch.
Bob
Jason
