On Thu, 2014-02-06 at 18:56 -0800, Andy Grover wrote: > On 02/06/2014 03:51 PM, Nicholas A. Bellinger wrote: > > The problem with this patch and all of the other patches that follow the > > same logic is the false assumption that it's safe to return from > > configfs_group_operations->drop_item() before all references to the > > underlying data structure containing the associated struct config_group > > have been dropped. > > > > In this particular case, target_core_alua_drop_lu_gp() -> > > config_item_put() -> target_core_alua_lu_gp_release() -> > > core_alua_free_lu_gp() is still being called from ->drop_item() via > > target_core_alua_lu_gp_ops->release(), so the same holds true here as > > well. > > Yes exactly. That's why the configfs release() doesn't free the lu_gp, > it just lowers the lu_gp refcount. release() being called doesn't mean > the object is going away, it just means configfs is done with it. > > If do_port_transition has incremented it in the meantime, the lu_gp > won't be freed from the release() (because the underlying object's > refcount will still be nonzero) but only when do_port_transition is > done. In the normal case where there isn't a ref from > do_port_transition, then it is safe to free the lu_gp from release -> > put_alua_lu_gp -> release_alua_lu_gp -> kmem_cache_free. It's still completely wrong to return from ->drop_item() before all references to the associated struct config_group have dropped because nothing prevents a parent struct config_group (and it's parent above that) from also being removed. The whole point is that this type of parent/child reference counting comes for free with configfs, and patches that introduce changes that don't actually wait for individual references to drop, but instead lazily allow references to drop in the background at their leisure break this model. --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
