Am 17.08.23 um 13:13 schrieb Danilo Krummrich:
On 8/17/23 07:33, Christian König wrote:
[SNIP]
The hardware seems to work mostly the same for all vendors, but you
somehow seem to think that filling the ring is somehow beneficial
which is really not the case as far as I can see.
I think that's a misunderstanding. I'm not trying to say that it is
*always* beneficial to fill up the ring as much as possible. But I
think it is under certain circumstances, exactly those circumstances I
described for Nouveau.
As far as I can see this is not correct for Nouveau either.
As mentioned, in Nouveau the size of a job is only really limited by
the ring size, which means that one job can (but does not necessarily)
fill up the whole ring. We both agree that this is inefficient,
because it potentially results into the HW run dry due to
hw_submission_limit == 1.
I recognize you said that one should define hw_submission_limit and
adjust the other parts of the equation accordingly, the options I see
are:
(1) Increase the ring size while keeping the maximum job size.
(2) Decrease the maximum job size while keeping the ring size.
(3) Let the scheduler track the actual job size rather than the
maximum job size.
(1) results into potentially wasted ring memory, because we're not
always reaching the maximum job size, but the scheduler assumes so.
(2) results into more IOCTLs from userspace for the same amount of IBs
and more jobs result into more memory allocations and more work being
submitted to the workqueue (with Matt's patches).
(3) doesn't seem to have any of those draw backs.
What would be your take on that?
Actually, if none of the other drivers is interested into a more
precise way of keeping track of the ring utilization, I'd be totally
fine to do it in a driver specific way. However, unfortunately I don't
see how this would be possible.
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.
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.
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.
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.
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>
