On Thu, Mar 30, 2017 at 1:29 AM, Jason Gunthorpe
<jgunthorpe@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, Mar 29, 2017 at 09:21:09PM +0300, Matan Barak wrote:
>> On Wed, Mar 29, 2017 at 5:53 PM, Jason Gunthorpe
>> <jgunthorpe@xxxxxxxxxxxxxxxxxxxx> wrote:
>> > On Sun, Mar 19, 2017 at 05:59:05PM +0200, Matan Barak wrote:
>> >> +static struct ib_uverbs_completion_event_file *
>> >> +ib_uverbs_lookup_comp_file(int fd, struct ib_ucontext *context)
>> >> +{
>> >> + struct ib_uobject *uobj = uobj_get_read(uobj_get_type(comp_channel),
>> >> + fd, context);
>> >> + struct ib_uobject_file *uobj_file;
>> >> +
>> >> + if (IS_ERR(uobj))
>> >> + return (void *)uobj;
>> >> +
>> >> + uobj_file = container_of(uobj, struct ib_uobject_file, uobj);
>> >> +
>> >> + uverbs_uobject_get(&uobj_file->uobj);
>> >> + uobj_put_read(uobj);
>> >
>> > That looks odd, isn't uobj == uobj_file->uobj ?
>>
>> Yeah, they're essentially the same. The uverbs_uobject_get is intended
>> to increase the reference count
>> on the returned object. The uobj_put_read is paired with uobj_get_read
>> (first line of this function).
>> We could change them both to uobj.
>
> I'm confused why you would ever write
>
> uverbs_uobject_get(uobj);
> uobj_put_read(uobj);
>
> Maybe drop both lines and just add a comment that the ref from
> uobj_get_read moves from the stack to XX
>
The problem with that is that get_read also locks the object for
reading (so if you try to lock it for write or destroy it, you'll get
a locking error).
So, you want to increase the reference count to ensure it exists in the memory,
but unlock it.
> 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
