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

Tue Oct 30 23:39:33 UTC 2018

śr., 31 paź 2018 o 00:23 Marek Olšák <maraeo at gmail.com> napisał(a):
>
> On Tue, Oct 30, 2018 at 7:11 PM Gustaw Smolarczyk <wielkiegie at gmail.com> wrote:
>>
>> wt., 30 paź 2018 o 23:55 Marek Olšák <maraeo at gmail.com> napisał(a):
>> >
>> > On Tue, Oct 30, 2018 at 6:32 PM Gustaw Smolarczyk <wielkiegie at gmail.com> wrote:
>> >>
>> >> wt., 30 paź 2018, 23:01 Marek Olšák <maraeo at gmail.com>:
>> >>>
>> >>> On Mon, Oct 29, 2018 at 12:43 PM Michel Dänzer <michel at daenzer.net> wrote:
>> >>>>
>> >>>> On 2018-10-28 11:27 a.m., Gustaw Smolarczyk wrote:
>> >>>> > pon., 17 wrz 2018 o 18:24 Michel Dänzer <michel at daenzer.net> napisał(a):
>> >>>> >>
>> >>>> >> On 2018-09-15 3:04 a.m., Marek Olšák wrote:
>> >>>> >>> On Fri, Sep 14, 2018 at 4:53 AM, Michel Dänzer <michel at daenzer.net> wrote:
>> >>>> >>>>
>> >>>> >>>> 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.
>> >>>> >>
>> >>>> >> In other words, only people who spent relatively large amounts of money
>> >>>> >> for relatively high-end CPUs will be affected (I'm sure they'll be glad
>> >>>> >> to know that "common people" aren't affected. ;). Affected applications
>> >>>> >> will see their performance decreased by a factor of 2-8 (the number of
>> >>>> >> CCXs in the CPU).
>> >>>> >>
>> >>>> >> OTOH, only a relatively small number of games will get a significant
>> >>>> >> benefit from the thread affinity, and the benefit will be smaller than a
>> >>>> >> factor of 2. This cannot justify risking a performance drop of up to a
>> >>>> >> factor of 8, no matter how small the risk.
>> >>>> >>
>> >>>> >> Therefore, the appropriate mechanism is a whitelist.
>> >>>> >
>> >>>> > Hi,
>> >>>> >
>> >>>> > What was the conclusion of this discussion? I don't see any
>> >>>> > whitelist/blacklist for this feature.
>> >>>> >
>> >>>> > I have just tested blender and it still renders on only a single CCX
>> >>>> > on mesa from git master. Also, there is a bug report that suggests
>> >>>> > this regressed performance in at least one game [1].
>> >>>>
>> >>>> I hooked up that bug report to the 18.3 blocker bug.
>> >>>>
>> >>>>
>> >>>> > If you think enabling it by default is the way to go, we should also
>> >>>> > implement a blacklist so that it can be turned off in such cases.
>> >>>>
>> >>>> I stand by my opinion that a white-list is appropriate, not a
>> >>>> black-list. It's pretty much the same as mesa_glthread.
>> >>>
>> >>>
>> >>> So you are saying that gallium multithreading show be slower than singlethreading by default.
>> >>>
>> >>> Marek
>> >>
>> >>
>> >> Hi Marek,
>> >>
>> >> The Ryzen optimization helps a lot of applications (mostly games) and improves their performance, mostly because of the reduced cost of communication between application's GL API thread and driver's pipe/winsys threads.
>> >>
>> >> However, not all of the applications respond in the same way. The thread affinity management is hacky, by which I mean that this mechanism was not meant to mess with application threads from within library's threads. As an example, blender's threads, which use OpenGL "by accident", are forced to use the same CCX as the main gallium/winsys thread, even if they are many and want to work on as many CCXs as are possible. The thread that starts using GL spawns many more threads that don't use GL at all, and the current atfork mechanism doesn't help.
>> >>
>> >> The current mechanism of tweaking thread affinities doesn't work universally with all Linux applications. We need a mechanism of tweaking this behavior, either through a whitelist or through a blacklist. As any application using OpenGL can be affected, I would opt towards disabling this by default and providing a whitelist for applications we know it would help.
>> >
>> >
>> > Multithreading is slower than singlethreading. You are pretty much saying that Mesa shouldn't use multithreading by default. Just think about that. NVIDIA would destroy us at all price points. I'm not that insane to disable the thread pinning by default.
>>
>> I have no idea what you are saying. The current implementation of
>> multithreading doesn't work well with all applications on Ryzen, due
>> to kernel scheduling application and gallium/winsys threads on
>> different CCXs. However, thread pinning is not a universal solution
>> for this problem. We can't express what we want at the moment (i.e.
>> schedule a couple of threads next to each other, regardless of where
>> they really are) and the current work-around of using thread
>> affinities hurts reals applications (like Blender).
>
>
> As far as we know, it hurts *only* Blender. Don't say "real applications" if you don't have a good list of applications.

Is that an excuse to hurt applications you don't know about? Should we
ban all applications that use OpenGL and not adhere to the Safe
Threading Behavior == use OpenGL from one thread and spawn more
threads from this thread afterwards?

>
>>
>> >
>> > The thread affinity API is very bad for this, but it's the only thing we have. Linux lacks a proper thread management API for the Zen architecture and the Linux scheduler does the worst thing for Ryzen (it puts threads on different CCXs), so the scheduler always works against us. It makes Ryzen as slow as possible.
>> >
>> > Only Blender is affected negatively and there is a patch for it. Blender is open source, so it can just reset the thread affinity for new threads by itself, or set a thread affinity that works best with Ryzen. Mesa can contain the workaround out of courtesy.
>>
>> What is the logistics of Blender delivering a release with this patch?
>> Won't people complain about very bad Ryzen performance after upgrading
>> to mesa 18.3 before they upgrade to a new Blender release?
>
>
> It's OK if people complain and file a bug. We'll tell them to upgrade Blender.
>
>>
>>
>> Also, what is the most important, Blender is just a single
>> application. Any application can have a similarly bad behavior
>> regarding thread affinity fixing of mesa. Do you expect any
>> application to "fix itself" for thread affinity shenanigans of the GL
>> library?
>>
>> The answer is that libraries shouldn't really mess with thread
>> affinities. If they do, there should better be a mechanism for
>> disabling such a behavior for applications that, justifiably, expect a
>> system-default behavior.
>
>
> Applications can change the thread affinity. Applications can also call setenv() to disable the Mesa behavior. Users can also disable the Mesa behavior by setting an environment variable or adding a new entry into /usr/share/drirc.d/.

You confuse things that are completely different. Applications don't
set mesa-related environments variables by accident - if they do they
either know what they do or have to manage the outcome by themselves.
Setting affinity is not something applications should worry about -
this is a standard OS behavior that should help them manage their own
threads.

Also, I don't think Blender changes thread affinity at all. It just
spawns threads from a thread that first does OpenGL calls - these
calls change this thread affinity due to the Ryzen optimization, and
this affinity setting is inherited by the child threads.

Regards,
Gustaw Smolarczyk

>
> New applications hurt by the Mesa behavior will be handled on a case-by-case basis.
>
> Marek