| Message ID | 1465930269-7883-10-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 14, 2016 at 08:51:04PM +0200, Daniel Vetter wrote: > Simplifies cleanup, and there's no reason drivers should ever care > about authmagic at all - it's all handled in the core. > > And with that, Ladies and Gentlemen, it's time to pop the champagen > and celebrate: dev->struct_mutex is now officially gone from modern > drivers, and if a driver is using gem_free_object_unlocked and doesn't > do anything else silly it's positively impossible to ever touch > dev->struct_mutex at runtime, anywhere. Still keen to know what the fuss is all about. > Well except for the mutex_init on driver load ;-) > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Wed, Jun 15, 2016 at 12:54:24PM +0100, Chris Wilson wrote: > On Tue, Jun 14, 2016 at 08:51:04PM +0200, Daniel Vetter wrote: > > Simplifies cleanup, and there's no reason drivers should ever care > > about authmagic at all - it's all handled in the core. > > > > And with that, Ladies and Gentlemen, it's time to pop the champagen > > and celebrate: dev->struct_mutex is now officially gone from modern > > drivers, and if a driver is using gem_free_object_unlocked and doesn't > > do anything else silly it's positively impossible to ever touch > > dev->struct_mutex at runtime, anywhere. > > Still keen to know what the fuss is all about. I hate sprawling locks, and go all ocd about them. There's not really anything else. It also makes reviewing changes painful since you have to re-read every code that somehow interacts with your lock. I much prefer locking where each part fits into my brain, and that one is of pityful capacity ;-) -Daniel > > > Well except for the mutex_init on driver load ;-) > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index e37d657dbf5f..94761e4bd1e5 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -49,7 +49,7 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) struct drm_auth *auth = data; int ret = 0; - mutex_lock(&dev->struct_mutex); + mutex_lock(&dev->master_mutex); if (!file_priv->magic) { ret = idr_alloc(&file_priv->master->magic_map, file_priv, 1, 0, GFP_KERNEL); @@ -57,7 +57,7 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) file_priv->magic = ret; } auth->magic = file_priv->magic; - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&dev->master_mutex); DRM_DEBUG("%u\n", auth->magic); @@ -82,13 +82,13 @@ int drm_authmagic(struct drm_device *dev, void *data, DRM_DEBUG("%u\n", auth->magic); - mutex_lock(&dev->struct_mutex); + mutex_lock(&dev->master_mutex); file = idr_find(&file_priv->master->magic_map, auth->magic); if (file) { file->authenticated = 1; idr_replace(&file_priv->master->magic_map, NULL, auth->magic); } - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&dev->master_mutex); return file ? 0 : -EINVAL; } @@ -117,7 +117,7 @@ static struct drm_master *drm_master_create(struct drm_device *dev) * @dev: The associated device. * @fpriv: File private identifying the client. * - * This function must be called with dev::struct_mutex held. + * This function must be called with dev::master_mutex held. * Returns negative error code on failure. Zero on success. */ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) @@ -248,12 +248,10 @@ void drm_master_release(struct drm_file *file_priv) struct drm_device *dev = file_priv->minor->dev; struct drm_master *master = file_priv->master; - mutex_lock(&dev->struct_mutex); + mutex_lock(&dev->master_mutex); if (file_priv->magic) idr_remove(&file_priv->master->magic_map, file_priv->magic); - mutex_unlock(&dev->struct_mutex); - mutex_lock(&dev->master_mutex); if (!file_priv->is_master) goto out_unlock;
Simplifies cleanup, and there's no reason drivers should ever care about authmagic at all - it's all handled in the core. And with that, Ladies and Gentlemen, it's time to pop the champagen and celebrate: dev->struct_mutex is now officially gone from modern drivers, and if a driver is using gem_free_object_unlocked and doesn't do anything else silly it's positively impossible to ever touch dev->struct_mutex at runtime, anywhere. Well except for the mutex_init on driver load ;-) Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/drm_auth.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)