Date: Mon, 25 Apr 2011 14:58:35 -0400 From: John Baldwin <jhb@freebsd.org> To: Ryan Stone <rysto32@gmail.com> Cc: freebsd-current@freebsd.org, Ed Maste <emaste@freebsd.org> Subject: Re: sched_4bsd startup crash trying to run a bound thread on an AP that hasn't started Message-ID: <201104251458.35718.jhb@freebsd.org> In-Reply-To: <BANLkTin9MgwyZtyiLWJsHbFxv0ihicbiEg@mail.gmail.com> References: <BANLkTinSyDaY-06N95n8c1NxOSdEnb5FkQ@mail.gmail.com> <201104061429.50185.jhb@freebsd.org> <BANLkTin9MgwyZtyiLWJsHbFxv0ihicbiEg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday, April 20, 2011 6:02:42 pm Ryan Stone wrote:
> On Wed, Apr 6, 2011 at 2:29 PM, John Baldwin <jhb@freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201104251458.35718.jhb>