search for: cancel_work_sync

On Thu, Nov 20, 2014 at 06:47:11PM +0200, Michael S. Tsirkin wrote: > > There's cancel_work_sync() to stop the self-requeueing ones. > > What happens if queue_work runs while cancel_work_sync is in progress? > Does it fail to queue? cancel_work_sync() is guaranteed to take self-requeueing work items no matter when it's called or what's going on. External (non-self) queueing...

On Thu, Nov 20, 2014 at 06:47:11PM +0200, Michael S. Tsirkin wrote: > > There's cancel_work_sync() to stop the self-requeueing ones. > > What happens if queue_work runs while cancel_work_sync is in progress? > Does it fail to queue? cancel_work_sync() is guaranteed to take self-requeueing work items no matter when it's called or what's going on. External (non-self) queueing...

...; > > > remove_common(vb); > > > > kfree(vb); > > > > } > > > > > > Shouldn't the work item be flushed before removal is complete? > > > > In fact, flushing it won't help because it can requeue itself, right? There's cancel_work_sync() to stop the self-requeueing ones. > From that POV a dedicated WQ kept it simple. A dedicated wq doesn't do anything for that. You can't shut down a workqueue with a pending work item on it. destroy_workqueue() will try to drain the target wq, warn if it doesn't finish in certai...

...; > > > remove_common(vb); > > > > kfree(vb); > > > > } > > > > > > Shouldn't the work item be flushed before removal is complete? > > > > In fact, flushing it won't help because it can requeue itself, right? There's cancel_work_sync() to stop the self-requeueing ones. > From that POV a dedicated WQ kept it simple. A dedicated wq doesn't do anything for that. You can't shut down a workqueue with a pending work item on it. destroy_workqueue() will try to drain the target wq, warn if it doesn't finish in certai...

...y not? > > > @@ -563,7 +534,7 @@ static void virtballoon_remove(struct virtio_device *vdev) > > > struct virtio_balloon *vb = vdev->priv; > > > > > > unregister_oom_notifier(&vb->nb); > > > - kthread_stop(vb->thread); > > > + cancel_work_sync(&vb->wq_work); > > > > OK but since job requeues itself, cancelling like this might not be enough. > > As long as there's no further external queueing, cancel_work_sync() is > guaranteed to kill a self-requeueing work item. > > Thanks. I didn't realise...

...y not? > > > @@ -563,7 +534,7 @@ static void virtballoon_remove(struct virtio_device *vdev) > > > struct virtio_balloon *vb = vdev->priv; > > > > > > unregister_oom_notifier(&vb->nb); > > > - kthread_stop(vb->thread); > > > + cancel_work_sync(&vb->wq_work); > > > > OK but since job requeues itself, cancelling like this might not be enough. > > As long as there's no further external queueing, cancel_work_sync() is > guaranteed to kill a self-requeueing work item. > > Thanks. I didn't realise...

...p;portdev->config_work, &config_work_handler); INIT_WORK(&portdev->control_work, &control_work_handler); if (multiport) { @@ -2114,6 +2127,8 @@ static void virtcons_remove(struct virtio_device *vdev) /* Finish up work that's lined up */ if (use_multiport(portdev)) cancel_work_sync(&portdev->control_work); + else + cancel_work_sync(&portdev->config_work); list_for_each_entry_safe(port, port2, &portdev->ports, list) unplug_port(port); @@ -2165,6 +2180,7 @@ static int virtcons_freeze(struct virtio_device *vdev) virtqueue_disable_cb(portdev->c...

...p;portdev->config_work, &config_work_handler); INIT_WORK(&portdev->control_work, &control_work_handler); if (multiport) { @@ -2114,6 +2127,8 @@ static void virtcons_remove(struct virtio_device *vdev) /* Finish up work that's lined up */ if (use_multiport(portdev)) cancel_work_sync(&portdev->control_work); + else + cancel_work_sync(&portdev->config_work); list_for_each_entry_safe(port, port2, &portdev->ports, list) unplug_port(port); @@ -2165,6 +2180,7 @@ static int virtcons_freeze(struct virtio_device *vdev) virtqueue_disable_cb(portdev->c...

...rs/gpu/drm/nouveau/dispnv04/disp.c > @@ -68,7 +68,7 @@ nv04_display_fini(struct drm_device *dev, bool runtime, bool suspend) > if (nv_two_heads(dev)) > NVWriteCRTC(dev, 1, NV_PCRTC_INTR_EN_0, 0); > > - if (!runtime) > + if (!runtime && !drm->headless) > cancel_work_sync(&drm->hpd_work); > > if (!suspend) > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c > index 88728a0b2c25..674dc567e179 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp....

...rnel/x86/built-in.o CC [M] /usr/local/kvm-88/kvm/kernel/x86/svm.o In file included from /usr/local/kvm-88/kvm/kernel/x86/external-module-compat.h:16, from <command line>:1: /usr/local/kvm-88/kvm/kernel/x86/../external-module-compat-comm.h:551: error: static declaration of ?cancel_work_sync? follows non-static declaration include/linux/workqueue.h:117: error: previous declaration of cancel_work_sync was here In file included from /usr/local/kvm-88/kvm/kernel/x86/external-module-compat.h:16, from <command line>:1: /usr/local/kvm-88/kvm/kernel/x86/../external-modu...

...; > > > > } > > > > > > > > > > Shouldn't the work item be flushed before removal is complete? > > Great catch! > > > > > In fact, flushing it won't help because it can requeue itself, right? > > > > There's cancel_work_sync() to stop the self-requeueing ones. > > Ah, one more problem is that remove_common(vb) calls leak_balloon() > that queues the work if not finished. We would need to add some flag > or variant that would disable the queuing when called here. > That's why Tejun suggested cancel_...

...; > > > > } > > > > > > > > > > Shouldn't the work item be flushed before removal is complete? > > Great catch! > > > > > In fact, flushing it won't help because it can requeue itself, right? > > > > There's cancel_work_sync() to stop the self-requeueing ones. > > Ah, one more problem is that remove_common(vb) calls leak_balloon() > that queues the work if not finished. We would need to add some flag > or variant that would disable the queuing when called here. > That's why Tejun suggested cancel_...

...otify: > vdev->config->del_vqs(vdev); > out_free_vb: > @@ -563,7 +534,7 @@ static void virtballoon_remove(struct virtio_device *vdev) > struct virtio_balloon *vb = vdev->priv; > > unregister_oom_notifier(&vb->nb); > - kthread_stop(vb->thread); > + cancel_work_sync(&vb->wq_work); OK but since job requeues itself, cancelling like this might not be enough. > remove_common(vb); > kfree(vb); > } > @@ -574,10 +545,9 @@ static int virtballoon_freeze(struct virtio_device *vdev) > struct virtio_balloon *vb = vdev->priv; > >...

