On Mon, Oct 08, 2018 at 10:28:53AM +0300, Shamir Rabinovitch wrote: > 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" thanks, it is much better. > > > > > 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. > I'm not arguing with the need and my suggestion is to introduce two new wrappers, something like this: rdma_restrack_uadd for user's objects and rdma_restrack_kadd for kernel's objects. Thanks > > > > Thanks > > > > Thanks
Attachment:
signature.asc
Description: PGP signature
