> Eric Anholt <eric@xxxxxxxxxx> hat am 8. Oktober 2017 um 19:09 geschrieben: > > > Stefan Wahren <stefan.wahren@xxxxxxxx> writes: > > > Hi Eric, > > > >> Eric Anholt <eric@xxxxxxxxxx> hat am 6. Oktober 2017 um 21:42 geschrieben: > >> > >> > >> Stefan Wahren <stefan.wahren@xxxxxxxx> writes: > >> > >> > This fixes the race between vc4_overflow_mem_work and the init of the > >> > job lock. Otherwise we could trigger a NULL pointer dereference > >> > during VC4 binding. > >> > > >> > Link: https://github.com/anholt/linux/issues/114 > >> > Signed-off-by: Stefan Wahren <stefan.wahren@xxxxxxxx> > >> > Fixes: d5b1a78a772f ("drm/vc4: Add support for drawing 3D frames.") > >> > --- > >> > drivers/gpu/drm/vc4/vc4_gem.c | 1 - > >> > drivers/gpu/drm/vc4/vc4_irq.c | 1 + > >> > 2 files changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c > >> > index d0c6bfb..47d964f 100644 > >> > --- a/drivers/gpu/drm/vc4/vc4_gem.c > >> > +++ b/drivers/gpu/drm/vc4/vc4_gem.c > >> > @@ -1088,7 +1088,6 @@ vc4_gem_init(struct drm_device *dev) > >> > INIT_LIST_HEAD(&vc4->render_job_list); > >> > INIT_LIST_HEAD(&vc4->job_done_list); > >> > INIT_LIST_HEAD(&vc4->seqno_cb_list); > >> > - spin_lock_init(&vc4->job_lock); > >> > > >> > INIT_WORK(&vc4->hangcheck.reset_work, vc4_reset_work); > >> > setup_timer(&vc4->hangcheck.timer, > >> > diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c > >> > index 7d7af3a..d508d13 100644 > >> > --- a/drivers/gpu/drm/vc4/vc4_irq.c > >> > +++ b/drivers/gpu/drm/vc4/vc4_irq.c > >> > @@ -195,6 +195,7 @@ vc4_irq_preinstall(struct drm_device *dev) > >> > struct vc4_dev *vc4 = to_vc4_dev(dev); > >> > > >> > init_waitqueue_head(&vc4->job_wait_queue); > >> > + spin_lock_init(&vc4->job_lock); > >> > INIT_WORK(&vc4->overflow_mem_work, vc4_overflow_mem_work); > >> > > >> > /* Clear any pending interrupts someone might have left around > >> > >> Are you sure this is a fix? We don't attach the IRQ handler until V3D > >> bind, and vc4_gem_init happens before component_bind_all(), right? > > > > i agree i should have mark it as a RFC. But is it really impossible > > that vc4_overflow_mem_work is triggered before vc4_gem_init? > > As far as I can see, it gets queued from the IRQ handler, the IRQ > handler is added during V3D bind, and binding happens after GEM init. > > Hmm. If we fail out of component binding and try again, we'll end up > doing the job_wait_queue and overflow_mem_work initialization on the > same pointer twice. Will that cause any trouble? We cancel any pending > work during uninstall (V3D unbind path), but does drm_irq_uninstall() > make sure that the irq handler has finished? I cannot see an issue. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
