From 203509662c62a4ce6593fd5104c62ddb35942a29 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 29 Dec 2021 22:25:14 +0000 Subject: [PATCH 1/3] sysmon(9): New sysmon_task_queue_barrier(pri) function. This waits for the completion of all tasks at priority pri or lower that are currently queued at the time of the call. --- share/man/man9/sysmon_taskq.9 | 10 +++- sys/dev/sysmon/sysmon_taskq.c | 87 +++++++++++++++++++++++++++++------ sys/dev/sysmon/sysmon_taskq.h | 1 + 3 files changed, 84 insertions(+), 14 deletions(-) diff --git a/share/man/man9/sysmon_taskq.9 b/share/man/man9/sysmon_taskq.9 index baa3c86ee79e..eb66f315adbf 100644 --- a/share/man/man9/sysmon_taskq.9 +++ b/share/man/man9/sysmon_taskq.9 @@ -43,6 +43,8 @@ .Fn sysmon_task_queue_fini "void" .Ft int .Fn sysmon_task_queue_sched "u_int pri" "void (*func)(void *)" "void *arg" +.Ft void +.Fn sysmon_task_queue_barrier "u_int pri" .Sh DESCRIPTION The machine-independent .Nm @@ -67,7 +69,7 @@ All scheduled tasks are executed before the queue is halted. .Pp The .Fn sysmon_task_queue_sched -enqueues +function enqueues .Fa func to be executed at the priority .Fa pri . @@ -78,6 +80,12 @@ The single argument passed to .Fa func is specified by .Fa arg . +.Pp +The +.Fn sysmon_task_queue_barrier +function waits for the completion of all tasks at +.Fa pri +or lower currently queued at the time of the call. .Sh RETURN VALUES Upon successful completion, .Fn sysmon_task_queue_sched diff --git a/sys/dev/sysmon/sysmon_taskq.c b/sys/dev/sysmon/sysmon_taskq.c index e027d009b925..ef8f14d601de 100644 --- a/sys/dev/sysmon/sysmon_taskq.c +++ b/sys/dev/sysmon/sysmon_taskq.c @@ -202,6 +202,26 @@ sysmon_task_queue_thread(void *arg) kthread_exit(0); } +static void +sysmon_task_queue_sched_task(struct sysmon_task *st) +{ + struct sysmon_task *lst; + + mutex_enter(&sysmon_task_queue_mtx); + TAILQ_FOREACH(lst, &sysmon_task_queue, st_list) { + if (st->st_pri > lst->st_pri) { + TAILQ_INSERT_BEFORE(lst, st, st_list); + break; + } + } + + if (lst == NULL) + TAILQ_INSERT_TAIL(&sysmon_task_queue, st, st_list); + + cv_broadcast(&sysmon_task_queue_cv); + mutex_exit(&sysmon_task_queue_mtx); +} + /* * sysmon_task_queue_sched: * @@ -210,7 +230,7 @@ sysmon_task_queue_thread(void *arg) int sysmon_task_queue_sched(u_int pri, void (*func)(void *), void *arg) { - struct sysmon_task *st, *lst; + struct sysmon_task *st; (void)RUN_ONCE(&once_tq, tq_preinit); @@ -229,21 +249,62 @@ sysmon_task_queue_sched(u_int pri, void (*func)(void *), void *arg) st->st_arg = arg; st->st_pri = pri; - mutex_enter(&sysmon_task_queue_mtx); - TAILQ_FOREACH(lst, &sysmon_task_queue, st_list) { - if (st->st_pri > lst->st_pri) { - TAILQ_INSERT_BEFORE(lst, st, st_list); - break; - } - } + sysmon_task_queue_sched_task(st); - if (lst == NULL) - TAILQ_INSERT_TAIL(&sysmon_task_queue, st, st_list); + return 0; +} - cv_broadcast(&sysmon_task_queue_cv); - mutex_exit(&sysmon_task_queue_mtx); +struct tqbarrier { + kmutex_t lock; + kcondvar_t cv; + bool done; +}; - return 0; +static void +tqbarrier_task(void *cookie) +{ + struct tqbarrier *bar = cookie; + + mutex_enter(&bar->lock); + bar->done = true; + cv_broadcast(&bar->cv); + mutex_exit(&bar->lock); +} + +/* + * sysmon_task_queue_barrier: + * + * Wait for the completion of all tasks at priority pri or lower + * currently queued at the time of the call. + */ +void +sysmon_task_queue_barrier(u_int pri) +{ + struct sysmon_task st; + struct tqbarrier bar; + + (void)RUN_ONCE(&once_tq, tq_preinit); + + KASSERT(sysmon_task_queue_lwp); + KASSERT(curlwp != sysmon_task_queue_lwp); + + mutex_init(&bar.lock, MUTEX_DEFAULT, IPL_NONE); + cv_init(&bar.cv, "sysmontq"); + bar.done = false; + + st.st_func = &tqbarrier_task; + st.st_arg = &bar; + st.st_pri = pri; + + sysmon_task_queue_sched_task(&st); + + mutex_enter(&bar.lock); + while (!bar.done) + cv_wait(&bar.cv, &bar.lock); + mutex_exit(&bar.lock); + + cv_destroy(&bar.cv); + mutex_destroy(&bar.lock); } static diff --git a/sys/dev/sysmon/sysmon_taskq.h b/sys/dev/sysmon/sysmon_taskq.h index 32a4fcd7a2f8..63cbcfab5d36 100644 --- a/sys/dev/sysmon/sysmon_taskq.h +++ b/sys/dev/sysmon/sysmon_taskq.h @@ -41,5 +41,6 @@ void sysmon_task_queue_init(void); int sysmon_task_queue_fini(void); int sysmon_task_queue_sched(u_int, void (*)(void *), void *); +void sysmon_task_queue_barrier(u_int); #endif /* _DEV_SYSMON_SYSMON_TASKQ_H_ */ From 310176af97dd00dd14c43cb616afd9769a509129 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Thu, 30 Dec 2021 15:19:55 +0000 Subject: [PATCH 2/3] acpi(9): Implement AcpiOsWaitEventsComplete. --- sys/dev/acpi/acpica/OsdSchedule.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/sys/dev/acpi/acpica/OsdSchedule.c b/sys/dev/acpi/acpica/OsdSchedule.c index f7a7eb31a46b..f67ce5be38d8 100644 --- a/sys/dev/acpi/acpica/OsdSchedule.c +++ b/sys/dev/acpi/acpica/OsdSchedule.c @@ -101,6 +101,10 @@ AcpiOsExecute(ACPI_EXECUTE_TYPE Type, ACPI_OSD_EXEC_CALLBACK Function, { int pri; + /* + * If you raise any priority here make sure to adjust + * AcpiOsWaitEventsComplete too. + */ switch (Type) { case OSL_GPE_HANDLER: pri = 10; @@ -185,14 +189,13 @@ AcpiOsGetTimer(void) } /* - * * AcpiOsWaitEventsComplete: * - * Wait for all asynchronous events to complete. This implementation - * does nothing. + * Wait for all asynchronous events to complete. */ void AcpiOsWaitEventsComplete(void) { - return; + /* Highest priority used by AcpiOsExecute. */ + sysmon_task_queue_barrier(10); } From 93b2e3927e0682e65fc2a42b96ba9aec116c1d39 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 29 Dec 2021 22:54:11 +0000 Subject: [PATCH 3/3] acpi(9): Fix memory ordering and completion bugs in notifiers. 1. Guarantee everything which happened before acpi_register_notify has also happened before the notifier is actually called. 2. On acpi_deregister_notify, don't return until the notifier is definitely not running any more on any CPU, using AcpiOsWaitEventsComplete. --- sys/dev/acpi/acpi.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/sys/dev/acpi/acpi.c b/sys/dev/acpi/acpi.c index 76bab345c5d3..53ffbf333b04 100644 --- a/sys/dev/acpi/acpi.c +++ b/sys/dev/acpi/acpi.c @@ -107,6 +107,7 @@ __KERNEL_RCSID(0, "$NetBSD: acpi.c,v 1.294 2021/12/20 11:17:40 skrll Exp $"); #include "opt_pcifixup.h" #include +#include #include #include #include @@ -1146,6 +1147,7 @@ acpi_notify_handler(ACPI_HANDLE handle, uint32_t event, void *aux) { struct acpi_softc *sc = acpi_softc; struct acpi_devnode *ad; + ACPI_NOTIFY_HANDLER notify; KASSERT(sc != NULL); KASSERT(aux == NULL); @@ -1189,13 +1191,13 @@ acpi_notify_handler(ACPI_HANDLE handle, uint32_t event, void *aux) if (ad->ad_device == NULL) continue; - if (ad->ad_notify == NULL) + if ((notify = atomic_load_acquire(&ad->ad_notify)) == NULL) continue; if (ad->ad_handle != handle) continue; - (*ad->ad_notify)(ad->ad_handle, event, ad->ad_device); + (*notify)(ad->ad_handle, event, ad->ad_device); return; } @@ -1218,7 +1220,7 @@ acpi_register_notify(struct acpi_devnode *ad, ACPI_NOTIFY_HANDLER notify) if (ad == NULL || notify == NULL) goto fail; - ad->ad_notify = notify; + atomic_store_release(&ad->ad_notify, notify); return true; @@ -1233,7 +1235,10 @@ void acpi_deregister_notify(struct acpi_devnode *ad) { - ad->ad_notify = NULL; + atomic_store_relaxed(&ad->ad_notify, NULL); + + /* Wait for any in-flight calls to the notifier to complete. */ + AcpiOsWaitEventsComplete(); } /*