On 11/01/2023 19:40, Matthew Brost wrote:
On Wed, Jan 11, 2023 at 08:50:37AM +0000, Tvrtko Ursulin wrote:
[snip]
This example is where it would hurt on large systems. Imagine only an even
wider media transcode card...
Second example is only a single engine class used (3d desktop?) but with a
bunch of not-runnable jobs queued and waiting on a fence to signal. Implicit
or explicit dependencies doesn't matter. Then the fence signals and call
backs run. N work items get scheduled, but they all submit to the same HW
engine. So we end up with:
/-- wi1 --\
/ .. .. \
cb --+--- wi.. ---+-- rq1 -- .. -- rqN
\ .. .. /
\-- wiN --/
All that we have achieved is waking up N CPUs to contend on the same lock
and effectively insert the job into the same single HW queue. I don't see
any positives there.
I've said this before, the CT channel in practice isn't going to be full
so the section of code protected by the mutex is really, really small.
The mutex really shouldn't ever have contention. Also does a mutex spin
for small period of time before going to sleep? I seem to recall some
type of core lock did this, if we can use a lock that spins for short
period of time this argument falls apart.
This argument already fell apart when we established it's the system_wq
and not the unbound one. So a digression only - it did not fall apart
because of the CT channel not being ever congested, there would still be
the question of what's the point to wake up N cpus when there is a
single work channel in the backend.
You would have been able to bypass all that by inserting work items
directly, not via the scheduler workers. I thought that was what Jason
was implying when he mentioned that a better frontend/backend drm
scheduler split was considered at some point.
Because for 1:1:1, where GuC is truly 1, it does seem it would work
better if that sort of a split would enable you to queue directly into
the backend bypassing the kthread/worker / wait_on wake_up dance.
Would that work? From drm_sched_entity_push_job directly to the backend
- not waking up but *calling* the equivalent of drm_sched_main.
Right, that's all solid I think. My takeaway is that frontend priority
sorting and that stuff isn't needed and that is okay. And that there are
multiple options to maybe improve drm scheduler, like the fore mentioned
making it deal with out of order, or split into functional components, or
split frontend/backend what you suggested. For most of them cost vs benefit
is more or less not completely clear, neither how much effort was invested
to look into them.
One thing I missed from this explanation is how drm_scheduler per engine
class interferes with the high level concepts. And I did not manage to pick
up on what exactly is the TDR problem in that case. Maybe the two are one
and the same.
Bottom line is I still have the concern that conversion to kworkers has an
opportunity to regress. Possibly more opportunity for some Xe use cases than
to affect other vendors, since they would still be using per physical engine
/ queue scheduler instances.
We certainly don't want to affect other vendors but I haven't yet heard
any push back from other vendors. I don't think speculating about
potential problems is helpful.
I haven't had any push back on the drm cgroup controller either. :D
And to put my money where my mouth is I will try to put testing Xe inside
the full blown ChromeOS environment in my team plans. It would probably also
be beneficial if Xe team could take a look at real world behaviour of the
extreme transcode use cases too. If the stack is ready for that and all. It
would be better to know earlier rather than later if there is a fundamental
issue.
We don't have a media UMD yet it will be tough to test at this point in
time. Also not sure when Xe is going to be POR for a Chrome product
either so porting Xe into ChromeOS likely isn't a top priority for your
team. I know from experience that porting things into ChromeOS isn't
trivial as I've support several of these efforts. Not saying don't do
this just mentioning the realities of what you are suggesting.
I know, I only said I'd put it in the plans, not that it will happen
tomorrow.
Regards,
Tvrtko
