On Thu, Apr 6, 2017 at 7:42 PM, Jason Gunthorpe <jgunthorpe@xxxxxxxxxxxxxxxxxxxx> wrote: > On Thu, Apr 06, 2017 at 05:10:51PM +0300, Matan Barak wrote: >> Effectively this kref is used to capture the dependencies between >> various objects. > > No, that is what the usecnt is for, the kref is just to make sure we > can still *access* the usecnt without derefing freed memory. > There are three usecnt(s) actually. One is intended to determine how many readers (or if there's a writer) currently accessing this uobject. Zero here means no one accessing this uobject and we could lock it for either read/write. This is uverbs only. Another one is to protect the lifetime of the uobject. The last usecnt is intended to capture dependencies, meaning when use_cnt > 1 we can't free that object as there are dependent objects. Nowadays, this use_cnt is in the verbs layer and semi-protects (when things are doing serially) kernel layer as well. >> This won't solve the "inc_uobject_kref; destroy_uobject" schema, as >> in this case we really want to destroy the object, but we want to >> keep the uobject alive in order to get information about the >> object's state at destruction. > > Right, this is why I said you need a destory_uobject varient that > retains the kref with the caller. > So currently we have three states in uverbs (when an object is destroyed): (a) uobject is alive, object is alive (b) uobject is alive, object isn't (c) both are destroyed You need to somehow capture the second state. This is used in order to write the updated last information when an object is destroyed (so no new events could be generated on this object). >> If we use kref on the uobject only, we lose the kref count in ULPs. >> Moreover, we'll need to somehow > > No, we can't really do that obviously.. > > I'd rather see the usecnt hoisted entirely into the uverbs layer where > it can work sanely with proper locking and reserve a second > for-debugging-only WARN_ON scheme in the verbs layer that checks > cleanup ordering for the kapi. > > The kapi returning error codes on destroy is insane, I cleaned up PD > at one point, but they all need fixing... > The current solution in the verbs layer is really half baked. It works well as long as you don't try to use an object while destroying it. If you do such a non-sense action, you should do that proper locking and checking yourself. So, if we move that to a uobject, we need three reference counts: (a) read_write_count (b) dependencies_count (or object_refcount) (c) uobject_refcount Anyway, this is really unrelated to the ABI work. In any solution we choose right now, we could move that reference count from objects to uobjects later on. The purpose here isn't doing a cleanup. The locking change was done as the new ABI relies on that new behavior to be dead-lock free, but the refcount change change could be delayed without risking anything. > Jason Matan -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
