[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<PH7PR21MB30716CC511AAC603DF1FEDE2CEFB2@PH7PR21MB3071.namprd21.prod.outlook.com>
Date: Fri, 7 Jun 2024 02:45:19 +0000
From: Long Li <longli@...rosoft.com>
To: Konstantin Taranov <kotaranov@...rosoft.com>, Konstantin Taranov
<kotaranov@...ux.microsoft.com>, Wei Hu <weh@...rosoft.com>,
"sharmaajay@...rosoft.com" <sharmaajay@...rosoft.com>, "jgg@...pe.ca"
<jgg@...pe.ca>, "leon@...nel.org" <leon@...nel.org>
CC: "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH rdma-next 1/1] RDMA/mana_ib: process QP error events
> > Strange logic. Why not do:
> > if (!refcount_dec_and_test(&qp->refcount))
> > wait_for_completion(&qp->free);
> >
>
> It might work, but the logic will be even stranger and it will prevent some
> debugging.
> With the proposed change, qp->free may not be completed even though the
> counter is 0.
Why this is a problem? mana_ib_destroy_rc_qp() is the only one waiting on it?
> As a result, the change makes an incorrect state to be an expected state, thereby
> making bugs with that side effect undetectable.
> E.g., we have a bug "use after free" and then we try to trace whether qp was in
> use.
I don't get it. Can you explain why?
> Plus, it is a good practice deinit everything that was inited. With the proposed
> change it is violated.
You shouldn't call wait_for_completion if it's not needed. This is not a "deinit".
Powered by blists - more mailing lists