From owner-freebsd-current@FreeBSD.ORG Mon Apr 25 18:58:37 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E3F62106564A; Mon, 25 Apr 2011 18:58:37 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 9DD548FC0C; Mon, 25 Apr 2011 18:58:37 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 2822746B99; Mon, 25 Apr 2011 14:58:37 -0400 (EDT) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 912338A027; Mon, 25 Apr 2011 14:58:36 -0400 (EDT) From: John Baldwin To: Ryan Stone Date: Mon, 25 Apr 2011 14:58:35 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110325; KDE/4.5.5; amd64; ; ) References: <201104061429.50185.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201104251458.35718.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Mon, 25 Apr 2011 14:58:36 -0400 (EDT) Cc: freebsd-current@freebsd.org, Ed Maste Subject: Re: sched_4bsd startup crash trying to run a bound thread on an AP that hasn't started X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Apr 2011 18:58:38 -0000 On Wednesday, April 20, 2011 6:02:42 pm Ryan Stone wrote: > On Wed, Apr 6, 2011 at 2:29 PM, John Baldwin wrote: > > I guess one other option would be something like this: > > > > if (smp_started && (td->td_pinned != 0 || td->td_flags & TDF_BOUND || > > ts->ts_flags & TSF_AFFINITY)) { > > if (td->td_pinned != 0) > > cpu = td->td_lastcpu; > > else if (td->td_flags & TDF_BOUND) { > > /* Find CPU from bound runq. */ > > KASSERT(...); > > cpu = ts->ts_runq - &runq_pcpu[0]; > > } else > > /* Find a valid CPU for our cpuset. */ > > cpu = sched_pickcpu(td); > > ts->ts_runq = &runq_pcpu[cpu]; > > single_cpu = 1; > > CTR3(KTR_RUNQ, ...); > > } else { > > /* Global runq case. */ > > } > > > > This also avoids duplicating some common code to all the single_cpu cases. > > > > -- > > John Baldwin > > > > I went with this option. Does this look right? Yes, I would perhaps tweak the comment to reflect the full if statement though. Maybe something like: /* * If SMP is started and the thread is pinned or otherwise limited to * a specific set of CPUs, queue the thread to a per-CPU run queue. * Otherwise, queue the thread to the global run queue. */ > > Index: sys/kern/sched_4bsd.c > =================================================================== > --- sys/kern/sched_4bsd.c (revision 220603) > +++ sys/kern/sched_4bsd.c (working copy) > @@ -1246,30 +1246,28 @@ > } > TD_SET_RUNQ(td); > > - if (td->td_pinned != 0) { > - cpu = td->td_lastcpu; > + /* > + * If SMP is not started, don't obey any requested CPU pinning as that > + * CPU has either not yet started or it is curcpu. Trying to run a > + * thread on a CPU that has not yet started will panic the system. > + */ > + if (smp_started && (td->td_pinned != 0 || td->td_flags & TDF_BOUND || > + ts->ts_flags & TSF_AFFINITY)) { > + if (td->td_pinned != 0) > + cpu = td->td_lastcpu; > + else if (td->td_flags & TDF_BOUND) { > + /* Find CPU from bound runq. */ > + KASSERT(SKE_RUNQ_PCPU(ts), > + ("sched_add: bound td_sched not on cpu runq")); > + cpu = ts->ts_runq - &runq_pcpu[0]; > + } else > + /* Find a valid CPU for our cpuset */ > + cpu = sched_pickcpu(td); > ts->ts_runq = &runq_pcpu[cpu]; > single_cpu = 1; > CTR3(KTR_RUNQ, > "sched_add: Put td_sched:%p(td:%p) on cpu%d runq", ts, td, > cpu); > - } else if (td->td_flags & TDF_BOUND) { > - /* Find CPU from bound runq. */ > - KASSERT(SKE_RUNQ_PCPU(ts), > - ("sched_add: bound td_sched not on cpu runq")); > - cpu = ts->ts_runq - &runq_pcpu[0]; > - single_cpu = 1; > - CTR3(KTR_RUNQ, > - "sched_add: Put td_sched:%p(td:%p) on cpu%d runq", ts, td, > - cpu); > - } else if (ts->ts_flags & TSF_AFFINITY) { > - /* Find a valid CPU for our cpuset */ > - cpu = sched_pickcpu(td); > - ts->ts_runq = &runq_pcpu[cpu]; > - single_cpu = 1; > - CTR3(KTR_RUNQ, > - "sched_add: Put td_sched:%p(td:%p) on cpu%d runq", ts, td, > - cpu); > } else { > CTR2(KTR_RUNQ, > "sched_add: adding td_sched:%p (td:%p) to gbl runq", ts, > -- John Baldwin