search for: kfree_rcu

...below is a faster equivalent of synchronize_rcu() > + */ > + for (i = 0; i < d->nvqs; ++i) { > + mutex_lock(&d->vqs[i]->mutex); > + mutex_unlock(&d->vqs[i]->mutex); > + } > kfree(oldmem); > return 0; > } This looks dubious What about using kfree_rcu() instead ? translate_desc() still uses rcu_read_lock(), its not clear if the mutex is really held.

...below is a faster equivalent of synchronize_rcu() > + */ > + for (i = 0; i < d->nvqs; ++i) { > + mutex_lock(&d->vqs[i]->mutex); > + mutex_unlock(&d->vqs[i]->mutex); > + } > kfree(oldmem); > return 0; > } This looks dubious What about using kfree_rcu() instead ? translate_desc() still uses rcu_read_lock(), its not clear if the mutex is really held.

On Tue, 2014-06-03 at 14:48 +0200, Paolo Bonzini wrote: > Il 02/06/2014 23:58, Eric Dumazet ha scritto: > > This looks dubious > > > > What about using kfree_rcu() instead ? > > It would lead to unbound allocation from userspace. Look at how we did this in commit c3059477fce2d956a0bb3e04357324780c5d8eeb > > > translate_desc() still uses rcu_read_lock(), its not clear if the mutex > > is really held. > > Yes, vhost_get_vq_desc...

On Tue, 2014-06-03 at 14:48 +0200, Paolo Bonzini wrote: > Il 02/06/2014 23:58, Eric Dumazet ha scritto: > > This looks dubious > > > > What about using kfree_rcu() instead ? > > It would lead to unbound allocation from userspace. Look at how we did this in commit c3059477fce2d956a0bb3e04357324780c5d8eeb > > > translate_desc() still uses rcu_read_lock(), its not clear if the mutex > > is really held. > > Yes, vhost_get_vq_desc...

....a17df1f4069a 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -334,7 +334,9 @@ static void vhost_uninit_vq_maps(struct vhost_virtqueue *vq) > } > spin_unlock(&vq->mmu_lock); > > - synchronize_rcu(); > + /* No need for synchronize_rcu() or kfree_rcu() since we are > + * serialized with memory accessors (e.g vq mutex held). > + */ > > for (i = 0; i < VHOST_NUM_ADDRS; i++) > if (map[i]) > -- > 2.18.1 .. however we can not RCU with no synchronization in sight. Sometimes there are hacks like using a lock/unlock p...

...& handler execution is lockless which prevents inverted lock order problems with the drm core. Safe deletion of the event handler is accomplished with synchronize_rcu() for those handlers stored in nouveau_object-based containers (nouveau_drm & nv50_/nvc0_software_context); otherwise, with kfree_rcu (for nouveau_connector & uevent temporary handlers). [1] ====================================================== [ INFO: possible circular locking dependency detected ] 3.10.0-0+tip-xeon+lockdep #0+tip Not tainted ------------------------------------------------------- swapper/7/0 is trying to...

...ase struct nouveau_software_chan same as above > > > RCU lifetime considerations for handlers: > > Event Handler Lifetime > ----- ---------------- --------------------------------- > gpio nouveau_connector_hotplug kfree_rcu(nv_connector) > uevent nouveau_fence_wait_uevent_handler explicit use of nouveau_event_hander_create/_destroy > cevent none n/a > vblank nouveau_drm_vblank_handler synchronize_rcu() in nouveau_drm_unload > vblank nv50_software_vblsem_release...

...ck)); > + map[i] = vq->maps[i]; > if (map[i]) { > vhost_set_map_dirty(vq, map[i], i); > - rcu_assign_pointer(vq->maps[i], NULL); > + vq->maps[i] = NULL; > } > } > spin_unlock(&vq->mmu_lock); > > - /* No need for synchronize_rcu() or kfree_rcu() since we are > - * serialized with memory accessors (e.g vq mutex held). > + /* No need for synchronization since we are serialized with > + * memory accessors (e.g vq mutex held). > */ > > for (i = 0; i < VHOST_NUM_ADDRS; i++) > @@ -362,6 +361,40 @@ static bool v...

...ck)); > + map[i] = vq->maps[i]; > if (map[i]) { > vhost_set_map_dirty(vq, map[i], i); > - rcu_assign_pointer(vq->maps[i], NULL); > + vq->maps[i] = NULL; > } > } > spin_unlock(&vq->mmu_lock); > > - /* No need for synchronize_rcu() or kfree_rcu() since we are > - * serialized with memory accessors (e.g vq mutex held). > + /* No need for synchronization since we are serialized with > + * memory accessors (e.g vq mutex held). > */ > > for (i = 0; i < VHOST_NUM_ADDRS; i++) > @@ -362,6 +361,40 @@ static bool v...

