svn commit: r326218 - head/sys/kern
Hans Petter Selasky
hps at selasky.org
Wed Nov 29 18:45:00 UTC 2017
On 11/29/17 17:59, Justin Hibbits wrote:
> On Wed, Nov 29, 2017 at 10:13 AM, Konstantin Belousov
> <kostikbel at gmail.com> wrote:
>> On Wed, Nov 29, 2017 at 04:33:50PM +0100, Hans Petter Selasky wrote:
>>> Hi Nathan,
>>>
>>> The chunk below causes sched_pin() to stop working and should be removed
>>> from your commit ??!!
>>>
>>> It probably explains the hangs seen recently reported by various brave
>>> people running 12-current :-)
>>>
>>> Specifically I see threads migrating between CPUs when td->td_pinned > 0
>>> using the LinuxKPI RCU API, which in turn leads to a hang when trying to
>>> synchronize RCU.
>>>
>>> --HPS
>>>
>>> diff --git a/sys/kern/sched_ule.c b/sys/kern/sched_ule.c
>>> index 5c8bae5afa1..bd4b505f6c3 100644
>>> --- a/sys/kern/sched_ule.c
>>> +++ b/sys/kern/sched_ule.c
>>> @@ -2453,7 +2453,7 @@ sched_add(struct thread *td, int flags)
>>> * Pick the destination cpu and if it isn't ours transfer to the
>>> * target cpu.
>>> */
>>> - td_get_sched(td)->ts_cpu = curcpu; /* Pick something valid to
>>> start */
>>> +// td_get_sched(td)->ts_cpu = curcpu; /* Pick something valid to
>>> start */
>>> cpu = sched_pickcpu(td, flags);
>>> tdq = sched_setcpu(td, cpu, flags);
>>> tdq_add(tdq, td, flags);
>>
>> To clarify. It seems that this change breaks sched_bind().
>>
>> It might be that LinuxKPI does use sched_pin() in somewhat questionable
>> way. Namely, the code puts the thread off the CPU (e.g. by taking a
>> lock). Then, is it guaranteed that the pinned thread returns to the same
>> cpu after sched_add() ?
>>
>> I think that the second behaviour is not guaranteed, but it might
>> happens by the way the things are arranged. If guaranteed, then the
>> sched_pin() breakage is same as for sched_bind().
>>
>
> I see the same breakage on PowerPC with the dtsec(4) driver, which
> pins interrupts to CPUs, now causing interrupts to migrate to cores
> without the matching portal mapping.
>
> - Justin
Hi Nathan,
Looking at your patch I think what you wanted to do is something like
shown below. I'm not sure if sched_setup() is run early enough to catch
all created threads in the registered eventhandler hook. I'll leave the
final solution and analysis up to you.
--HPS
> diff --git a/sys/kern/sched_ule.c b/sys/kern/sched_ule.c
> index 5c8bae5afa1..42ccf58ea0c 100644
> --- a/sys/kern/sched_ule.c
> +++ b/sys/kern/sched_ule.c
> @@ -1387,6 +1387,13 @@ sched_setup_smp(void)
> }
> #endif
>
> +static void
> +sched_thread_ctor(void *arg __unused, struct thread *td)
> +{
> + /* Pick a valid initial CPU number */
> + td_get_sched(td)->ts_cpu = curcpu;
> +}
> +
> /*
> * Setup the thread queues and initialize the topology based on MD
> * information.
> @@ -1396,6 +1403,9 @@ sched_setup(void *dummy)
> {
> struct tdq *tdq;
>
> + (void) EVENTHANDLER_REGISTER(thread_ctor,
> + sched_thread_ctor, NULL, EVENTHANDLER_PRI_ANY);
> +
> tdq = TDQ_SELF();
> #ifdef SMP
> sched_setup_smp();
> @@ -2453,7 +2463,6 @@ sched_add(struct thread *td, int flags)
> * Pick the destination cpu and if it isn't ours transfer to the
> * target cpu.
> */
> - td_get_sched(td)->ts_cpu = curcpu; /* Pick something valid to start */
> cpu = sched_pickcpu(td, flags);
> tdq = sched_setcpu(td, cpu, flags);
> tdq_add(tdq, td, flags);
More information about the svn-src-all
mailing list