[PATCH] drm/amdgpu/fence: Fix oops due to non-matching drm_sched init/fini

[Alex Deucher]

On Tue, Jan 31, 2023 at 9:32 AM Guilherme G. Piccoli
<gpiccoli at igalia.com> wrote:
>
> On 31/01/2023 10:58, Chen, Guchun wrote:
> > Hi Christian,
> >
> > Do you think if it makes sense that we can set 'ring->sched.ready' to be true in each ring init, even if before executing/setting up drm_sched_init in amdgpu_device_init_schedulers? As 'ready' is a member of gpu scheduler structure.
> >
> > Regards,
> > Guchun
> >
>
> Hi folks, thanks a lot for the feedback so far, much appreciated!
>
> I'm feeling a bit confused specially since there seems to be 2
> orthogonal (yet related) topics being discussed; let me try to summarize
> my understanding and we can then further discuss better:
>
> (a) The first problem is the one addressed in the patch - how to prevent
> drm_sched_fini() to get called if drm_sched_init() wasn't called?
>
> I've proposed sched.name, seems Christian prefers sched.ops, correct?
>
>
> (b) We can't use sched.ready, which would make sense...but amdgpu
> overrides its meaning, the driver manipulates this value for its own
> purposes of tracking ring init, or something like that.
>
> This is the tangential topic: what should we do here? My understanding
> of Alex's message is that we could have a "ready" field in the ring
> structure and stop messing with sched.ready - does it make sense Alex?

Yes, I think so.  The tricky part will be figuring out which current
sched.ready checks are checking for the scheduler being ready vs.
whether the ring itself is ready.

>
> Guchun / Christian, does it also make sense for you?
>
>
> Regarding (a), I could re-submit having s/sched.name/sched.ops, no
> biggies, I tested both to be fair, before sending...I just chose name
> but any field that is proper initialized on drm_sched_init() would work.

Yeah, I think ops is fine.  You could even use sched.ready after we
clean up the use of that in the driver.  There are already a bunch of
places where we check sched.ready to check if the scheduler really is
ready.

Alex

>
> Thanks,
>
>
> Guilherme