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:
signature.asc
Description: PGP signature