...otify: > vdev->config->del_vqs(vdev); > out_free_vb: > @@ -563,7 +534,7 @@ static void virtballoon_remove(struct virtio_device *vdev) > struct virtio_balloon *vb = vdev->priv; > > unregister_oom_notifier(&vb->nb); > - kthread_stop(vb->thread); > + cancel_work_sync(&vb->wq_work); OK but since job requeues itself, cancelling like this might not be enough. > remove_common(vb); > kfree(vb); > } > @@ -574,10 +545,9 @@ static int virtballoon_freeze(struct virtio_device *vdev) > struct virtio_balloon *vb = vdev->priv; > >...

...gt;] flush_work+0x0/0x1d7 62031c18: [<60050234>] try_to_grab_pending+0x0/0x17e 62031c38: [<6004e984>] get_work_gcwq+0x71/0x8f 62031c48: [<60050539>] __cancel_work_timer+0x5b/0x115 62031c78: [<628acc85>] unplug_port+0x0/0x191 [virtio_console] 62031c98: [<6005061c>] cancel_work_sync+0x12/0x14 62031ca8: [<628ace96>] virtcons_remove+0x80/0x15c [virtio_console] 62031ce8: [<628191de>] virtio_dev_remove+0x1e/0x7e [virtio] 62031d08: [<601cf242>] __device_release_driver+0x75/0xe4 62031d28: [<601cf2dd>] device_release_driver+0x2c/0x40 62031d48: [<601ce0...

...gt;] flush_work+0x0/0x1d7 62031c18: [<60050234>] try_to_grab_pending+0x0/0x17e 62031c38: [<6004e984>] get_work_gcwq+0x71/0x8f 62031c48: [<60050539>] __cancel_work_timer+0x5b/0x115 62031c78: [<628acc85>] unplug_port+0x0/0x191 [virtio_console] 62031c98: [<6005061c>] cancel_work_sync+0x12/0x14 62031ca8: [<628ace96>] virtcons_remove+0x80/0x15c [virtio_console] 62031ce8: [<628191de>] virtio_dev_remove+0x1e/0x7e [virtio] 62031d08: [<601cf242>] __device_release_driver+0x75/0xe4 62031d28: [<601cf2dd>] device_release_driver+0x2c/0x40 62031d48: [<601ce0...

...rs/gpu/drm/nouveau/dispnv04/disp.c > @@ -68,7 +68,7 @@ nv04_display_fini(struct drm_device *dev, bool runtime, bool suspend) > if (nv_two_heads(dev)) > NVWriteCRTC(dev, 1, NV_PCRTC_INTR_EN_0, 0); > > - if (!runtime) > + if (!runtime && !drm->headless) > cancel_work_sync(&drm->hpd_work); > > if (!suspend) > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c > index 88728a0b2c25..674dc567e179 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp....

...vi = container_of(napi, struct virtnet_info, napi); @@ -787,6 +797,8 @@ static int virtnet_close(struct net_device *dev) /* Make sure refill_work doesn't re-enable napi! */ cancel_delayed_work_sync(&vi->refill); + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ANNOUNCE)) + cancel_work_sync(&vi->announce); napi_disable(&vi->napi); return 0; @@ -962,11 +974,23 @@ static void virtnet_update_status(struct virtnet_info *vi) return; /* Ignore unknown (future) status bits */ - v &= VIRTIO_NET_S_LINK_UP; + v &= VIRTIO_NET_S_LINK_UP | VIRTIO_NET_S_ANNOUNCE;...

...vi = container_of(napi, struct virtnet_info, napi); @@ -787,6 +797,8 @@ static int virtnet_close(struct net_device *dev) /* Make sure refill_work doesn't re-enable napi! */ cancel_delayed_work_sync(&vi->refill); + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ANNOUNCE)) + cancel_work_sync(&vi->announce); napi_disable(&vi->napi); return 0; @@ -962,11 +974,23 @@ static void virtnet_update_status(struct virtnet_info *vi) return; /* Ignore unknown (future) status bits */ - v &= VIRTIO_NET_S_LINK_UP; + v &= VIRTIO_NET_S_LINK_UP | VIRTIO_NET_S_ANNOUNCE;...

...alarm; freezer waken the kthread and it restored the balloon later + added a spin lock to prevent updating the balloon when being removed + split the work into two; it allows to update stats even when the balloon is being resized Changes against v3: + rebased on 4.4-rc3 + call cancel_work_sync() when removing the balloon + do not queue the work from fill_balloon() and leak_balloon() because they are called also independently from the workqueue, e.g. remove_common(), virtballoon_oom_notify(). Re-queue the work from the work function when necessary. Changes against v2:...