> -----Original Message----- > From: Long Li <longli@xxxxxxxxxxxxx> > Sent: Friday, 7 June 2024 04:45 > To: Konstantin Taranov <kotaranov@xxxxxxxxxxxxx>; Konstantin Taranov > <kotaranov@xxxxxxxxxxxxxxxxxxx>; Wei Hu <weh@xxxxxxxxxxxxx>; > sharmaajay@xxxxxxxxxxxxx; jgg@xxxxxxxx; leon@xxxxxxxxxx > Cc: linux-rdma@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > 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? Sure, it is not a problem if you do not have a bug. The code is subject to change and bugs could appear. > > > 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? Please re-read my explanation again. Also please check the kernel code of other drivers that use wait_for_completion. Many of them do the same three lines as I do in this patch. > > > 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". See your message, you proposed to remove complete as well.
