> > 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".
