<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
Am 27.10.23 um 10:22 schrieb Boris Brezillon:<br>
<blockquote type="cite" cite="mid:20231027102237.0cdb85af@collabora.com">
<pre class="moz-quote-pre" wrap="">On Fri, 27 Oct 2023 09:44:13 +0200
Christian König <a class="moz-txt-link-rfc2396E" href="mailto:christian.koenig@amd.com"><christian.koenig@amd.com></a> wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Am 27.10.23 um 09:39 schrieb Boris Brezillon:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On Fri, 27 Oct 2023 09:35:01 +0200
Christian König<a class="moz-txt-link-rfc2396E" href="mailto:christian.koenig@amd.com"><christian.koenig@amd.com></a> wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Am 27.10.23 um 09:32 schrieb Boris Brezillon:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On Fri, 27 Oct 2023 09:22:12 +0200
Christian König<a class="moz-txt-link-rfc2396E" href="mailto:christian.koenig@amd.com"><christian.koenig@amd.com></a> wrote:
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
+ /**
+ * @update_job_credits: Called once the scheduler is considering this
+ * job for execution.
+ *
+ * Drivers may use this to update the job's submission credits, which is
+ * useful to e.g. deduct the number of native fences which have been
+ * signaled meanwhile.
+ *
+ * The callback must either return the new number of submission credits
+ * for the given job, or zero if no update is required.
+ *
+ * This callback is optional.
+ */
+ u32 (*update_job_credits)(struct drm_sched_job *sched_job);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Why do we need an extra callback for this?
Just document that prepare_job() is allowed to reduce the number of
credits the job might need.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">->prepare_job() is called only once if the returned fence is NULL, but
we need this credit-update to happen every time a job is considered for
execution by the scheduler.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">But the job is only considered for execution once. How do you see that
this is called multiple times?
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Nope, it's not. If drm_sched_can_queue() returns false, the scheduler
will go look for another entity that has a job ready for execution, and
get back to this entity later, and test drm_sched_can_queue() again.
Basically, any time drm_sched_can_queue() is called, the job credits
update should happen, so we have an accurate view of how many credits
this job needs.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Well, that is the handling which I already rejected because it creates
unfairness between processes. When you consider the credits needed
*before* scheduling jobs with a lower credit count are always preferred
over jobs with a higher credit count.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
My bad, it doesn't pick another entity when an entity with a
ready job that doesn't fit the queue is found, it just bails out from
drm_sched_rq_select_entity_rr() and returns NULL (AKA: no ready entity
found). But we still want to update the job credits before checking if
the job fits or not (next time this entity is tested).</pre>
</blockquote>
<br>
Why? I only see a few possibility here:<br>
<br>
1. You need to wait for submissions to the same scheduler to finish
before the one you want to push to the ring.<br>
This case can be avoided by trying to cast the dependency fences
to drm_sched_fences and looking if they are already scheduled.<br>
<br>
2. You need to wait for submissions to a different scheduler
instance and in this case you should probably return that as
dependency instead.<br>
<br>
So to me it looks like when prepare_job() is called because it is
selected as next job for submission you should already know how many
credits it needs.<br>
<br>
<span style="white-space: pre-wrap">
</span>
<blockquote type="cite" cite="mid:20231027102237.0cdb85af@collabora.com">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">What you can do is to look at the credits of a job *after* it was picked
up for scheduling so that you can scheduler more jobs.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Sure, but then you might further delay your job if something made it
smaller (ie. native fences got signaled) between ->prepare_job() and
drm_sched_can_queue(). And any new drm_sched_can_queue() test would
just see the old credits value.
Out of curiosity, what are you worried about with this optional
->update_job_credits() call in the drm_sched_can_queue() path? Is the
if (sched->update_job_credits) overhead considered too high for drivers
that don't need it?</pre>
</blockquote>
<br>
Since the dma_fences are also used for resource management the
scheduler is vital for correct system operation.<br>
<br>
We had massively problems because people tried to over-optimize the
dma_fence handling which lead to very hard to narrow down memory
corruptions.<br>
<br>
So for every change you come up here you need to have a very very
good justification. And just saving a bit size of your ring buffer
is certainly not one of them.<br>
<br>
Regards,<br>
Christian.<br>
</body>
</html>