gfx: drv: Clean pending page flip events when device is closed
authorPauli Nieminen <pauli.nieminen@linux.intel.com>
Tue, 3 Jan 2012 15:36:44 +0000 (17:36 +0200)
committerMarkus Lehtonen <markus.lehtonen@linux.intel.com>
Tue, 3 Jul 2012 09:29:24 +0000 (12:29 +0300)
If userspace closes the device node with pending page flips driver keeps
a pointer to file private that is already freed. Following backtrace
shows the oops happening.

[ 643.828884] BUG: unable to handle kernel paging request at 6b6b6b6b
[ 643.835082] IP: [<c1224b6e>] __list_add+0x2a/0x5c
[ 643.888465] EIP is at __list_add+0x2a/0x5c
[ 643.943931] Call Trace:
[ 643.946376] [<c139c0d8>] psb_intel_flip_complete+0x71/0xcb
[ 643.951937] [<c139c1c8>] sync_callback+0x96/0x9f
[ 643.956631] [<c1371310>] PVRSRVCallbackOnSync+0x37/0xb7
[ 643.961934] [<c139c132>] ? psb_intel_flip_complete+0xcb/0xcb
[ 643.967673] [<c139c049>] psb_intel_crtc_page_flip+0xa1/0xbf
[ 643.973324] [<c1282f5d>] drm_mode_page_flip_ioctl+0x13f/0x190
[ 643.979151] [<c12778bf>] drm_ioctl+0x276/0x332

To cleanup pending events driver has to keep linked list of pending
flips in file private. When file is closed all events in pending flips
for the file are cleared.

To avoid race conditions dev->event_lock is used to protect pending
flips lists from parallel access.

Issue: KER-47
Signed-off-by: Pauli Nieminen <pauli.nieminen@linux.intel.com>
CC: Rahul Verman <rahul.verma@intel.com>
Acked-by: Sean V Kelley <sean.v.kelley@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
drivers/staging/mrst/drv/psb_drv.h
drivers/staging/mrst/drv/psb_page_flip.c
drivers/staging/mrst/drv/psb_page_flip.h
drivers/staging/mrst/imgv/psb_ttm_glue.c

