Hi,
On 10 January 2016 at 23:48, Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
On Saturday 09 January 2016 14:28:46 Daniel Vetter wrote:
@@ -353,18 +354,16 @@ static void drm_events_release(struct drm_file *file_priv) { struct drm_device *dev = file_priv->minor->dev; struct drm_pending_event *e, *et;
struct drm_pending_vblank_event *v, *vt; unsigned long flags; spin_lock_irqsave(&dev->event_lock, flags);/* Remove pending flips */list_for_each_entry_safe(v, vt, &dev->vblank_event_list, base.link)if (v->base.file_priv == file_priv) {list_del(&v->base.link);drm_vblank_put(dev, v->pipe);v->base.destroy(&v->base);}Where does this code go ?
It doesn't: instead of deleting the events, the helpers to either cancel or send the event just notice that file_priv is NULL and bail out early.
/* Unlink pending events */list_for_each_entry_safe(e, et, &file_priv->pending_event_list,pending_link) {list_del(&e->pending_link);e->file_priv = NULL;}
file_priv gets reset here ...
@@ -736,7 +736,10 @@ void drm_event_cancel_free(struct drm_device *dev, { unsigned long flags; spin_lock_irqsave(&dev->event_lock, flags);
p->file_priv->event_space += p->event->length;
if (p->file_priv) {p->file_priv->event_space += p->event->length;list_del(&p->pending_link);} spin_unlock_irqrestore(&dev->event_lock, flags); p->destroy(p);
Allowing us to DTRT here ...
void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) { assert_spin_locked(&dev->event_lock);
if (!e->file_priv) {I don't think this could happen before this patch as e->file_priv is dereferenced below, and I don't see anything in this patch that makes the condition possible.
e->destroy(e);return;}
... and now here.
This looks good to me, and a good sight better than doing it in every driver. Still drowning in stuff after three weeks off though, so the best I can offer for the series right now is: Acked-by: Daniel Stone daniels@collabora.com
Cheers, Daniel