Re: sched_4bsd startup crash trying to run a bound thread on an AP that hasn't started

On Wednesday, April 20, 2011 6:02:42 pm Ryan Stone wrote:
> On Wed, Apr 6, 2011 at 2:29 PM, John Baldwin <jhb_at_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)
> _at__at_ -1246,30 +1246,28 _at__at_
>         }
>         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