Re: [PATCH rdma-next v6 3/8] RDMA/restrack: Add general infrastructure to track RDMA resources

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]

On Thu, Jan 25, 2018 at 02:57:16PM -0700, Jason Gunthorpe wrote:
> On Thu, Jan 25, 2018 at 11:18:31PM +0200, Leon Romanovsky wrote:
> > On Thu, Jan 25, 2018 at 02:00:26PM -0700, Jason Gunthorpe wrote:
> > > On Thu, Jan 25, 2018 at 10:27:10PM +0200, Leon Romanovsky wrote:
> > > > On Thu, Jan 25, 2018 at 01:10:23PM -0700, Jason Gunthorpe wrote:
> > > > > On Thu, Jan 25, 2018 at 09:44:36PM +0200, Leon Romanovsky wrote:
> > > > > > > > +	/*
> > > > > > > > +	 * @kref: Protect destroy of the resource
> > > > > > > > +	 */
> > > > > > > > +	struct kref		kref;
> > > > > > >
> > > > > > > Sticking a kref in a random structure that is itself not malloc'd is a
> > > > > > > big red flag:
> > > > > >
> > > > > > It is not "rand", but embedded part of ib_qp,ib_pd and ib_cq. It is
> > > > > > malloced as part of their creation.
> > > > >
> > > > > I mean the kref isn't in any way linked the lifetime of the malloc
> > > > > that contains it. So it isn't really a kref, it is just a refcount.
> > > >
> > > > For now, yes, in the future no. IMHO it is the direction to manage RDMA
> > > > objects.
> > >
> > > Maybe but until we do that this doesn't have struct kref semantics at
> > > all and shouldn't be called kref..
> >
> > You are actually open-coded kref semantics with addition of completions.
>
> I generally don't like seeing 'complete()' as a kref release
> function. There have been very subtle problems with code trying that
> in the past..
>
> But yes, you can force things to work that way if very, very careful.
>
> I've often thought I'd like a helper type that had the kref+complete
> semantics, because that does seem to crop up in a few places and it
> would really help clarity.

Actually, it is used a lot, one of closest examples to us - qib sdma
kref counting.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]