On 7/16/25 00:46, Jason Gunthorpe wrote:
On Sun, Jul 13, 2025 at 09:27:53AM +0300, Leon Romanovsky wrote:
Let's do what all other drivers do, please. I prefer simplest solution
and objects that can potentially be around after verbs objects were
cleaned doesn't sound right.
I think it is OK, at least QP makes sense and matches some other
drivers.
+static void ionic_qp_event(struct ionic_ibdev *dev, u32 qpid, u8 code)
+{
+ struct ib_event ibev;
+ struct ionic_qp *qp;
+
+ rcu_read_lock();
+ qp = xa_load(&dev->qp_tbl, qpid);
+ if (qp)
+ kref_get(&qp->qp_kref);
+ rcu_read_unlock();
+
The above is an async event path, and the kref is effectively the open
coded rwlock pattern we use often.
The unlock triggers a completion:
+ kref_put(&qp->qp_kref, ionic_qp_complete);
+static inline void ionic_qp_complete(struct kref *kref)
+{
+ struct ionic_qp *qp = container_of(kref, struct ionic_qp, qp_kref);
+
+ complete(&qp->qp_rel_comp);
+}
Which acts as the unlock. And then qp destruction:
+int ionic_destroy_qp(struct ib_qp *ibqp, struct ib_udata *udata)
+{
+ kref_put(&qp->qp_kref, ionic_qp_complete);
+ wait_for_completion(&qp->qp_rel_comp);
Which is the typical "write" side of the lock.
So this is all normal, the qp doesn't outlive destroy, destroy waits
for all the async event deliver to complete. It has to, we free the
underlying memory in the core code.
As long as the other case are like this it is fine
+ xa_erase_irq(&dev->qp_tbl, qp->qpid);
+ synchronize_rcu();
This should go away though, don't like to see synchronize_rcu(). The
idea is you kfree the QP with RCU. But the core code doesn't do that..
So in the short term you should take the lock instead of using rcu:
xa_lock(&dev->qp_tbl);
qp = xa_load(&dev->qp_tbl, qpid);
if (qp)
kref_get(&qp->qp_kref);
Jason
Thank you, Jason, for reviewing the logic and explaining how the
kref/RCU mechanism effectively ensures correct synchronization and clean
tear down during async event handling and QP destruction. A similar
mechanism is currently used for CQ event handling and destruction as well.
Your suggestion to avoid synchronize_rcu() and instead take the lock
directly for xarray lookups makes sense. I will proceed to replace the
RCU critical section with xa_lock()/xa_load(), as you outlined, to
better align with current best practices—unless there are any objections.
Thanks again for the valuable feedback!
Abhijit
