Re: [PATCH for-next V5 1/5] IB/uverbs: Fix reference counting usage of event files

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Tue, Jun 30, 2015 at 12:22:02AM +0300, Yishai Hadas wrote:
> It should be:
> kref_put(&uverbs_file->async_file->ref, ib_uverbs_release_event_file)
> instead of:
> kref_put(&uverbs_file->async_file->ref, ib_uverbs_release_file);

Right

> Please note that in that approach we duplicate above line as it appears here
> and in below err_put_refs label as uverbs_file->async_file is really
> ev_file.

Well, sort of. Yes, it is the same value, but no, it isn't the same
kref.

Just like locking always works on data, not code, krefs should always
be applied to the pointer not to the pointed data. The kref
manipulation should always be near the pointer value change it is
working for, and have a clear relation to the pointer it is krefing.

> In my patch we used one line instead of those duplicated lines, however as
> you think that it clarifies things will go with your suggestion after fixing
> above note, thanks.

You shouldn't think of them as duplicates, they are for different
things, even if the actual functionality is the same. It is
*documentation*

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html