[Mesa-dev] [PATCH] gallium/util: don't let children of fork & exec inherit our thread affinity

[Marek Olšák]

Since only Blender and Firefox experience problems, they can be
blacklisted for this optimization, and then we can expand the
blacklist as we go.

Marek

On Fri, Sep 14, 2018 at 9:04 PM, Marek Olšák <maraeo at gmail.com> wrote:
> On Fri, Sep 14, 2018 at 4:53 AM, Michel Dänzer <michel at daenzer.net> wrote:
>> On 2018-09-13 8:56 p.m., Marek Olšák wrote:
>>> On Thu, Sep 13, 2018 at 11:48 AM, Michel Dänzer <michel at daenzer.net> wrote:
>>>> On 2018-09-13 2:40 a.m., Marek Olšák wrote:
>>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>>
>>>>> [...]
>>>>>
>>>>>  static void
>>>>> -util_init_cache_number(void)
>>>>> +util_init_thread_pinning(void)
>>>>>  {
>>>>>     /* Get a semi-random number. */
>>>>>     int64_t t = os_time_get_nano();
>>>>>     L3_cache_number = (t ^ (t >> 8) ^ (t >> 16));
>>>>> +
>>>>> +   /* Reset thread affinity for all children of fork and exec to prevent
>>>>
>>>> I don't think exec (which doesn't spawn a child, it replaces the current
>>>> process "image" with a new one) has anything to do with this.
>>>>
>>>>
>>>>> +    * spawned processes and threads from inheriting the current thread's
>>>>> +    * affinity.
>>>>
>>>> As the name implies, pthread_atfork only affects child processes spawned
>>>> with fork(), not new threads. As such, I'm afraid this won't help at
>>>> least for blender, which AFAICT doesn't call fork, it only spawns threads.
>>>
>>> All created threads and processes are just some variants of fork.
>>
>> fork(2) spawns a new process, not a new thread in the same process. Its
>> current implementation in glibc uses clone(2), which is also used for
>> spawning threads, but that's an implementation detail. The kernel still
>> has the dedicated fork system calls.
>>
>> pthread_atfork only affects new processes created with fork(2), not new
>> threads created in the same process.
>>
>>
>>> This patch is enough to stop inheriting thread affinity from X and
>>> gnome-shell to GL apps, to gcc run within an X terminal, etc.
>>> Everything within X inherits the thread affinity of X or gnome-shell,
>>> including gcc.
>>
>> FWIW, X clients are not descendants of the X server, so they must have
>> inherited it from something else.
>>
>> Anyway, now I understand the scope of this patch, thanks. But the commit
>> log and comment need to be fixed not to be misleading by talking about
>> exec and spawned threads. E.g.:
>>
>>  gallium/util: don't let child processes inherit our thread affinity
>>
>>    /* Prevent child processes from inheriting the current thread's
>>     * affinity.
>>
>>
>> That leaves:
>>
>>> +    * What happens if a driver is unloaded and the app creates a thread?
>>
>> I suppose the child process will likely crash, because the memory
>> address where util_set_full_cpu_affinity was located will either be
>> unmapped or have random other contents?
>>
>> At least in theory, there could also be an issue where the application
>> might have set its own thread affinity before calling fork, which would
>> be clobbered by util_set_full_cpu_affinity in the child process.
>>
>>
>> Last but not least, this doesn't solve the issue of apps such as
>> blender, which spawn their own worker threads after initializing OpenGL
>> (possibly not themselves directly, but via the toolkit or another
>> library; e.g. GTK+4 uses OpenGL by default), inheriting the thread affinity.
>>
>>
>> Due to these issues, setting the thread affinity needs to be disabled by
>> default, and only white-listed for applications where it's known safe
>> and beneficial. This sucks, but I'm afraid that's the reality until
>> there's better API available which allows solving these issues.
>
> We don't have the bandwidth to maintain whitelists. This will either
> have to be always on or always off.
>
> On the positive side, only Ryzens with multiple CCXs get all the
> benefits and disadvantages.
>
> Marek