On 08/08/2010 10:37 AM, Chris Wilson wrote:
Dave Airlie: I might be missing something, but what stops the race with something reopening while we are in lastclose now? The global_mutex which appears to fill this role is only taken inside the drm_stub_open() and not drm_open() itself. As that mutex appears to serve no other role, retask it to serialise open/lastclose.
Hmm. But, if used this way, drm_global_mutex needs to protect *all* accesses to dev::open_count, what is count_clock used for? Can't it be removed? /Thomas
Signed-off-by: Chris Wilson<chris@xxxxxxxxxxxxxxxxxx> --- drivers/gpu/drm/drm_fops.c | 35 ++++++++++++++++++----------------- 1 files changed, 18 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 6367961..71bcd1b 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -123,27 +123,30 @@ int drm_open(struct inode *inode, struct file *filp) struct drm_device *dev = NULL; int minor_id = iminor(inode); struct drm_minor *minor; - int retcode = 0; + int retcode = -ENODEV; + mutex_lock(&drm_global_mutex); minor = idr_find(&drm_minors_idr, minor_id); if (!minor) - return -ENODEV; + goto err; if (!(dev = minor->dev)) - return -ENODEV; + goto err; retcode = drm_open_helper(inode, filp, dev); - if (!retcode) { - atomic_inc(&dev->counts[_DRM_STAT_OPENS]); - spin_lock(&dev->count_lock); - if (!dev->open_count++) { - spin_unlock(&dev->count_lock); - retcode = drm_setup(dev); - goto out; - } + if (retcode) + goto err; + + atomic_inc(&dev->counts[_DRM_STAT_OPENS]); + spin_lock(&dev->count_lock); + if (!dev->open_count++) { spin_unlock(&dev->count_lock); - } -out: + retcode = drm_setup(dev); + } else + spin_unlock(&dev->count_lock); +err: + mutex_unlock(&drm_global_mutex); + if (!retcode) { mutex_lock(&dev->struct_mutex); if (minor->type == DRM_MINOR_LEGACY) { @@ -178,7 +181,6 @@ int drm_stub_open(struct inode *inode, struct file *filp) DRM_DEBUG("\n"); - mutex_lock(&drm_global_mutex); minor = idr_find(&drm_minors_idr, minor_id); if (!minor) goto out; @@ -198,8 +200,8 @@ int drm_stub_open(struct inode *inode, struct file *filp) } fops_put(old_fops); + ret = 0; out: - mutex_unlock(&drm_global_mutex); return err; } @@ -474,8 +476,6 @@ int drm_release(struct inode *inode, struct file *filp) struct drm_device *dev = file_priv->minor->dev; int retcode = 0; - mutex_lock(&drm_global_mutex); - DRM_DEBUG("open_count = %d\n", dev->open_count); if (dev->driver->preclose) @@ -569,6 +569,7 @@ int drm_release(struct inode *inode, struct file *filp) * End inline drm_release */ + mutex_lock(&drm_global_mutex); atomic_inc(&dev->counts[_DRM_STAT_CLOSES]); spin_lock(&dev->count_lock); if (!--dev->open_count) {
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel