Re: [PATCH for-rc 4/6] RDMA/vmw_pvrdma: Use refcount_dec_and_test to avoid warning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Dec 12, 2017 at 07:20:31PM -0700, Jason Gunthorpe wrote:
> On Tue, Dec 12, 2017 at 04:08:16PM -0800, Bryan Tan wrote:
> > On Tue, Dec 12, 2017 at 11:55:30AM -0700, Jason Gunthorpe wrote:
> 
> > Hm, it's still not clear to me what expectations are not followed here,
> > and how you're differentiating 'refcount' from 'usecnt'.
> 
> A refcount is something that has exactly one 'refcount_dec_and_test'
> that then goes on to 'free' the thing being refcountedt. It never has
> a wait queue.
> 
> A usecnt is something that has an async path that waits for all users
> to stop using it then 'frees' it. It always includes a wait
> queue.

I see, thanks for the explanation.

> > And not that it makes it right, but I did notice that mlx4 uses the
> > same pattern as what I've done here [1].
> 
> Ah, this is good, I was trying to find something like it. Kees
> approves so I have no problem taking your patch.
> 
> Though you might want to reflect on if using a completion is better
> than a wait_event.. Not immediately clear to me.

Oof, thanks for pointing that out. There's a problem here it seems:
0. refcount for a resource is 2 after entering an event handler
1. refcount_dec_and_test gets called by the resource destroy
2. refcount_dec_and_test gets called by the resource event handler
3. resource destroy call checks the condition in wait_event and
   proceeds to free the resource
4. resource event handler calls wake_up() on free'd resource

I'll use a completion here instead.
--
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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux