Module Name: src Committed By: riastradh Date: Sun Dec 19 12:42:25 UTC 2021
Modified Files: src/sys/external/bsd/drm2/dist/drm/scheduler: sched_main.c src/sys/external/bsd/drm2/include/linux: kthread.h src/sys/external/bsd/drm2/linux: linux_kthread.c Log Message: drm: Rework Linux `kthread' abstraction to avoid race to sleep. Requires passing in the caller's lock and condvar to kthread_run, but for the one user that appears not to be an onerous requirement. To generate a diff of this commit: cvs rdiff -u -r1.7 -r1.8 \ src/sys/external/bsd/drm2/dist/drm/scheduler/sched_main.c cvs rdiff -u -r1.3 -r1.4 src/sys/external/bsd/drm2/include/linux/kthread.h cvs rdiff -u -r1.5 -r1.6 src/sys/external/bsd/drm2/linux/linux_kthread.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/external/bsd/drm2/dist/drm/scheduler/sched_main.c diff -u src/sys/external/bsd/drm2/dist/drm/scheduler/sched_main.c:1.7 src/sys/external/bsd/drm2/dist/drm/scheduler/sched_main.c:1.8 --- src/sys/external/bsd/drm2/dist/drm/scheduler/sched_main.c:1.7 Sun Dec 19 12:41:44 2021 +++ src/sys/external/bsd/drm2/dist/drm/scheduler/sched_main.c Sun Dec 19 12:42:25 2021 @@ -1,4 +1,4 @@ -/* $NetBSD: sched_main.c,v 1.7 2021/12/19 12:41:44 riastradh Exp $ */ +/* $NetBSD: sched_main.c,v 1.8 2021/12/19 12:42:25 riastradh Exp $ */ /* * Copyright 2015 Advanced Micro Devices, Inc. @@ -47,7 +47,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: sched_main.c,v 1.7 2021/12/19 12:41:44 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: sched_main.c,v 1.8 2021/12/19 12:42:25 riastradh Exp $"); #include <linux/kthread.h> #include <linux/wait.h> @@ -857,7 +857,8 @@ int drm_sched_init(struct drm_gpu_schedu atomic64_set(&sched->job_id_count, 0); /* Each scheduler will run on a seperate kernel thread */ - sched->thread = kthread_run(drm_sched_main, sched, sched->name); + sched->thread = kthread_run(drm_sched_main, sched, sched->name, + &sched->job_list_lock, &sched->wake_up_worker); if (IS_ERR(sched->thread)) { ret = PTR_ERR(sched->thread); sched->thread = NULL; Index: src/sys/external/bsd/drm2/include/linux/kthread.h diff -u src/sys/external/bsd/drm2/include/linux/kthread.h:1.3 src/sys/external/bsd/drm2/include/linux/kthread.h:1.4 --- src/sys/external/bsd/drm2/include/linux/kthread.h:1.3 Sun Dec 19 12:23:07 2021 +++ src/sys/external/bsd/drm2/include/linux/kthread.h Sun Dec 19 12:42:25 2021 @@ -1,4 +1,4 @@ -/* $NetBSD: kthread.h,v 1.3 2021/12/19 12:23:07 riastradh Exp $ */ +/* $NetBSD: kthread.h,v 1.4 2021/12/19 12:42:25 riastradh Exp $ */ /*- * Copyright (c) 2013 The NetBSD Foundation, Inc. @@ -32,6 +32,10 @@ #ifndef _LINUX_KTHREAD_H_ #define _LINUX_KTHREAD_H_ +#include <linux/spinlock.h> + +#include <drm/drm_wait_netbsd.h> + struct task_struct; #define __kthread_should_park linux___kthread_should_park @@ -43,7 +47,8 @@ struct task_struct; #define kthread_stop linux_kthread_stop #define kthread_unpark linux_kthread_unpark -struct task_struct *kthread_run(int (*)(void *), void *, const char *); +struct task_struct *kthread_run(int (*)(void *), void *, const char *, + spinlock_t *, drm_waitqueue_t *); int kthread_stop(struct task_struct *); int kthread_should_stop(void); Index: src/sys/external/bsd/drm2/linux/linux_kthread.c diff -u src/sys/external/bsd/drm2/linux/linux_kthread.c:1.5 src/sys/external/bsd/drm2/linux/linux_kthread.c:1.6 --- src/sys/external/bsd/drm2/linux/linux_kthread.c:1.5 Sun Dec 19 12:42:14 2021 +++ src/sys/external/bsd/drm2/linux/linux_kthread.c Sun Dec 19 12:42:25 2021 @@ -1,4 +1,4 @@ -/* $NetBSD: linux_kthread.c,v 1.5 2021/12/19 12:42:14 riastradh Exp $ */ +/* $NetBSD: linux_kthread.c,v 1.6 2021/12/19 12:42:25 riastradh Exp $ */ /*- * Copyright (c) 2021 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: linux_kthread.c,v 1.5 2021/12/19 12:42:14 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: linux_kthread.c,v 1.6 2021/12/19 12:42:25 riastradh Exp $"); #include <sys/types.h> @@ -42,6 +42,9 @@ __KERNEL_RCSID(0, "$NetBSD: linux_kthrea #include <sys/specificdata.h> #include <linux/kthread.h> +#include <linux/spinlock.h> + +#include <drm/drm_wait_netbsd.h> struct task_struct { kmutex_t kt_lock; @@ -52,6 +55,8 @@ struct task_struct { int (*kt_func)(void *); void *kt_cookie; + spinlock_t *kt_interlock; + drm_waitqueue_t *kt_wq; struct lwp *kt_lwp; }; @@ -109,7 +114,8 @@ linux_kthread_start(void *cookie) } static struct task_struct * -kthread_alloc(int (*func)(void *), void *cookie) +kthread_alloc(int (*func)(void *), void *cookie, spinlock_t *interlock, + drm_waitqueue_t *wq) { struct task_struct *T; @@ -120,6 +126,8 @@ kthread_alloc(int (*func)(void *), void T->kt_func = func; T->kt_cookie = cookie; + T->kt_interlock = interlock; + T->kt_wq = wq; return T; } @@ -134,12 +142,13 @@ kthread_free(struct task_struct *T) } struct task_struct * -kthread_run(int (*func)(void *), void *cookie, const char *name) +kthread_run(int (*func)(void *), void *cookie, const char *name, + spinlock_t *interlock, drm_waitqueue_t *wq) { struct task_struct *T; int error; - T = kthread_alloc(func, cookie); + T = kthread_alloc(func, cookie, interlock, wq); error = kthread_create(PRI_NONE, KTHREAD_MPSAFE|KTHREAD_MUSTJOIN, NULL, linux_kthread_start, T, &T->kt_lwp, "%s", name); if (error) { @@ -150,59 +159,35 @@ kthread_run(int (*func)(void *), void *c return T; } -/* - * lwp_kick(l) - * - * Cause l to wake up if it is asleep, no matter what condvar or - * other wchan it's asleep on. This logic is like sleepq_timeout, - * but without setting LW_STIMO. This is not a general-purpose - * mechanism -- don't go around using this instead of condvars. - */ -static void -lwp_kick(struct lwp *l) -{ - - lwp_lock(l); - if (l->l_wchan == NULL) { - /* Not sleeping, so no need to wake up. */ - lwp_unlock(l); - } else { - /* - * Sleeping, so wake it up. lwp_unsleep has the side - * effect of unlocking l when we pass unlock=true. - */ - lwp_unsleep(l, /*unlock*/true); - } -} - int kthread_stop(struct task_struct *T) { - struct lwp *l; int ret; + /* Lock order: interlock, then kthread lock. */ + spin_lock(T->kt_interlock); + mutex_enter(&T->kt_lock); + /* * Notify the thread that it's stopping, and wake it if it's - * parked. + * parked or sleeping on its own waitqueue. */ - mutex_enter(&T->kt_lock); T->kt_shouldpark = false; T->kt_shouldstop = true; cv_broadcast(&T->kt_cv); + DRM_SPIN_WAKEUP_ALL(T->kt_wq, T->kt_interlock); + + /* Release the locks. */ mutex_exit(&T->kt_lock); + spin_unlock(T->kt_interlock); - /* - * Kick the lwp in case it's waiting on anything else, and then - * wait for it to complete. It is the thread's obligation to - * check kthread_shouldstop before sleeping again. - */ - l = T->kt_lwp; - KASSERT(l != curlwp); - lwp_kick(l); - ret = kthread_join(l); + /* Wait for the (NetBSD) kthread to exit. */ + ret = kthread_join(T->kt_lwp); + /* Free the (Linux) kthread. */ kthread_free(T); + /* Return what the thread returned. */ return ret; } @@ -222,8 +207,9 @@ kthread_should_stop(void) void kthread_park(struct task_struct *T) { - struct lwp *l; + /* Lock order: interlock, then kthread lock. */ + spin_lock(T->kt_interlock); mutex_enter(&T->kt_lock); /* Caller must not ask to park if they've already asked to stop. */ @@ -232,22 +218,26 @@ kthread_park(struct task_struct *T) /* Ask the thread to park. */ T->kt_shouldpark = true; - /* Don't wait for ourselves -- Linux allows this semantics. */ - if ((l = T->kt_lwp) == curlwp) - goto out; - /* - * If the thread is asleep for any reason, give it a spurious - * wakeup. The thread is responsible for checking - * kthread_shouldpark before sleeping. + * Ensure the thread is not sleeping on its condvar. After + * this point, we are done with the interlock, which we must + * not hold while we wait on the kthread condvar. */ - lwp_kick(l); + DRM_SPIN_WAKEUP_ALL(T->kt_wq, T->kt_interlock); + spin_unlock(T->kt_interlock); - /* Wait until the thread has issued kthread_parkme. */ - while (!T->kt_parked) - cv_wait(&T->kt_cv, &T->kt_lock); + /* + * Wait until the thread has issued kthread_parkme, unless we + * are already the thread, which Linux allows and interprets to + * mean don't wait. + */ + if (T->kt_lwp != curlwp) { + while (!T->kt_parked) + cv_wait(&T->kt_cv, &T->kt_lock); + } -out: mutex_exit(&T->kt_lock); + /* Release the kthread lock too. */ + mutex_exit(&T->kt_lock); } void