On Mon, Oct 08, 2018 at 09:06:43AM +0300, Leon Romanovsky wrote: > On Sun, Oct 07, 2018 at 10:23:06AM +0300, Shamir Rabinovitch wrote: > > uobject pointers should not be used in ib core layer. they only > > belong to ib uverbs. as result, resource tracker should not use > > them as well. > > I can't admit that I agree with this claim and the change below. > "uobject" is part of IB/core structures and has visibility and access to > all such members. I have no issue to change this claim to "having uobject pointer embedded in ib core objects is not aligned with future shared ib_x model. Future patches will change this so resource tracker must not use this pointer as well" > > In addition, we had the following assumption for restrack: > 1. "Reuse already existing information" - this is why we had res_is_user() > 2. "Don't add fields without need to restrack struct" - it is created > for every object and increases memory footprint. This information will no longer be available from the uobject pointer. It can be added to each and every ib_x object but it will not save memory nor make the code more simple. > 3, "Avoid direct access to fields to restrack" - no to "cq->res.user = false" Sure. We can wrap this as I did with inline. > 4. "Minimize human errors" - this is why annotating every caller to > rdma_restrack_add() is prone to errors. For example, you didn't change cma.c. I'd suggest that every resource will start as user/kernel=uninitialized and if 'res_is_user' is called and resource is in that state we could at least show good error messages & assume resource is from kernel thus avoiding issues where the code try to access ucontext that is not there. > > Thanks > Thanks