...pa.class, > + MKDEV(MAJOR(vhost_vdpa.devt), v->minor)); > + vhost_vdpa_free_minor(v->minor); > + mutex_unlock(&vhost_vdpa.mutex); > + kfree(v->vqs); > + kfree(v); This use after-fress vs vhost_vdpa_open prior to it setting the open bit. Maybe use xarray, rcu and kfree_rcu .. > +static int __init vhost_vdpa_init(void) > +{ > + int r; > + > + idr_init(&vhost_vdpa.idr); > + mutex_init(&vhost_vdpa.mutex); > + init_waitqueue_head(&vhost_vdpa.release_q); > + > + /* /dev/vhost-vdpa/$vdpa_device_index */ > + vhost_vdpa.class = clas...

...pa.class, > + MKDEV(MAJOR(vhost_vdpa.devt), v->minor)); > + vhost_vdpa_free_minor(v->minor); > + mutex_unlock(&vhost_vdpa.mutex); > + kfree(v->vqs); > + kfree(v); This use after-fress vs vhost_vdpa_open prior to it setting the open bit. Maybe use xarray, rcu and kfree_rcu .. > +static int __init vhost_vdpa_init(void) > +{ > + int r; > + > + idr_init(&vhost_vdpa.idr); > + mutex_init(&vhost_vdpa.mutex); > + init_waitqueue_head(&vhost_vdpa.release_q); > + > + /* /dev/vhost-vdpa/$vdpa_device_index */ > + vhost_vdpa.class = clas...

...vblank nvc0_software_vblsem_release struct nouveau_software_chan same as above RCU lifetime considerations for handlers: Event Handler Lifetime ----- ---------------- --------------------------------- gpio nouveau_connector_hotplug kfree_rcu(nv_connector) uevent nouveau_fence_wait_uevent_handler explicit use of nouveau_event_hander_create/_destroy cevent none n/a vblank nouveau_drm_vblank_handler synchronize_rcu() in nouveau_drm_unload vblank nv50_software_vblsem_release synchronize_rcu() in...

...rivers/vhost/vhost.c index 5b8821d00fe4..a17df1f4069a 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -334,7 +334,9 @@ static void vhost_uninit_vq_maps(struct vhost_virtqueue *vq) } spin_unlock(&vq->mmu_lock); - synchronize_rcu(); + /* No need for synchronize_rcu() or kfree_rcu() since we are + * serialized with memory accessors (e.g vq mutex held). + */ for (i = 0; i < VHOST_NUM_ADDRS; i++) if (map[i]) -- 2.18.1

...rs/gpu/drm/nouveau/core/core/event.c b/drivers/gpu/drm/nouveau/core/core/event.c index ce0a0ef..45bcb37 100644 --- a/drivers/gpu/drm/nouveau/core/core/event.c +++ b/drivers/gpu/drm/nouveau/core/core/event.c @@ -100,18 +100,6 @@ nouveau_event_handler_destroy(struct nouveau_event *event, int index, kfree_rcu(handler, rcu); } -static void -nouveau_event_put_locked(struct nouveau_event *event, int index, - struct nouveau_eventh *handler) -{ - if (__test_and_clear_bit(NVKM_EVENT_ENABLE, &handler->flags)) { - if (!--event->index[index].refs) { - if (event->disable) - event->disa...

Il 02/06/2014 23:58, Eric Dumazet ha scritto: > This looks dubious > > What about using kfree_rcu() instead ? It would lead to unbound allocation from userspace. > translate_desc() still uses rcu_read_lock(), its not clear if the mutex > is really held. Yes, vhost_get_vq_desc must be called with the vq mutex held. The rcu_read_lock/unlock in translate_desc is unnecessary. Paolo

On 06/03/2014 08:48 AM, Paolo Bonzini wrote: > Il 02/06/2014 23:58, Eric Dumazet ha scritto: >> This looks dubious >> >> What about using kfree_rcu() instead ? > > It would lead to unbound allocation from userspace. > >> translate_desc() still uses rcu_read_lock(), its not clear if the mutex >> is really held. > > Yes, vhost_get_vq_desc must be called with the vq mutex held. > > The rcu_read_lock/unlock in...

On Tue, Jun 03, 2014 at 06:57:43AM -0700, Eric Dumazet wrote: > On Tue, 2014-06-03 at 14:48 +0200, Paolo Bonzini wrote: > > Il 02/06/2014 23:58, Eric Dumazet ha scritto: > > > This looks dubious > > > > > > What about using kfree_rcu() instead ? > > > > It would lead to unbound allocation from userspace. > > Look at how we did this in commit > c3059477fce2d956a0bb3e04357324780c5d8eeb > > > > > > translate_desc() still uses rcu_read_lock(), its not clear if the mutex > > > is r...

On 06/03/2014 08:48 AM, Paolo Bonzini wrote: > Il 02/06/2014 23:58, Eric Dumazet ha scritto: >> This looks dubious >> >> What about using kfree_rcu() instead ? > > It would lead to unbound allocation from userspace. > >> translate_desc() still uses rcu_read_lock(), its not clear if the mutex >> is really held. > > Yes, vhost_get_vq_desc must be called with the vq mutex held. > > The rcu_read_lock/unlock in...

..._map_dirty(vq, map[i], i); >>> -??????????? rcu_assign_pointer(vq->maps[i], NULL); >>> +??????????? vq->maps[i] = NULL; >>> ????????? } >>> ????? } >>> ????? spin_unlock(&vq->mmu_lock); >>> ? -??? /* No need for synchronize_rcu() or kfree_rcu() since we are >>> -???? * serialized with memory accessors (e.g vq mutex held). >>> +??? /* No need for synchronization since we are serialized with >>> +???? * memory accessors (e.g vq mutex held). >>> ?????? */ >>> ? ????? for (i = 0; i < VHOST_NUM...

...;> if (map[i]) { >> vhost_set_map_dirty(vq, map[i], i); >> - rcu_assign_pointer(vq->maps[i], NULL); >> + vq->maps[i] = NULL; >> } >> } >> spin_unlock(&vq->mmu_lock); >> >> - /* No need for synchronize_rcu() or kfree_rcu() since we are >> - * serialized with memory accessors (e.g vq mutex held). >> + /* No need for synchronization since we are serialized with >> + * memory accessors (e.g vq mutex held). >> */ >> >> for (i = 0; i < VHOST_NUM_ADDRS; i++) >> @@...