On 12/10/19 6:32 PM, Christian König wrote:
Maybe make this "num_sched_list > 1 ? sched_list : NULL" to avoid
accidentally dereferencing a stale pointer to the stack.
Do you mean "num_sched_list >= 1 ? sched_list : NULL"
No, the entity->sched_list field should be NULL when num_sched_list==1.
When num_sched_list==1 the entity->sched_list shouldn't be used and we
can use a dummy list on the stack as parameter. But we should set the
pointer to NULL in this case just to make sure that nobody is
dereferencing it.
Okay I understand now.
- if(!entity->sched_list)
- return -ENOMEM;
init_completion(&entity->entity_idle);
-
- for (i = 0; i < num_sched_list; i++)
- entity->sched_list[i] = sched_list[i];
-
if (num_sched_list)
That check can actually be dropped as well. We return -EINVAL when
the num_sched_list is zero.
This one took me some time to understand. So we don't really return
-EINVAL if num_sched_list == 0
if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
return -EINVAL;
This is coming from below patch. Are we suppose to tolerate IPs with
uninitialized sched so that ctx creation dosn't return error ?
Yes, exactly. That's intentionally this way.
GPU reset sometimes resulted in schedulers being disabled because we
couldn't re-init them. In this case we had entities with an empty
scheduler list.
That can't happen any more after you make the scheduler arrays
constant, but I would stick with that behavior.
So I will keep the check.
Regards,
Christian.
Regards,
Nirmoy
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
