Hi,
On 10 January 2016 at 23:48, Laurent Pinchart
<laurent.pinchart at 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 at collabora.com>
Cheers,
Daniel
