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

[Michel Dänzer]

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.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer