> Date: Thu, 1 Oct 2020 09:09:50 +0200
> From: Sebastien Marie <sema...@online.fr>
>
> Hi,
>
> Currently, when a process is calling kthread_stop(), it sets a flag
> asking the thread to stop, and enters in sleep mode, but the code
> doing the stop doesn't wakeup the caller of kthread_stop().
>
> The thread should also be unparked as else it will not seen the
> KTHREAD_SHOULDSTOP flag. it follows what Linux is doing.
>
> While here, I added some comments in the locking logic for park/unpark
> and stop.
>
> Comments or OK ?
I don't think adding all those comments makes a lot of sense. This
uses a fairly standard tsleep/wakeup pattern and the some of the
comments really state the obvious. Can you do a diff that just adds
the missing wakeup() and kthread_unpark() call?
> -----------------------------------------------
> commit 70e71461c8598e28820f1743923cac40670f7c33
> from: Sébastien Marie <sema...@online.fr>
> date: Thu Oct 1 07:02:46 2020 UTC
>
> properly support kthread_stop()
> - wakeup pthread_stop() caller
> - unpark the thread if parked
>
> while here, add comments for locking logic for park/unpark/stop
>
> diff ec329a4429e2542bc24dd017b8001b22df43564c
> ce2b5031503711bbdd7a3067c76c4f18b1d8da82
> blob - 2cbd0905406ccc9d89c86cee38673a4e9c3fcf42
> blob + f0e5a5a1b282c071c97505556510952ee7a6282a
> --- sys/dev/pci/drm/drm_linux.c
> +++ sys/dev/pci/drm/drm_linux.c
> @@ -206,6 +206,10 @@ kthread_func(void *arg)
>
> ret = thread->func(thread->data);
> thread->flags |= KTHREAD_STOPPED;
> +
> + /* wakeup thread waiting in kthread_stop() */
> + wakeup(thread);
> +
> kthread_exit(ret);
> }
>
> @@ -256,7 +260,14 @@ kthread_parkme(void)
>
> while (thread->flags & KTHREAD_SHOULDPARK) {
> thread->flags |= KTHREAD_PARKED;
> +
> + /*
> + * wakeup kthread_park() caller
> + * to signal I am parked as asked.
> + */
> wakeup(thread);
> +
> + /* wait for someone to kthread_unpark() me */
> tsleep_nsec(thread, PPAUSE, "parkme", INFSLP);
> thread->flags &= ~KTHREAD_PARKED;
> }
> @@ -269,7 +280,13 @@ kthread_park(struct proc *p)
>
> while ((thread->flags & KTHREAD_PARKED) == 0) {
> thread->flags |= KTHREAD_SHOULDPARK;
> +
> wake_up_process(thread->proc);
> +
> + /*
> + * wait for thread to be parked.
> + * the asked thread should call kthread_parkme()
> + */
> tsleep_nsec(thread, PPAUSE, "park", INFSLP);
> }
> }
> @@ -280,6 +297,8 @@ kthread_unpark(struct proc *p)
> struct kthread *thread = kthread_lookup(p);
>
> thread->flags &= ~KTHREAD_SHOULDPARK;
> +
> + /* wakeup kthread_parkme() caller */
> wakeup(thread);
> }
>
> @@ -297,7 +316,13 @@ kthread_stop(struct proc *p)
>
> while ((thread->flags & KTHREAD_STOPPED) == 0) {
> thread->flags |= KTHREAD_SHOULDSTOP;
> +
> + /* kthread_unpark() the thread if parked */
> + kthread_unpark(p);
> +
> wake_up_process(thread->proc);
> +
> + /* wait for thread to stop (func() should return) */
> tsleep_nsec(thread, PPAUSE, "stop", INFSLP);
> }
> LIST_REMOVE(thread, next);
>
>
>
