Date: Thu, 24 Aug 2017 12:25:13 -0700 (PDT) From: Don Lewis <truckman@FreeBSD.org> To: avg@FreeBSD.org Cc: freebsd-arch@FreeBSD.org Subject: Re: ULE steal_idle questions Message-ID: <201708241925.v7OJPDTT043392@gw.catspoiler.org> In-Reply-To: <d9dae0c1-e718-13fe-b6b5-87160c71784e@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 23 Aug, Andriy Gapon wrote:
> On 23/08/2017 18:04, Don Lewis wrote:
>> I've been looking at the steal_idle code in tdq_idled() and found some
>> things that puzzle me.
>>
>> Consider a machine with three CPUs:
>> A, which is idle
>> B, which is busy running a thread
>> C, which is busy running a thread and has another thread in queue
>> It would seem to make sense that the tdq_load values for these three
>> CPUs would be 0, 1, and 2 respectively in order to select the best CPU
>> to run a new thread.
>>
>> If so, then why do we pass thresh=1 to sched_highest() in the code that
>> implements steal_idle? That value is used to set cs_limit which is used
>> in this comparison in cpu_search:
>> if (match & CPU_SEARCH_HIGHEST)
>> if (tdq->tdq_load >= hgroup.cs_limit &&
>> That would seem to make CPU B a candidate for stealing a thread from.
>> Ignoring CPU C for the moment, that shouldn't happen if the thread is
>> running, but even if it was possible, it would just make CPU B go idle,
>> which isn't terribly helpful in terms of load balancing and would just
>> thrash the caches. The same comparison is repeated in tdq_idled() after
>> a candidate CPU has been chosen:
>> if (steal->tdq_load < thresh || steal->tdq_transferable == 0) {
>> tdq_unlock_pair(tdq, steal);
>> continue;
>> }
>>
>> It looks to me like there is an off-by-one error here, and there is a
>> similar problem in the code that implements kern.sched.balance.
>
>
> I agree with your analysis. I had the same questions as well.
> I think that the tdq_transferable check is what saves the code from
> running into any problems. But it indeed would make sense for the code
> to understand that tdq_load includes a currently running, never
> transferable thread as well.
Things aren't quite as bad as I initially thought. cpu_search() does
look at tdq_transferable so sched_highest() should not return a cpu that
does not have a transferable thread at the time it was examined, so in
most cases the unnecessary lock/unlock shouldn't happen. The extra
check after the lock will catch the case where tdq_transferable went to
zero between when it was examined by cpu_search() and when we actually
grabbed the lock. Using a larger thresh value for SMT threads is still
a no-op, though.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201708241925.v7OJPDTT043392>