Re: [RFC 5/8] drm/fence: add fence to drm_pending_event

From: Daniel Vetter
Date: Fri Apr 15 2016 - 15:31:45 EST


On Fri, Apr 15, 2016 at 11:59:00AM -0700, Gustavo Padovan wrote:
> 2016-04-15 Daniel Vetter <daniel@xxxxxxxx>:
> > On Thu, Apr 14, 2016 at 06:29:38PM -0700, Gustavo Padovan wrote:
> > > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> > > index aeef58e..38def49 100644
> > > --- a/drivers/gpu/drm/drm_fops.c
> > > +++ b/drivers/gpu/drm/drm_fops.c
> > > @@ -801,8 +801,9 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e)
> > > {
> > > assert_spin_locked(&dev->event_lock);
> > >
> > > - if (!e->file_priv) {
> > > - e->destroy(e);
> > > + if (!e->file_priv || !e->event) {
> >
> > This would be a bug: e->file_priv != NULL iff e->event != NULL. How did
> > this happen?
>
> Not sure now. But I needed this to prevent a crash, I don't have logs of
> it anymore, I'll check this again.

There was a massive irc discussion with Daniel Stone, so I'll try to
summarize it here. There are 3 possible cases:

e->file_priv == NULL && e->event == NULL:
This is a drm_event without a drm_event. Probably e->fence is set, if not
then it's a completeley useless thing (but not forbidden).

e->file_priv != NULL && e->event != NULL:
drm_event with an event for the given file_priv attached.

e->file_priv == NULL && e->event != NULL:
Above case, but with the file_priv closed and unlinked from the event.

The 4th case, which is the only case things will change with the above
hunk, is not allowed. If you hit it, there's a bug somewhere.

This is all completely idependent of e->fence, which this patch adds.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch