search for: kthread_stop

upstream commit 2ad49887150894b9ed6a87a76b409adceee6b074 Motivated from the commit, I found that cleaner_kthread() and transaction_kthread() can result similar error since these two function can exit even though kthread_should_stop() is not true. In order to resolve the bug, break statements are changed into continue statements in order to wait until kthread_should_stop() becomes true. ---

...2014 at 11:07:46AM -0500, Tejun Heo wrote: > On Thu, Nov 20, 2014 at 05:03:17PM +0100, Petr Mladek wrote: > ... > > @@ -476,7 +460,6 @@ static void virtballoon_remove(struct virtio_device *vdev) > > { > > struct virtio_balloon *vb = vdev->priv; > > > > - kthread_stop(vb->thread); > > 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? > Thanks. > > -- > tejun

...2014 at 11:07:46AM -0500, Tejun Heo wrote: > On Thu, Nov 20, 2014 at 05:03:17PM +0100, Petr Mladek wrote: > ... > > @@ -476,7 +460,6 @@ static void virtballoon_remove(struct virtio_device *vdev) > > { > > struct virtio_balloon *vb = vdev->priv; > > > > - kthread_stop(vb->thread); > > 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? > Thanks. > > -- > tejun

...20, 2014 at 05:03:17PM +0100, Petr Mladek wrote: > > > ... > > > > @@ -476,7 +460,6 @@ static void virtballoon_remove(struct virtio_device *vdev) > > > > { > > > > struct virtio_balloon *vb = vdev->priv; > > > > > > > > - kthread_stop(vb->thread); > > > > 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,...

...20, 2014 at 05:03:17PM +0100, Petr Mladek wrote: > > > ... > > > > @@ -476,7 +460,6 @@ static void virtballoon_remove(struct virtio_device *vdev) > > > > { > > > > struct virtio_balloon *vb = vdev->priv; > > > > > > > > - kthread_stop(vb->thread); > > > > 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,...

On Mon, Aug 22, 2016 at 08:16:17AM -0600, Jonathan Corbet wrote: > On Mon, 22 Aug 2016 16:57:46 +0300 > "Michael S. Tsirkin" <mst at redhat.com> wrote: > > > commit commit ea04036032edda6f771c1381d03832d2ed0f6c31 ("CodingStyle: > > add some more error handling guidelines") suggests never naming goto > > labels after the goto location - that is

On Mon, Aug 22, 2016 at 08:16:17AM -0600, Jonathan Corbet wrote: > On Mon, 22 Aug 2016 16:57:46 +0300 > "Michael S. Tsirkin" <mst at redhat.com> wrote: > > > commit commit ea04036032edda6f771c1381d03832d2ed0f6c31 ("CodingStyle: > > add some more error handling guidelines") suggests never naming goto > > labels after the goto location - that is

...pattern, things will start to break, will they 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 wo...

...pattern, things will start to break, will they 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 wo...

We used to implement the work flushing through tracking queued seq, done seq, and the number of flushing. This patch simplify this by just implement work flushing through another kind of vhost work with completion. This will be used by lockless enqueuing patch. Signed-off-by: Jason Wang <jasowang at redhat.com> --- drivers/vhost/vhost.c | 53

We used to implement the work flushing through tracking queued seq, done seq, and the number of flushing. This patch simplify this by just implement work flushing through another kind of vhost work with completion. This will be used by lockless enqueuing patch. Signed-off-by: Jason Wang <jasowang at redhat.com> --- drivers/vhost/vhost.c | 53

...@@ -98,6 +99,8 @@ static inline void cleanup_rng(struct kref *kref) if (rng->cleanup) rng->cleanup(rng); + rng->cleanup_done = true; + wake_up_all(&rng_done); } static void set_current_rng(struct hwrng *rng) @@ -536,6 +539,11 @@ void hwrng_unregister(struct hwrng *rng) kthread_stop(hwrng_fill); } else mutex_unlock(&rng_mutex); + + /* Just in case rng is reading right now, wait. */ + wait_event(rng_done, rng->cleanup_done && + atomic_read(&rng->ref.refcount) == 0); + } EXPORT_SYMBOL_GPL(hwrng_unregister); diff --git a/include/linux/hw_random....

...@@ -98,6 +99,8 @@ static inline void cleanup_rng(struct kref *kref) if (rng->cleanup) rng->cleanup(rng); + rng->cleanup_done = true; + wake_up_all(&rng_done); } static void set_current_rng(struct hwrng *rng) @@ -536,6 +539,11 @@ void hwrng_unregister(struct hwrng *rng) kthread_stop(hwrng_fill); } else mutex_unlock(&rng_mutex); + + /* Just in case rng is reading right now, wait. */ + wait_event(rng_done, rng->cleanup_done && + atomic_read(&rng->ref.refcount) == 0); + } EXPORT_SYMBOL_GPL(hwrng_unregister); diff --git a/include/linux/hw_random....

