On 03/06/2026 09:37, Philipp Stanner wrote:
On Tue, 2026-06-02 at 16:33 +0100, Tvrtko Ursulin wrote:
Commit
28c5bf28763d ("drm/sched: Disallow initializing entities with no schedulers")
failed to notice clearing of entity->rq in drm_sched_entity_init() is now
By clearing you also mean the setting to NULL?
I'd just use "initialization" consistently, like in the commit title.
It is initialized properly a bit lower down so clearing is I think more
accurate.
redundant and can be removed.
Given that entity->rq can now never be NULL, we also remove two impossible
checks, from drm_sched_entity_kill() and drm_sched_entity_flush()
respectively.
Similarly, we can also remove the !entity->rq check in
drm_sched_job_init(). And for the better, given that the error message, if
it ever triggered, would have dereferenced the yet un-initialized job->
sched (only initialized later in drm_sched_job_arm()). This appears to
have been theoretically broken ever since commit
56e449603f0a ("drm/sched: Convert the GPU scheduler to variable number of
run-queues")
.
Signed-off-by: Tvrtko Ursulin <[email protected]>
Cc: Christian König <[email protected]>
Cc: Danilo Krummrich <[email protected]>
Cc: Matthew Brost <[email protected]>
Cc: Philipp Stanner <[email protected]>
---
drivers/gpu/drm/scheduler/sched_entity.c | 11 ++---------
drivers/gpu/drm/scheduler/sched_main.c | 9 ---------
2 files changed, 2 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
b/drivers/gpu/drm/scheduler/sched_entity.c
index 4ebb513255ed..c51101ec70c1 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -129,7 +129,6 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
return -ENOMEM;
INIT_LIST_HEAD(&entity->list);
- entity->rq = NULL;
It would seem that has always been redundant because of the memset(0)
directly above.
True, ever since 1decbf6bb0b4 ("drm/sched: Fix entities with 0 rqs.").
Just that in this patch the redundant is focusing on:
entity->rq = NULL;
...
entity->rq = &sched_list[0]->rq;
Good enough or you want a respin and if so in what flavour?
Regards,
Tvrtko
entity->guilty = guilty;
entity->priority = priority;
entity->last_user = current->group_leader;
@@ -280,9 +279,6 @@ void drm_sched_entity_kill(struct drm_sched_entity *entity)
struct drm_sched_job *job;
struct dma_fence *prev;
- if (!entity->rq)
- return;
-
spin_lock(&entity->lock);
entity->stopped = true;
drm_sched_rq_remove_entity(entity->rq, entity);
@@ -329,14 +325,11 @@ EXPORT_SYMBOL(drm_sched_entity_kill);
*/
long drm_sched_entity_flush(struct drm_sched_entity *entity, long timeout)
{
- struct drm_gpu_scheduler *sched;
+ struct drm_gpu_scheduler *sched =
+ container_of(entity->rq, typeof(*sched), rq);
struct task_struct *last_user;
long ret = timeout;
- if (!entity->rq)
- return 0;
-
- sched = container_of(entity->rq, typeof(*sched), rq);
/*
* The client will not queue more jobs during this fini - consume
* existing queued ones, or discard them on SIGKILL.
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 818d3d4434b5..d2ca01b31ee4 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -588,15 +588,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
u32 credits, void *owner,
uint64_t drm_client_id)
{
- if (!entity->rq) {
- /* This will most likely be followed by missing frames
- * or worse--a blank screen--leave a trail in the
- * logs, so this can be debugged easier.
- */
- dev_err(job->sched->dev, "%s: entity has no rq!\n", __func__);
- return -ENOENT;
- }
-
if (unlikely(!credits)) {
pr_err("*ERROR* %s: credits cannot be 0!\n", __func__);
return -EINVAL;
But overall a very nice cleanup
P.