index d324bfc..0e0a295 100644 (file)
@@ -983,6 +983,7 @@ struct drm_psb_private {
 struct psb_fpriv {
         int bcd_index;
        struct ttm_object_file *tfile;
+       struct list_head pending_flips;
 };
 
 struct psb_mmu_driver;
index 4edd223..1b0cec0 100644 (file)
@@ -24,6 +24,7 @@
 #include <linux/list.h>
 
 #include <drm/drmP.h>
+#include "psb_drv.h"
 #include "psb_fb.h"
 #include "psb_intel_reg.h"
 #include "psb_page_flip.h"
@@ -43,9 +44,26 @@ struct pending_flip {
        struct drm_pending_vblank_event *event;
        PVRSRV_KERNEL_MEM_INFO *old_mem_info;
        uint32_t offset;
+       struct list_head uncompleted;
        struct pvr_pending_sync pending_sync;
 };
 
+void
+psb_cleanup_pending_events(struct drm_device *dev, struct psb_fpriv *priv)
+{
+       struct drm_pending_vblank_event *e;
+       struct pending_flip *pending_flip;
+       unsigned long flags;
+
+       spin_lock_irqsave(&dev->event_lock, flags);
+       list_for_each_entry(pending_flip, &priv->pending_flips, uncompleted) {
+               e = pending_flip->event;
+               pending_flip->event = NULL;
+               e->base.destroy(&e->base);
+       }
+       spin_unlock_irqrestore(&dev->event_lock, flags);
+}
+
 static void
 send_page_flip_event(struct drm_device *dev, int pipe,
                     struct pending_flip *pending_flip)
@@ -54,11 +72,12 @@ send_page_flip_event(struct drm_device *dev, int pipe,
        struct timeval now;
        unsigned long flags;
 
-       if (!pending_flip->event)
-               return;
-
        spin_lock_irqsave(&dev->event_lock, flags);
 
+       if (!pending_flip->event)
+               goto unlock;
+
+       list_del(&pending_flip->uncompleted);
        e = pending_flip->event;
        do_gettimeofday(&now);
        e->event.sequence = drm_vblank_count(dev, pipe);
@@ -68,6 +87,7 @@ send_page_flip_event(struct drm_device *dev, int pipe,
                        &e->base.file_priv->event_list);
        wake_up_interruptible(&e->base.file_priv->event_wait);
 
+unlock:
        spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
@@ -194,6 +214,8 @@ psb_intel_crtc_page_flip(struct drm_crtc *crtc,
        struct psb_framebuffer *psbfb = to_psb_fb(fb);
        PVRSRV_KERNEL_MEM_INFO *new_fb_mem_info, *current_fb_mem_info;
        struct pending_flip *new_pending_flip;
+       struct psb_fpriv *priv;
+       unsigned long flags;
 
        if (psb_get_meminfo_by_handle(psbfb->hKernelMemInfo, &new_fb_mem_info))
                return -EINVAL;
@@ -206,6 +228,15 @@ psb_intel_crtc_page_flip(struct drm_crtc *crtc,
        new_pending_flip->event = event;
        new_pending_flip->offset = psbfb->offset;
 
+       if (event) {
+               spin_lock_irqsave(&crtc->dev->event_lock, flags);
+               priv = psb_fpriv(event->base.file_priv);
+               list_add(&new_pending_flip->uncompleted, &priv->pending_flips);
+               spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
+       } else {
+               INIT_LIST_HEAD(&new_pending_flip->uncompleted);
+       }
+
        current_fb_mem_info = get_fb_meminfo(crtc->fb);
 
        crtc->fb = fb;
index 41c8b6b..4867431 100644 (file)
@@ -26,5 +26,7 @@ void psb_intel_crtc_process_vblank(struct drm_crtc *crtc);
 int psb_intel_crtc_page_flip(struct drm_crtc *crtc,
                              struct drm_framebuffer *fb,
                              struct drm_pending_vblank_event *event);
+void psb_cleanup_pending_events(struct drm_device *dev,
+                               struct psb_fpriv *priv);
 
 #endif /* _PSB_PAGE_FLIP_H_ */
index af5abfd..e598b21 100644 (file)
@@ -22,6 +22,7 @@
 
 #include <drm/drmP.h>
 #include "psb_drv.h"
+#include "psb_page_flip.h"
 #include "psb_ttm_userobj_api.h"
 #include <linux/io.h>
 #include "psb_msvdx.h"
@@ -66,6 +67,8 @@ int psb_open(struct inode *inode, struct file *filp)
 
        psb_fp->tfile = ttm_object_file_init(dev_priv->tdev,
                                             PSB_FILE_OBJECT_HASH_ORDER);
+       INIT_LIST_HEAD(&psb_fp->pending_flips);
+
        if (unlikely(psb_fp->tfile == NULL))
                goto out_err1;
 
@@ -91,15 +94,19 @@ out_err0:
 int psb_release(struct inode *inode, struct file *filp)
 {
        struct drm_file *file_priv;
+       struct drm_device *dev;
        struct psb_fpriv *psb_fp;
        struct drm_psb_private *dev_priv;
        struct msvdx_private *msvdx_priv;
        int ret;
        file_priv = (struct drm_file *) filp->private_data;
        psb_fp = psb_fpriv(file_priv);
-       dev_priv = psb_priv(file_priv->minor->dev);
+       dev = file_priv->minor->dev;
+       dev_priv = psb_priv(dev);
        msvdx_priv = (struct msvdx_private *)dev_priv->msvdx_private;
 
+       psb_cleanup_pending_events(dev, psb_fp);
+
        /*cleanup for msvdx*/
        if (msvdx_priv->tfile == psb_fpriv(file_priv)->tfile) {
                msvdx_priv->fw_status = 0;