...501 if (err) 502 goto err_cgroup; This name doesn't make sense because it's a come-from label which is used twice. Some people do: if (err) goto err_iovecs; 503 504 return 0; Then they add two labels here: err_iovecs: err_cgroup: kthread_stop(worker); But if you base the label name on the label location then it makes sense. goto stop_kthread; goto err_mmput;. 505 err_cgroup: 506 kthread_stop(worker); 507 dev->worker = NULL; 508 err_worker: 509 if (dev->mm) 510 mmp...

...ier(&vb->nb); > out_oom_notify: > 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_ba...

...ier(&vb->nb); > out_oom_notify: > 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_ba...

...thread)) { - err = PTR_ERR(vb->thread); - goto out_del_vqs; - } - return 0; -out_del_vqs: - vdev->config->del_vqs(vdev); out_free_vb: kfree(vb); out: @@ -476,7 +460,6 @@ static void virtballoon_remove(struct virtio_device *vdev) { struct virtio_balloon *vb = vdev->priv; - kthread_stop(vb->thread); remove_common(vb); kfree(vb); } @@ -487,7 +470,7 @@ static int virtballoon_freeze(struct virtio_device *vdev) struct virtio_balloon *vb = vdev->priv; /* - * The kthread is already frozen by the PM core before this + * The workqueue is already frozen by the PM core be...

...thread)) { - err = PTR_ERR(vb->thread); - goto out_del_vqs; - } - return 0; -out_del_vqs: - vdev->config->del_vqs(vdev); out_free_vb: kfree(vb); out: @@ -476,7 +460,6 @@ static void virtballoon_remove(struct virtio_device *vdev) { struct virtio_balloon *vb = vdev->priv; - kthread_stop(vb->thread); remove_common(vb); kfree(vb); } @@ -487,7 +470,7 @@ static int virtballoon_freeze(struct virtio_device *vdev) struct virtio_balloon *vb = vdev->priv; /* - * The kthread is already frozen by the PM core before this + * The workqueue is already frozen by the PM core be...

...fffffff8108b68e>] worker_thread+0x2ce/0x4e0 [ 0.831266] [<ffffffff8108b3c0>] ? process_one_work+0x500/0x500 [ 0.831266] [<ffffffff81090b28>] kthread+0xf8/0x100 [ 0.831266] [<ffffffff810bad7d>] ? trace_hardirqs_on+0xd/0x10 [ 0.831266] [<ffffffff81090a30>] ? kthread_stop+0x140/0x140 [ 0.831266] [<ffffffff816ea92c>] ret_from_fork+0x7c/0xb0 [ 0.831266] [<ffffffff81090a30>] ? kthread_stop+0x140/0x140 [ 0.831266] Code: c7 c2 48 31 01 83 48 c7 c6 e9 55 a9 81 e8 55 b4 c6 ff 4d 8b b4 24 58 01 00 00 49 8b 86 e8 04 00 00 4c 89 f7 ff 50 10 a8 04 74 0...

...atic int vhost_worker(void *data) { struct vhost_dev *dev = data; - struct vhost_work *work = NULL; + struct vhost_work *work, *work_next; + struct llist_node *node; mm_segment_t oldfs = get_fs(); set_fs(USER_DS); @@ -315,29 +317,25 @@ static int vhost_worker(void *data) /* mb paired w/ kthread_stop */ set_current_state(TASK_INTERRUPTIBLE); - spin_lock_irq(&dev->work_lock); - if (kthread_should_stop()) { - spin_unlock_irq(&dev->work_lock); __set_current_state(TASK_RUNNING); break; } - if (!list_empty(&dev->work_list)) { - work = list_first_entry(&a...