To: "Krishnamraju Eraparaju" <krishna2@xxxxxxxxxxx>
From: "Jason Gunthorpe" <jgg@xxxxxxxx>
Date: 09/16/2019 06:28PM
Cc: "Steve Wise" <larrystevenwise@xxxxxxxxx>, "Bernard Metzler"
<BMT@xxxxxxxxxxxxxx>, "Sagi Grimberg" <sagi@xxxxxxxxxxx>,
"linux-rdma@xxxxxxxxxxxxxxx" <linux-rdma@xxxxxxxxxxxxxxx>
Subject: [EXTERNAL] Re: Re: [PATCH v3] iwcm: don't hold the irq
disabled lock on iw_rem_ref
On Wed, Sep 11, 2019 at 09:28:16PM +0530, Krishnamraju Eraparaju
wrote:
Hi Steve & Bernard,
Thanks for the review comments.
I will do those formating changes.
I don't see anything in patchworks, but the consensus is to drop
Sagi's patch pending this future patch?
Jason
This is my impression as well. But consensus should be
explicit...Sagi, what do you think?
I don't really care, but given the changes from Krishnamraju cause other
problems I'd ask if my version is also offending his test.
In general, I do not think that making resources free routines (both
explict or implicit via ref dec) under a spinlock is not a robust
design.
I would first make it clear and documented what cm_id_priv->lock is
protecting. In my mind, it should protect *its own* mutations of
cm_id_priv and by design leave all the ops calls outside the lock.
I don't understand what is causing the Chelsio issue observed, but
it looks like c4iw_destroy_qp blocks on a completion that depends on a
refcount that is taken also by iwcm, which means that I cannot call
ib_destroy_qp if the cm is not destroyed as well?
If that is madatory, I'd say that instead of blocking on this
completion, we can simply convert c4iw_qp_rem_ref if use a kref
which is not order dependent.
