| Message ID | 1359044448-3861-3-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jan 24, 2013 at 10:20 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > We don't need to hold onto mode_config.mutex any more to keep the fb > objects around. And locking dev->struct_mutex is also not required, > since omap_gem_describe only reads data anyway. And for a debug > interface it's better to grab fewer locks in case the driver is > deadlocked already ... > > The only thing we need is to hold onto mode_config.fb_lock to ensure > the user-created fbs don't disappear. The fbcon fb doesn't need any > protection, since it lives as long as the driver (and so the debugfs > files) itself. And if the teardown/setup isn't following the right > sequence grabbing locks won't prevent a NULL deref on priv->fbdev if > the fb is not yet (or no longer) there. agreed Reviewed-by: Rob Clark <rob@ti.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/staging/omapdrm/omap_debugfs.c | 14 -------------- > 1 file changed, 14 deletions(-) > > diff --git a/drivers/staging/omapdrm/omap_debugfs.c b/drivers/staging/omapdrm/omap_debugfs.c > index e95540b..2bf0664 100644 > --- a/drivers/staging/omapdrm/omap_debugfs.c > +++ b/drivers/staging/omapdrm/omap_debugfs.c > @@ -57,17 +57,6 @@ static int fb_show(struct seq_file *m, void *arg) > struct drm_device *dev = node->minor->dev; > struct omap_drm_private *priv = dev->dev_private; > struct drm_framebuffer *fb; > - int ret; > - > - ret = mutex_lock_interruptible(&dev->mode_config.mutex); > - if (ret) > - return ret; > - > - ret = mutex_lock_interruptible(&dev->struct_mutex); > - if (ret) { > - mutex_unlock(&dev->mode_config.mutex); > - return ret; > - } > > seq_printf(m, "fbcon "); > omap_framebuffer_describe(priv->fbdev->fb, m); > @@ -82,9 +71,6 @@ static int fb_show(struct seq_file *m, void *arg) > } > mutex_unlock(&dev->mode_config.fb_lock); > > - mutex_unlock(&dev->struct_mutex); > - mutex_unlock(&dev->mode_config.mutex); > - > return 0; > } > > -- > 1.7.10.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/staging/omapdrm/omap_debugfs.c b/drivers/staging/omapdrm/omap_debugfs.c index e95540b..2bf0664 100644 --- a/drivers/staging/omapdrm/omap_debugfs.c +++ b/drivers/staging/omapdrm/omap_debugfs.c @@ -57,17 +57,6 @@ static int fb_show(struct seq_file *m, void *arg) struct drm_device *dev = node->minor->dev; struct omap_drm_private *priv = dev->dev_private; struct drm_framebuffer *fb; - int ret; - - ret = mutex_lock_interruptible(&dev->mode_config.mutex); - if (ret) - return ret; - - ret = mutex_lock_interruptible(&dev->struct_mutex); - if (ret) { - mutex_unlock(&dev->mode_config.mutex); - return ret; - } seq_printf(m, "fbcon "); omap_framebuffer_describe(priv->fbdev->fb, m); @@ -82,9 +71,6 @@ static int fb_show(struct seq_file *m, void *arg) } mutex_unlock(&dev->mode_config.fb_lock); - mutex_unlock(&dev->struct_mutex); - mutex_unlock(&dev->mode_config.mutex); - return 0; }
We don't need to hold onto mode_config.mutex any more to keep the fb objects around. And locking dev->struct_mutex is also not required, since omap_gem_describe only reads data anyway. And for a debug interface it's better to grab fewer locks in case the driver is deadlocked already ... The only thing we need is to hold onto mode_config.fb_lock to ensure the user-created fbs don't disappear. The fbcon fb doesn't need any protection, since it lives as long as the driver (and so the debugfs files) itself. And if the teardown/setup isn't following the right sequence grabbing locks won't prevent a NULL deref on priv->fbdev if the fb is not yet (or no longer) there. Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/staging/omapdrm/omap_debugfs.c | 14 -------------- 1 file changed, 14 deletions(-)