Re: [PATCH v2 1/9] drm/sched: Convert drm scheduler to use a work queue rather than kthread

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 17.08.23 um 14:48 schrieb Danilo Krummrich:
On 8/17/23 15:35, Christian König wrote:
Am 17.08.23 um 13:13 schrieb Danilo Krummrich:
On 8/17/23 07:33, Christian König wrote:
[SNIP]
My proposal would be to just keep the hw_submission_limit (maybe rename it to submission_unit_limit) and add a submission_units field to struct drm_sched_job. By default a jobs submission_units field would be 0 and the scheduler would behave the exact same way as it does now.

Accordingly, jobs with submission_units > 1 would contribute more than one unit to the submission_unit_limit.

What do you think about that?

I think you are approaching this from the completely wrong side.

First of all, thanks for keeping up the discussion - I appreciate it. Some more comments / questions below.


See the UAPI needs to be stable, so you need a maximum job size otherwise it can happen that a combination of large and small submissions work while a different combination doesn't.

How is this related to the uAPI being stable? What do you mean by 'stable' in this context?

Stable is in you don't get indifferent behavior, not stable is in the sense of backward compatibility. Sorry for the confusing wording :)


The Nouveau uAPI allows userspace to pass EXEC jobs by supplying the ring ID (channel), in-/out-syncs and a certain amount of indirect push buffers. The amount of IBs per job is limited by the amount of IBs fitting into the ring. Just to be clear, when I say 'job size' I mean the amount of IBs per job.

Well that more or less sounds identical to all other hardware I know of, e.g. AMD, Intel and the different ARM chips seem to all work like this. But on those drivers the job size limit is not the ring size, but rather a fixed value (at least as far as I know).


Maybe I should also mention that the rings we are talking about are software rings managed by a firmware scheduler. We can have an arbitrary amount of software rings and even multiple ones per FD.

Given a constant ring size I really don't see why I should limit the maximum amount of IBs userspace can push per job just to end up with a hw_submission_limit > 1.

For example, let's just assume the ring can take 128 IBs, why would I limit userspace to submit just e.g. 16 IBs at a time, such that the hw_submission_limit becomes 8?

Well the question is what happens when you have two submissions back to back which use more than halve of the ring buffer?

I only see two possible outcomes:
1. You return -EBUSY (or similar) error code indicating the the hw can't receive more commands.
2. Wait on previously pushed commands to be executed.
(3. Your driver crash because you accidentally overwrite stuff in the ring buffer which is still executed. I just assume that's prevented).

Resolution #1 with -EBUSY is actually something the UAPI should not do, because your UAPI then depends on the specific timing of submissions which is a really bad idea.

Resolution #2 is usually bad because it forces the hw to run dry between submission and so degrade performance.


What is the advantage of doing that, rather than letting userspace submit *up to* 128 IBs per job and just letting the scheduler push IBs to the ring as long as there's actually space left on the ring?

Predictable behavior I think. Basically you want organize things so that the hw is at least kept busy all the time without depending on actual timing.



So what you usually do, and this is driver independent because simply a requirement of the UAPI, is that you say here that's my maximum job size as well as the number of submission which should be pushed to the hw at the same time. And then get the resulting ring size by the product of the two.

Given the above, how is that a requirement of the uAPI?

The requirement of the UAPI is actually pretty simple: You should get consistent results, independent of the timing (at least as long as you don't do stuff in parallel).

Otherwise you can run into issues when on a certain configuration stuff suddenly runs faster or slower than expected. In other words you should not depend on that stuff finishes in a certain amount of time.



That the ring in this use case can't be fully utilized is not a draw back, this is completely intentional design which should apply to all drivers independent of the vendor.

Why wouldn't we want to fully utilize the ring size?

As far as I know everybody restricts the submission size to something fixed which is at least smaller than halve the ring size to avoid the problems mentioned above.

Regards,
Christian.


- Danilo



Besides all that, you said that filling up the ring just enough to not let the HW run dry rather than filling it up entirely is desirable. Why do you think so? I tend to think that in most cases it shouldn't make difference.

That results in better scheduling behavior. It's mostly beneficial if you don't have a hw scheduler, but as far as I can see there is no need to pump everything to the hw as fast as possible.

Regards,
Christian.


- Danilo


Regards,
Christian.

Because one really is the minimum if you want to do work at all, but as you mentioned above a job limit of one can let the ring run dry.

In the end my proposal comes down to tracking the actual size of a job rather than just assuming a pre-defined maximum job size, and hence a dynamic job limit.

I don't think this would hurt the scheduler granularity. In fact, it should even contribute to the desire of not letting the ring run dry even better. Especially for sequences of small jobs, where the current implementation might wrongly assume the ring is already full although actually there would still be enough space left.


Christian.



Otherwise your scheduler might just overwrite the ring buffer by pushing things to fast.

Christian.


Given that, it seems like it would be better to let the scheduler keep track of empty ring "slots" instead, such that the scheduler can deceide whether a subsequent job will still fit on the ring and if not re-evaluate once a previous job finished. Of course each submitted job would be required to carry the number of slots it requires on the ring.

What to you think of implementing this as alternative flow control mechanism? Implementation wise this could be a union with the existing hw_submission_limit.

- Danilo


A problem with this design is currently a drm_gpu_scheduler uses a kthread for submission / job cleanup. This doesn't scale if a large number of drm_gpu_scheduler are used. To work around the scaling issue,
use a worker rather than kthread for submission / job cleanup.

v2:
   - (Rob Clark) Fix msm build
   - Pass in run work queue
v3:
   - (Boris) don't have loop in worker
v4:
   - (Tvrtko) break out submit ready, stop, start helpers into own patch

Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>













[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux