Hi Roland, On Mon, 2012-02-27 at 14:39 -0800, Roland Dreier wrote: > On Mon, Feb 27, 2012 at 12:45 PM, Jörn Engel <joern@xxxxxxxxx> wrote: > >> + if (ha->tgt_ops->put_sess(sess) != 0) > >> + return; > >> + spin_lock_irqsave(&ha->hardware_lock, flags); > > > > Can we get rid of the " != 0"? It does not add anything logically an > > to me personally it actually makes the code harder to read. > > I agree and in fact I think I agree with your bigger point -- it seems > almost impossible to use the return value of put_sess() in any safe > way, so we are better off never checking it (and having the method > be void). The comment in kref.h certainly seems to apply: > Thanks for the clarification here. Making tgt_ops->put_sess() return void now.. > * Return 1 if the object was removed, otherwise return 0. Beware, if this > * function returns 0, you still can not count on the kref from remaining in > * memory. Only use the return value if you want to see if the kref is now > * gone, not present. > > so even if we don't drop the last reference in our put call, nothing > prevents another context from dropping the last reference immediately > after our call and blowing things up. > <nod> This is the reason why the extra refcount at creation time is still required AFAICT.. --nab -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
