| Message ID | 1452463323-4499-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
In general, looks good. Two things though. 1) vc4_drv.h looks like it ended up in the wrong patch. 2) Should be able to nuke also struct vmw_fpriv::fence_events and struct vmw_event_fence_action::fpriv_head? /Thomas On 01/10/2016 11:02 PM, Daniel Vetter wrote: > Again since the drm core takes care of event unlinking/disarming this > is now just needless code. > > v2: I've completely missed eaction->fpriv_head and all the related > code. We need to nuke that too to avoid accidentally deferencing the > freed-up vmwgfx-private fpriv. > > Cc: Thomas Hellström <thellstrom@vmware.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/vc4/vc4_drv.h | 1 - > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 -------- > drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 48 ----------------------------------- > drivers/gpu/drm/vmwgfx/vmwgfx_fence.h | 2 -- > 4 files changed, 61 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h > index 080865ec2bae..4c734d087d7f 100644 > --- a/drivers/gpu/drm/vc4/vc4_drv.h > +++ b/drivers/gpu/drm/vc4/vc4_drv.h > @@ -376,7 +376,6 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *arg); > extern struct platform_driver vc4_crtc_driver; > int vc4_enable_vblank(struct drm_device *dev, unsigned int crtc_id); > void vc4_disable_vblank(struct drm_device *dev, unsigned int crtc_id); > -void vc4_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file); > int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg); > > /* vc4_debugfs.c */ > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > index c49812b80dd0..c04c4d1e2f3e 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c > @@ -971,15 +971,6 @@ static int vmw_driver_unload(struct drm_device *dev) > return 0; > } > > -static void vmw_preclose(struct drm_device *dev, > - struct drm_file *file_priv) > -{ > - struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv); > - struct vmw_private *dev_priv = vmw_priv(dev); > - > - vmw_event_fence_fpriv_gone(dev_priv->fman, &vmw_fp->fence_events); > -} > - > static void vmw_postclose(struct drm_device *dev, > struct drm_file *file_priv) > { > @@ -1500,7 +1491,6 @@ static struct drm_driver driver = { > .master_set = vmw_master_set, > .master_drop = vmw_master_drop, > .open = vmw_driver_open, > - .preclose = vmw_preclose, > .postclose = vmw_postclose, > .set_busid = drm_pci_set_busid, > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > index e0edf149d9d5..bb95bd20d415 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > @@ -71,7 +71,6 @@ struct vmw_user_fence { > */ > struct vmw_event_fence_action { > struct vmw_fence_action action; > - struct list_head fpriv_head; > > struct drm_pending_event *event; > struct vmw_fence_obj *fence; > @@ -808,44 +807,6 @@ int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data, > } > > /** > - * vmw_event_fence_fpriv_gone - Remove references to struct drm_file objects > - * > - * @fman: Pointer to a struct vmw_fence_manager > - * @event_list: Pointer to linked list of struct vmw_event_fence_action objects > - * with pointers to a struct drm_file object about to be closed. > - * > - * This function removes all pending fence events with references to a > - * specific struct drm_file object about to be closed. The caller is required > - * to pass a list of all struct vmw_event_fence_action objects with such > - * events attached. This function is typically called before the > - * struct drm_file object's event management is taken down. > - */ > -void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman, > - struct list_head *event_list) > -{ > - struct vmw_event_fence_action *eaction; > - struct drm_pending_event *event; > - unsigned long irq_flags; > - > - while (1) { > - spin_lock_irqsave(&fman->lock, irq_flags); > - if (list_empty(event_list)) > - goto out_unlock; > - eaction = list_first_entry(event_list, > - struct vmw_event_fence_action, > - fpriv_head); > - list_del_init(&eaction->fpriv_head); > - event = eaction->event; > - eaction->event = NULL; > - spin_unlock_irqrestore(&fman->lock, irq_flags); > - event->destroy(event); > - } > -out_unlock: > - spin_unlock_irqrestore(&fman->lock, irq_flags); > -} > - > - > -/** > * vmw_event_fence_action_seq_passed > * > * @action: The struct vmw_fence_action embedded in a struct > @@ -879,7 +840,6 @@ static void vmw_event_fence_action_seq_passed(struct vmw_fence_action *action) > *eaction->tv_usec = tv.tv_usec; > } > > - list_del_init(&eaction->fpriv_head); > eaction->event = NULL; > drm_send_event_locked(dev, eaction->event); > spin_unlock_irqrestore(&dev->event_lock, irq_flags); > @@ -901,10 +861,6 @@ static void vmw_event_fence_action_cleanup(struct vmw_fence_action *action) > struct vmw_fence_manager *fman = fman_from_fence(eaction->fence); > unsigned long irq_flags; > > - spin_lock_irqsave(&fman->lock, irq_flags); > - list_del(&eaction->fpriv_head); > - spin_unlock_irqrestore(&fman->lock, irq_flags); > - > vmw_fence_obj_unreference(&eaction->fence); > kfree(eaction); > } > @@ -1001,10 +957,6 @@ int vmw_event_fence_action_queue(struct drm_file *file_priv, > eaction->tv_sec = tv_sec; > eaction->tv_usec = tv_usec; > > - spin_lock_irqsave(&fman->lock, irq_flags); > - list_add_tail(&eaction->fpriv_head, &vmw_fp->fence_events); > - spin_unlock_irqrestore(&fman->lock, irq_flags); > - > vmw_fence_obj_add_action(fence, &eaction->action); > > return 0; > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h > index 8be6c29f5eb5..83ae301ee141 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h > @@ -116,8 +116,6 @@ extern int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv); > extern int vmw_fence_event_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv); > -extern void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman, > - struct list_head *event_list); > extern int vmw_event_fence_action_queue(struct drm_file *filee_priv, > struct vmw_fence_obj *fence, > struct drm_pending_event *event,
On Sun, Jan 10, 2016 at 11:17 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote: > In general, looks good. Two things though. > > 1) vc4_drv.h looks like it ended up in the wrong patch. Oops, will fix. > 2) Should be able to nuke also struct vmw_fpriv::fence_events and struct > vmw_event_fence_action::fpriv_head? Yes. I already remove fpriv_head, but fence_events should go to. Plus a few other now unused local variable. -Daniel > /Thomas > > On 01/10/2016 11:02 PM, Daniel Vetter wrote: >> Again since the drm core takes care of event unlinking/disarming this >> is now just needless code. >> >> v2: I've completely missed eaction->fpriv_head and all the related >> code. We need to nuke that too to avoid accidentally deferencing the >> freed-up vmwgfx-private fpriv. >> >> Cc: Thomas Hellström <thellstrom@vmware.com> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> --- >> drivers/gpu/drm/vc4/vc4_drv.h | 1 - >> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 -------- >> drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 48 ----------------------------------- >> drivers/gpu/drm/vmwgfx/vmwgfx_fence.h | 2 -- >> 4 files changed, 61 deletions(-) >> >> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h >> index 080865ec2bae..4c734d087d7f 100644 >> --- a/drivers/gpu/drm/vc4/vc4_drv.h >> +++ b/drivers/gpu/drm/vc4/vc4_drv.h >> @@ -376,7 +376,6 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *arg); >> extern struct platform_driver vc4_crtc_driver; >> int vc4_enable_vblank(struct drm_device *dev, unsigned int crtc_id); >> void vc4_disable_vblank(struct drm_device *dev, unsigned int crtc_id); >> -void vc4_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file); >> int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg); >> >> /* vc4_debugfs.c */ >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c >> index c49812b80dd0..c04c4d1e2f3e 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c >> @@ -971,15 +971,6 @@ static int vmw_driver_unload(struct drm_device *dev) >> return 0; >> } >> >> -static void vmw_preclose(struct drm_device *dev, >> - struct drm_file *file_priv) >> -{ >> - struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv); >> - struct vmw_private *dev_priv = vmw_priv(dev); >> - >> - vmw_event_fence_fpriv_gone(dev_priv->fman, &vmw_fp->fence_events); >> -} >> - >> static void vmw_postclose(struct drm_device *dev, >> struct drm_file *file_priv) >> { >> @@ -1500,7 +1491,6 @@ static struct drm_driver driver = { >> .master_set = vmw_master_set, >> .master_drop = vmw_master_drop, >> .open = vmw_driver_open, >> - .preclose = vmw_preclose, >> .postclose = vmw_postclose, >> .set_busid = drm_pci_set_busid, >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c >> index e0edf149d9d5..bb95bd20d415 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c >> @@ -71,7 +71,6 @@ struct vmw_user_fence { >> */ >> struct vmw_event_fence_action { >> struct vmw_fence_action action; >> - struct list_head fpriv_head; >> >> struct drm_pending_event *event; >> struct vmw_fence_obj *fence; >> @@ -808,44 +807,6 @@ int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data, >> } >> >> /** >> - * vmw_event_fence_fpriv_gone - Remove references to struct drm_file objects >> - * >> - * @fman: Pointer to a struct vmw_fence_manager >> - * @event_list: Pointer to linked list of struct vmw_event_fence_action objects >> - * with pointers to a struct drm_file object about to be closed. >> - * >> - * This function removes all pending fence events with references to a >> - * specific struct drm_file object about to be closed. The caller is required >> - * to pass a list of all struct vmw_event_fence_action objects with such >> - * events attached. This function is typically called before the >> - * struct drm_file object's event management is taken down. >> - */ >> -void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman, >> - struct list_head *event_list) >> -{ >> - struct vmw_event_fence_action *eaction; >> - struct drm_pending_event *event; >> - unsigned long irq_flags; >> - >> - while (1) { >> - spin_lock_irqsave(&fman->lock, irq_flags); >> - if (list_empty(event_list)) >> - goto out_unlock; >> - eaction = list_first_entry(event_list, >> - struct vmw_event_fence_action, >> - fpriv_head); >> - list_del_init(&eaction->fpriv_head); >> - event = eaction->event; >> - eaction->event = NULL; >> - spin_unlock_irqrestore(&fman->lock, irq_flags); >> - event->destroy(event); >> - } >> -out_unlock: >> - spin_unlock_irqrestore(&fman->lock, irq_flags); >> -} >> - >> - >> -/** >> * vmw_event_fence_action_seq_passed >> * >> * @action: The struct vmw_fence_action embedded in a struct >> @@ -879,7 +840,6 @@ static void vmw_event_fence_action_seq_passed(struct vmw_fence_action *action) >> *eaction->tv_usec = tv.tv_usec; >> } >> >> - list_del_init(&eaction->fpriv_head); >> eaction->event = NULL; >> drm_send_event_locked(dev, eaction->event); >> spin_unlock_irqrestore(&dev->event_lock, irq_flags); >> @@ -901,10 +861,6 @@ static void vmw_event_fence_action_cleanup(struct vmw_fence_action *action) >> struct vmw_fence_manager *fman = fman_from_fence(eaction->fence); >> unsigned long irq_flags; >> >> - spin_lock_irqsave(&fman->lock, irq_flags); >> - list_del(&eaction->fpriv_head); >> - spin_unlock_irqrestore(&fman->lock, irq_flags); >> - >> vmw_fence_obj_unreference(&eaction->fence); >> kfree(eaction); >> } >> @@ -1001,10 +957,6 @@ int vmw_event_fence_action_queue(struct drm_file *file_priv, >> eaction->tv_sec = tv_sec; >> eaction->tv_usec = tv_usec; >> >> - spin_lock_irqsave(&fman->lock, irq_flags); >> - list_add_tail(&eaction->fpriv_head, &vmw_fp->fence_events); >> - spin_unlock_irqrestore(&fman->lock, irq_flags); >> - >> vmw_fence_obj_add_action(fence, &eaction->action); >> >> return 0; >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h >> index 8be6c29f5eb5..83ae301ee141 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h >> @@ -116,8 +116,6 @@ extern int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data, >> struct drm_file *file_priv); >> extern int vmw_fence_event_ioctl(struct drm_device *dev, void *data, >> struct drm_file *file_priv); >> -extern void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman, >> - struct list_head *event_list); >> extern int vmw_event_fence_action_queue(struct drm_file *filee_priv, >> struct vmw_fence_obj *fence, >> struct drm_pending_event *event, >
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 080865ec2bae..4c734d087d7f 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -376,7 +376,6 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *arg); extern struct platform_driver vc4_crtc_driver; int vc4_enable_vblank(struct drm_device *dev, unsigned int crtc_id); void vc4_disable_vblank(struct drm_device *dev, unsigned int crtc_id); -void vc4_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file); int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg); /* vc4_debugfs.c */ diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index c49812b80dd0..c04c4d1e2f3e 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -971,15 +971,6 @@ static int vmw_driver_unload(struct drm_device *dev) return 0; } -static void vmw_preclose(struct drm_device *dev, - struct drm_file *file_priv) -{ - struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv); - struct vmw_private *dev_priv = vmw_priv(dev); - - vmw_event_fence_fpriv_gone(dev_priv->fman, &vmw_fp->fence_events); -} - static void vmw_postclose(struct drm_device *dev, struct drm_file *file_priv) { @@ -1500,7 +1491,6 @@ static struct drm_driver driver = { .master_set = vmw_master_set, .master_drop = vmw_master_drop, .open = vmw_driver_open, - .preclose = vmw_preclose, .postclose = vmw_postclose, .set_busid = drm_pci_set_busid, diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c index e0edf149d9d5..bb95bd20d415 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c @@ -71,7 +71,6 @@ struct vmw_user_fence { */ struct vmw_event_fence_action { struct vmw_fence_action action; - struct list_head fpriv_head; struct drm_pending_event *event; struct vmw_fence_obj *fence; @@ -808,44 +807,6 @@ int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data, } /** - * vmw_event_fence_fpriv_gone - Remove references to struct drm_file objects - * - * @fman: Pointer to a struct vmw_fence_manager - * @event_list: Pointer to linked list of struct vmw_event_fence_action objects - * with pointers to a struct drm_file object about to be closed. - * - * This function removes all pending fence events with references to a - * specific struct drm_file object about to be closed. The caller is required - * to pass a list of all struct vmw_event_fence_action objects with such - * events attached. This function is typically called before the - * struct drm_file object's event management is taken down. - */ -void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman, - struct list_head *event_list) -{ - struct vmw_event_fence_action *eaction; - struct drm_pending_event *event; - unsigned long irq_flags; - - while (1) { - spin_lock_irqsave(&fman->lock, irq_flags); - if (list_empty(event_list)) - goto out_unlock; - eaction = list_first_entry(event_list, - struct vmw_event_fence_action, - fpriv_head); - list_del_init(&eaction->fpriv_head); - event = eaction->event; - eaction->event = NULL; - spin_unlock_irqrestore(&fman->lock, irq_flags); - event->destroy(event); - } -out_unlock: - spin_unlock_irqrestore(&fman->lock, irq_flags); -} - - -/** * vmw_event_fence_action_seq_passed * * @action: The struct vmw_fence_action embedded in a struct @@ -879,7 +840,6 @@ static void vmw_event_fence_action_seq_passed(struct vmw_fence_action *action) *eaction->tv_usec = tv.tv_usec; } - list_del_init(&eaction->fpriv_head); eaction->event = NULL; drm_send_event_locked(dev, eaction->event); spin_unlock_irqrestore(&dev->event_lock, irq_flags); @@ -901,10 +861,6 @@ static void vmw_event_fence_action_cleanup(struct vmw_fence_action *action) struct vmw_fence_manager *fman = fman_from_fence(eaction->fence); unsigned long irq_flags; - spin_lock_irqsave(&fman->lock, irq_flags); - list_del(&eaction->fpriv_head); - spin_unlock_irqrestore(&fman->lock, irq_flags); - vmw_fence_obj_unreference(&eaction->fence); kfree(eaction); } @@ -1001,10 +957,6 @@ int vmw_event_fence_action_queue(struct drm_file *file_priv, eaction->tv_sec = tv_sec; eaction->tv_usec = tv_usec; - spin_lock_irqsave(&fman->lock, irq_flags); - list_add_tail(&eaction->fpriv_head, &vmw_fp->fence_events); - spin_unlock_irqrestore(&fman->lock, irq_flags); - vmw_fence_obj_add_action(fence, &eaction->action); return 0; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h index 8be6c29f5eb5..83ae301ee141 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h @@ -116,8 +116,6 @@ extern int vmw_fence_obj_unref_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); extern int vmw_fence_event_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); -extern void vmw_event_fence_fpriv_gone(struct vmw_fence_manager *fman, - struct list_head *event_list); extern int vmw_event_fence_action_queue(struct drm_file *filee_priv, struct vmw_fence_obj *fence, struct drm_pending_event *event,
Again since the drm core takes care of event unlinking/disarming this is now just needless code. v2: I've completely missed eaction->fpriv_head and all the related code. We need to nuke that too to avoid accidentally deferencing the freed-up vmwgfx-private fpriv. Cc: Thomas Hellström <thellstrom@vmware.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/vc4/vc4_drv.h | 1 - drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 -------- drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 48 ----------------------------------- drivers/gpu/drm/vmwgfx/vmwgfx_fence.h | 2 -- 4 files changed, 61 deletions(-)