On Wed, Feb 12, 2014 at 07:06:37PM -0500, David Miller wrote:> From: Greg Kroah-Hartman <gregkh at linuxfoundation.org> > Date: Wed, 12 Feb 2014 08:56:30 -0800 > > > On Wed, Feb 12, 2014 at 06:38:21PM +0200, Michael S. Tsirkin wrote: > >> It is sometimes useful to get the value of the reference count after > >> decrement. > >> For example, vhost wants to execute some periodic cleanup operations > >> once number of references drops below a specific value, before it > >> reaches zero (for efficiency). > > > > You should never care about what the value of the kref is, if you are > > using it correctly :) > > It isn't being used to determine when to destroy things. > > They use it to as a heuristic of when to trigger polling. > > Each ubuf attached gets a kref to the higher level virtio_net buffer > holding object, they want to trigger polling when that reference drops > to 1 or lower. > > Right now they are reading the atomic refcount directly, which > I think is much worse than this helper.Yes, that's horrible as well, but as was already pointed out in this thread, you can't rely on that value to really be "1" after reading it due to the way krefs work, what happened if someone else just grabbed it? If all they want is a "count" for when to start polling, then use a separate atomic count, but don't abuse the kref interface for this, I don't think that will work properly at all. thanks, greg k-h
From: Greg KH <gregkh at linuxfoundation.org> Date: Wed, 12 Feb 2014 17:39:02 -0800> Yes, that's horrible as well, but as was already pointed out in this > thread, you can't rely on that value to really be "1" after reading it > due to the way krefs work, what happened if someone else just grabbed > it? > > If all they want is a "count" for when to start polling, then use a > separate atomic count, but don't abuse the kref interface for this, I > don't think that will work properly at all.They want to know which thread of control decrements the count to "1" as buffers are released. That seems entirely reasonable to me. They could add another atomic counter for this, but that's rather silly since the kref already has an atomic they can use for this purpose.
From: David Miller <davem at davemloft.net> Date: Wed, 12 Feb 2014 23:05:06 -0500 (EST)> From: Greg KH <gregkh at linuxfoundation.org> > Date: Wed, 12 Feb 2014 17:39:02 -0800 > >> Yes, that's horrible as well, but as was already pointed out in this >> thread, you can't rely on that value to really be "1" after reading it >> due to the way krefs work, what happened if someone else just grabbed >> it? >> >> If all they want is a "count" for when to start polling, then use a >> separate atomic count, but don't abuse the kref interface for this, I >> don't think that will work properly at all. > > They want to know which thread of control decrements the count to "1" > as buffers are released. > > That seems entirely reasonable to me. > > They could add another atomic counter for this, but that's rather > silly since the kref already has an atomic they can use for this > purpose.If you still can't understand what they are trying to do, they want to do something precisely when the number of pending buffers is dropped to 1 or less. They are using krefs to track how many buffers are attached at a given moment. The counter can re-increment after the decrement to 1 or less occurs, they don't care. But they want precisely the entity that drops it down to 1 or less to perform that action. Just reading the atomic value directly, they cannot do this.