search for: kref

...or efficiency). Add an API to do this atomically and efficiently using atomic_sub_return. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- Greg, could you ack this API extension please? I think it is cleanest to merge this through -net together with the first user. include/linux/kref.h | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/include/linux/kref.h b/include/linux/kref.h index 484604d..cb20550 100644 --- a/include/linux/kref.h +++ b/include/linux/kref.h @@ -61,7 +61,7 @@ static inline void kref_get(struct kref *kref)...

This fixes a deadlock with vhost reported in the field, as well as a theoretical race issue found by code review. Patches 1+2 are needed for stable. Thanks to Qin Chuanyu for reporting the issue! Michael S. Tsirkin (3): kref: add kref_sub_return vhost: fix ref cnt checking deadlock vhost: fix a theoretical race in device cleanup include/linux/kref.h | 33 ++++++++++++++++++++++++++++++++- drivers/vhost/net.c | 15 ++++++++++----- 2 files changed, 42 insertions(+), 6 deletions(-) -- MST

This fixes a deadlock with vhost reported in the field, as well as a theoretical race issue found by code review. Patches 1+2 are needed for stable. Thanks to Qin Chuanyu for reporting the issue! Michael S. Tsirkin (3): kref: add kref_sub_return vhost: fix ref cnt checking deadlock vhost: fix a theoretical race in device cleanup include/linux/kref.h | 33 ++++++++++++++++++++++++++++++++- drivers/vhost/net.c | 15 ++++++++++----- 2 files changed, 42 insertions(+), 6 deletions(-) -- MST

...rom: 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....

...rom: 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....

...a race: all users might see a high count and decide to defer freeing resources. In the end no one initiates freeing resources until the last reference is gone (which is on VM shotdown so might happen after a looooong time). Let's do what we probably should have done straight away: switch from kref to plain atomic, documenting the semantics, return the refcount value atomically after decrement, then use that to avoid the deadlock. Reported-by: Qin Chuanyu <qinchuanyu at huawei.com> Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- This patch is needed for 3.14 and -stabl...

...a race: all users might see a high count and decide to defer freeing resources. In the end no one initiates freeing resources until the last reference is gone (which is on VM shotdown so might happen after a looooong time). Let's do what we probably should have done straight away: switch from kref to plain atomic, documenting the semantics, return the refcount value atomically after decrement, then use that to avoid the deadlock. Reported-by: Qin Chuanyu <qinchuanyu at huawei.com> Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- This patch is needed for 3.14 and -stabl...

...seful 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 :) So I really don't want to add this function, as I'm sure people will use it incorrectly. You should only care if the reference drops to 0, if not, then your usage doesn't really fit into the "kref" model, and so, just use an atomic variab...

...seful 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 :) So I really don't want to add this function, as I'm sure people will use it incorrectly. You should only care if the reference drops to 0, if not, then your usage doesn't really fit into the "kref" model, and so, just use an atomic variab...

...of my understanding, the current scheme works like this: drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When this refcount hits 0 for either of the two, they're removed from the topology state, but not immediately freed. Both ports and branch devices will reinitialize their kref once it's hit 0 before actually destroying themselves. The intended purpose behind this is so that we can avoid problems like not being able to free a remote payload that might still be active, due to us having removed all of the port/branch device structures in memory, as per: commit 91a25e46...

...>> 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...

...>> 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...

...t 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 w...

...of my understanding, the current scheme works like this: drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When this refcount hits 0 for either of the two, they're removed from the topology state, but not immediately freed. Both ports and branch devices will reinitialize their kref once it's hit 0 before actually destroying themselves. The intended purpose behind this is so that we can avoid problems like not being able to free a remote payload that might still be active, due to us having removed all of the port/branch device structures in memory, as per: commit 91a25e46...

..., the current scheme works like this: > drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When > this refcount hits 0 for either of the two, they're removed from the > topology state, but not immediately freed. Both ports and branch devices > will reinitialize their kref once it's hit 0 before actually destroying > themselves. The intended purpose behind this is so that we can avoid > problems like not being able to free a remote payload that might still > be active, due to us having removed all of the port/branch device > structures in memory, as p...

...of my understanding, the current scheme works like this: drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When this refcount hits 0 for either of the two, they're removed from the topology state, but not immediately freed. Both ports and branch devices will reinitialize their kref once it's hit 0 before actually destroying themselves. The intended purpose behind this is so that we can avoid problems like not being able to free a remote payload that might still be active, due to us having removed all of the port/branch device structures in memory, as per: 91a25e463130 (&...

...t; > > drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When > > > this refcount hits 0 for either of the two, they're removed from the > > > topology state, but not immediately freed. Both ports and branch devices > > > will reinitialize their kref once it's hit 0 before actually destroying > > > themselves. The intended purpose behind this is so that we can avoid > > > problems like not being able to free a remote payload that might still > > > be active, due to us having removed all of the port/branch device &...

...works like this: > > drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When > > this refcount hits 0 for either of the two, they're removed from the > > topology state, but not immediately freed. Both ports and branch devices > > will reinitialize their kref once it's hit 0 before actually destroying > > themselves. The intended purpose behind this is so that we can avoid > > problems like not being able to free a remote payload that might still > > be active, due to us having removed all of the port/branch device > > struct...

...b/drivers/gpu/drm/drm_gpuvm.c index 53e2c406fb04..ef968eba6fe6 100644 --- a/drivers/gpu/drm/drm_gpuvm.c +++ b/drivers/gpu/drm/drm_gpuvm.c @@ -746,6 +746,8 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm, const char *name, gpuvm->rb.tree = RB_ROOT_CACHED; INIT_LIST_HEAD(&gpuvm->rb.list); + kref_init(&gpuvm->kref); + gpuvm->name = name ? name : "unknown"; gpuvm->flags = flags; gpuvm->ops = ops; @@ -770,15 +772,8 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm, const char *name, } EXPORT_SYMBOL_GPL(drm_gpuvm_init); -/** - * drm_gpuvm_destroy() - cleanup a &...

...changed, 142 insertions(+), 101 deletions(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 683d9a1..c194045 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -70,8 +70,24 @@ enum vhost_net_poll_state { VHOST_NET_POLL_STOPPED = 2, }; +struct vhost_ubuf_ref { + struct kref kref; + wait_queue_head_t wait; + struct vhost_virtqueue *vq; +}; + struct vhost_net_virtqueue { struct vhost_virtqueue vq; + /* vhost zerocopy support fields below: */ + /* last used idx for outstanding DMA zerocopy buffers */ + int upend_idx; + /* first used idx for DMA done zerocopy buffers *...