make sched_barrier use cond_wait/cond_signal.
authordlg <dlg@openbsd.org>
Thu, 14 Dec 2017 23:21:04 +0000 (23:21 +0000)
committerdlg <dlg@openbsd.org>
Thu, 14 Dec 2017 23:21:04 +0000 (23:21 +0000)
previously the code was using a percpu flag to manage the sleeps/wakeups,
which means multiple threads waiting for a barrier on a cpu could
race. moving to a cond struct on the stack fixes this.

while here, get rid of the sbar taskq and just use systqmp instead.
the barrier tasks are short, so there's no real downside.

ok mpi@

sys/kern/kern_sched.c
sys/sys/sched.h

index 54705ee..cf45011 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_sched.c,v 1.46 2017/11/28 16:22:27 visa Exp $    */
+/*     $OpenBSD: kern_sched.c,v 1.47 2017/12/14 23:21:04 dlg Exp $     */
 /*
  * Copyright (c) 2007, 2008 Artur Grabowski <art@openbsd.org>
  *
@@ -227,12 +227,6 @@ sched_exit(struct proc *p)
 void
 sched_init_runqueues(void)
 {
-#ifdef MULTIPROCESSOR
-       sbartq = taskq_create("sbar", 1, IPL_VM,
-           TASKQ_MPSAFE | TASKQ_CANTSLEEP);
-       if (sbartq == NULL)
-               panic("unable to create sbar taskq");
-#endif
 }
 
 void
@@ -658,24 +652,28 @@ sched_stop_secondary_cpus(void)
        }
 }
 
+struct sched_barrier_state {
+       struct cpu_info *ci;
+       struct cond cond;
+};
+
 void
 sched_barrier_task(void *arg)
 {
-       struct cpu_info *ci = arg;
+       struct sched_barrier_state *sb = arg;
+       struct cpu_info *ci = sb->ci;
 
        sched_peg_curproc(ci);
-       ci->ci_schedstate.spc_barrier = 1;
-       wakeup(&ci->ci_schedstate.spc_barrier);
+       cond_signal(&sb->cond);
        atomic_clearbits_int(&curproc->p_flag, P_CPUPEG);
 }
 
 void
 sched_barrier(struct cpu_info *ci)
 {
-       struct sleep_state sls;
+       struct sched_barrier_state sb;
        struct task task;
        CPU_INFO_ITERATOR cii;
-       struct schedstate_percpu *spc;
 
        if (ci == NULL) {
                CPU_INFO_FOREACH(cii, ci) {
@@ -688,14 +686,12 @@ sched_barrier(struct cpu_info *ci)
        if (ci == curcpu())
                return;
 
-       task_set(&task, sched_barrier_task, ci);
-       spc = &ci->ci_schedstate;
-       spc->spc_barrier = 0;
-       task_add(sbartq, &task);
-       while (!spc->spc_barrier) {
-               sleep_setup(&sls, &spc->spc_barrier, PWAIT, "sbar");
-               sleep_finish(&sls, !spc->spc_barrier);
-       }
+       sb.ci = ci;
+       cond_init(&sb.cond);
+       task_set(&task, sched_barrier_task, &sb);
+
+       task_add(systqmp, &task);
+       cond_wait(&sb.cond, "sbar");
 }
 
 #else
index 0da1f2c..698eda8 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: sched.h,v 1.43 2017/12/04 09:51:03 mpi Exp $  */
+/*     $OpenBSD: sched.h,v 1.44 2017/12/14 23:21:04 dlg Exp $  */
 /* $NetBSD: sched.h,v 1.2 1999/02/28 18:14:58 ross Exp $ */
 
 /*-
@@ -114,8 +114,6 @@ struct schedstate_percpu {
        struct proc *spc_reaper;        /* dead proc reaper */
 #endif
        LIST_HEAD(,proc) spc_deadproc;
-
-       volatile int spc_barrier;       /* for sched_barrier() */
 };
 
 #ifdef _KERNEL