Re: [Intel-gfx] [RFC PATCH 04/20] drm/sched: Convert drm scheduler to use a work queue rather than kthread

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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