Date: Wed, 6 Apr 2011 13:21:04 -0400 From: Attilio Rao <attilio@freebsd.org> To: Ryan Stone <rysto32@gmail.com> Cc: freebsd-current@freebsd.org Subject: Re: sched_4bsd startup crash trying to run a bound thread on an AP that hasn't started Message-ID: <BANLkTimDrgGN_e9V8H7O%2BbqXTpYnKPrPZg@mail.gmail.com> In-Reply-To: <BANLkTinvisvGiQOs5w-nsxzRVbJUN5%2B5yQ@mail.gmail.com> References: <BANLkTinSyDaY-06N95n8c1NxOSdEnb5FkQ@mail.gmail.com> <201104060836.56542.jhb@freebsd.org> <BANLkTinvisvGiQOs5w-nsxzRVbJUN5%2B5yQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
2011/4/6 Ryan Stone <rysto32@gmail.com>:
> On Wed, Apr 6, 2011 at 8:36 AM, John Baldwin <jhb@freebsd.org> wrote:
>> Hummm. =C2=A0Patching 4BSD to use the same route as ULE may be the best =
solution
>> for now if that is easiest. =C2=A0Alternatively, 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 =C2=A0 =C2=A0 =C2=A0 =C2=A0(revision 220222)
> +++ sched_4bsd.c =C2=A0 =C2=A0 =C2=A0 =C2=A0(working copy)
> @@ -1242,14 +1242,14 @@
> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> =C2=A0 =C2=A0 =C2=A0 =C2=A0TD_SET_RUNQ(td);
>
> - =C2=A0 =C2=A0 =C2=A0 if (td->td_pinned !=3D 0) {
> + =C2=A0 =C2=A0 =C2=A0 if (smp_started && td->td_pinned !=3D 0) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cpu =3D td->td_las=
tcpu;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ts->ts_runq =3D &r=
unq_pcpu[cpu];
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0single_cpu =3D 1;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0CTR3(KTR_RUNQ,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"sch=
ed_add: Put td_sched:%p(td:%p) on cpu%d runq", ts, td,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cpu)=
;
> - =C2=A0 =C2=A0 =C2=A0 } else if (td->td_flags & TDF_BOUND) {
> + =C2=A0 =C2=A0 =C2=A0 } else if (smp_started && (td->td_flags & TDF_BOUN=
D)) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Find CPU from b=
ound runq. */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0KASSERT(SKE_RUNQ_P=
CPU(ts),
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0("sc=
hed_add: bound td_sched not on cpu runq"));
> @@ -1258,7 +1258,7 @@
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0CTR3(KTR_RUNQ,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"sch=
ed_add: Put td_sched:%p(td:%p) on cpu%d runq", ts, td,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cpu)=
;
> - =C2=A0 =C2=A0 =C2=A0 } else if (ts->ts_flags & TSF_AFFINITY) {
> + =C2=A0 =C2=A0 =C2=A0 } else if (smp_started && (ts->ts_flags & TSF_AFFI=
NITY)) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Find a valid CP=
U for our cpuset */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cpu =3D sched_pick=
cpu(td);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ts->ts_runq =3D &r=
unq_pcpu[cpu];
>
> The flow control is a bit awkward because of the multiple
> affinity/bound cpu cases. =C2=A0If somebody prefers the code to be
> structured differently I'd be open to suggestions.
That is more or less what ULE does -- in ULE it is simpler because it
goes via sched_pickcpu(), which still returns always CPU0 if APs still
didn't kick off.
I would also add a comment on top explaining the check, eventually,
but otherwise looks fine.
Attilio
--=20
Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BANLkTimDrgGN_e9V8H7O%2BbqXTpYnKPrPZg>