On Wednesday, April 06, 2011 1:08:20 pm Ryan Stone wrote:
> On Wed, Apr 6, 2011 at 8:36 AM, John Baldwin <j...@freebsd.org> wrote:
> > Hummm.  Patching 4BSD to use the same route as ULE may be the best 
solution
> > for now if that is easiest.  Alternatively, you could change 4BSD's
> > sched_add() to not try to kick other CPUs until smp_started is true.
> 
> At first I thought that it was a consequence of the way it does CPU
> affinity, but now I see that it shortcuts if smp_started is not true.
> How about something like the following for 4BSD?
> 
> --- sched_4bsd.c        (revision 220222)
> +++ sched_4bsd.c        (working copy)
> @@ -1242,14 +1242,14 @@
>         }
>         TD_SET_RUNQ(td);
> 
> -       if (td->td_pinned != 0) {
> +       if (smp_started && td->td_pinned != 0) {
>                 cpu = td->td_lastcpu;
>                 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) {
> +       } else if (smp_started && (td->td_flags & TDF_BOUND)) {
>                 /* Find CPU from bound runq. */
>                 KASSERT(SKE_RUNQ_PCPU(ts),
>                     ("sched_add: bound td_sched not on cpu runq"));
> @@ -1258,7 +1258,7 @@
>                 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) {
> +       } else if (smp_started && (ts->ts_flags & TSF_AFFINITY)) {
>                 /* Find a valid CPU for our cpuset */
>                 cpu = sched_pickcpu(td);
>                 ts->ts_runq = &runq_pcpu[cpu];
> 
> The flow control is a bit awkward because of the multiple
> affinity/bound cpu cases.  If somebody prefers the code to be
> structured differently I'd be open to suggestions.

Maybe it could do this:

                if (!smp_started) {
                        cpu = NOCPU;
                        ts->runq = &runq;
                } else if (td->td_pinned) {
                        ...

That would be a smaller patch and I think more obvious to the reader even
though it duplicates the global runq selection.  I would even be ok with a
goto for this case that if !smp_started it just jumps to the global runq
bit in the last else.

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
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to