Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 26 Aug 2010 15:24:37 -0700
From:      mdf@FreeBSD.org
To:        Andriy Gapon <avg@icyb.net.ua>
Cc:        freebsd-current@freebsd.org
Subject:   Re: sched_pin() bug in SCHED_ULE
Message-ID:  <AANLkTi=fo2LiGm8rhffTd8rPryq7b1doQdVihmL0YMdT@mail.gmail.com>
In-Reply-To: <4C76E661.60600@icyb.net.ua>
References:  <AANLkTin6V9pc3d7ifyOmR2V-H5-g_AQFji-LbZzWfAKM@mail.gmail.com> <201008261649.20543.jhb@freebsd.org> <AANLkTi=7cLfH3h3jMVhbEzNsuiPod3Zx=PiazK7Dn4%2BA@mail.gmail.com> <4C76E661.60600@icyb.net.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Aug 26, 2010 at 3:10 PM, Andriy Gapon <avg@icyb.net.ua> wrote:
> on 27/08/2010 00:20 mdf@FreeBSD.org said the following:
>>
>> I tried making sched_pin() a real function which used
>> intr_disable/intr_restore around saving off td->td_oncpu,
>> td->td_lastcpu and ts->ts_cpu, and the stack at the time of call. =A0In
>> sched_switch when I saw an unexpected migration I printed all that
>> out. =A0I found that on my boxes, at sched_pin() time ts_cpu was already
>> different from td->td_oncpu, so the specific problem I was having was
>> that while another thread can change ts_cpu it has no way to force
>> that thread to immediately migrate.
>
> Like e.g. in sched_affinity where ts_cpu is first changed and then the ol=
d cpu
> is ipi-ed?

That could do it.  I didn't follow the stack of the places that were
touching ts_cpu for non-curthread.

>> I believe the below patch fixes the issue, though I'm open to other meth=
ods:
>>
>>
>> Index: kern/sched_ule.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- kern/sched_ule.c =A0(.../head/src/sys) =A0 =A0 =A0(revision 158279)
>> +++ kern/sched_ule.c =A0(.../branches/BR_BUG_67957/src/sys) =A0 =A0 (rev=
ision 158279)
>> @@ -112,6 +112,7 @@
>> =A0/* flags kept in ts_flags */
>> =A0#define =A0 =A0 =A0TSF_BOUND =A0 =A0 =A0 0x0001 =A0 =A0 =A0 =A0 =A0/*=
 Thread can not migrate. */
>> =A0#define =A0 =A0 =A0TSF_XFERABLE =A0 =A00x0002 =A0 =A0 =A0 =A0 =A0/* T=
hread was added as transferable. */
>> +#define =A0 =A0 =A0TSF_BINDING =A0 =A0 0x0004 =A0 =A0 =A0 =A0 =A0/* Thr=
ead is being bound. */
>
> I don't really follow why TSF_BINDING is needed, i.e. why TSF_BOUND is no=
t
> sufficient in this case?

I wanted to tighten up the asserts, so that the only time it was okay
for a td_pinned thread to migrate was the one time it was moving to
the target cpu.  Having a separate flag allows that.

Thanks,
matthew



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTi=fo2LiGm8rhffTd8rPryq7b1doQdVihmL0YMdT>