From 366884b17fae7d2a7517eea60e64d6d6754fa9db Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 26 Apr 2016 19:29:34 +0200 Subject: drm: Give drm_agp_clear drm_legacy_ prefix It has a DRIVER_MODESET check to sure make it's not creating havoc for drm drivers. Make that clear in the name too. v2: Move misplaced hunk, spotted by 0day and Thierry. Cc: Thierry Reding Reviewed-by: Chris Wilson Acked-by: Alex Deucher Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461691808-12414-2-git-send-email-daniel.vetter@ffwll.ch --- include/drm/drm_agpsupport.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h index 193ef19dfc5c..b2d912670a7f 100644 --- a/include/drm/drm_agpsupport.h +++ b/include/drm/drm_agpsupport.h @@ -37,7 +37,7 @@ struct agp_memory *drm_agp_bind_pages(struct drm_device *dev, uint32_t type); struct drm_agp_head *drm_agp_init(struct drm_device *dev); -void drm_agp_clear(struct drm_device *dev); +void drm_legacy_agp_clear(struct drm_device *dev); int drm_agp_acquire(struct drm_device *dev); int drm_agp_acquire_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); @@ -93,7 +93,7 @@ static inline struct drm_agp_head *drm_agp_init(struct drm_device *dev) return NULL; } -static inline void drm_agp_clear(struct drm_device *dev) +static inline void drm_legacy_agp_clear(struct drm_device *dev) { } -- cgit v1.2.3 From 40647e45b92b1da599048332ec8fbd40d8d29457 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 27 Apr 2016 09:20:18 +0200 Subject: drm: Hide master MAP cleanup in drm_bufs.c And again make sure it's a no-op for modern drivers. Another case of dev->struct_mutex gone for modern drivers! Note that the entirety of the legacy addmap interface is now protected by DRIVER_MODESET. Note that just auditing kernel code is not enough, since userspace loves to set up legacy maps on it's own for various things - with ums userspace and kernel space share control over resources. v2: Also add a DRIVER_* check like for all other maps functions to really short-circuit the code. And give drm_legacy_rmmap used by the dev unregister code the same treatment. v3: - remove redundant return; (Alex, Chris) - don't special case nouveau with DRIVER_KMS_LEGACY_CONTEXT. v4: Again special case nouveau. The problem is not directly in the ddx, but that it calls dri1 functions from the X server. And those do call drmAddMap. Fixed only in commit b1a630b48210d6a3c44994fce1b73273000ace5c Author: Dave Airlie Date: Wed Nov 7 14:45:14 2012 +1000 nouveau: drop DRI1 device open interface. Acked-by: Alex Deucher Reviewed-by: Chris Wilson Cc: Alex Deucher Cc: Chris Wilson Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461741618-12679-1-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_bufs.c | 27 ++++++++++++++++++++++----- drivers/gpu/drm/drm_drv.c | 10 +--------- include/drm/drm_legacy.h | 4 +++- 3 files changed, 26 insertions(+), 15 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index e8a12a4fd400..9b34158c0f77 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -542,18 +542,35 @@ int drm_legacy_rmmap_locked(struct drm_device *dev, struct drm_local_map *map) } EXPORT_SYMBOL(drm_legacy_rmmap_locked); -int drm_legacy_rmmap(struct drm_device *dev, struct drm_local_map *map) +void drm_legacy_rmmap(struct drm_device *dev, struct drm_local_map *map) { - int ret; + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) && + drm_core_check_feature(dev, DRIVER_MODESET)) + return; mutex_lock(&dev->struct_mutex); - ret = drm_legacy_rmmap_locked(dev, map); + drm_legacy_rmmap_locked(dev, map); mutex_unlock(&dev->struct_mutex); - - return ret; } EXPORT_SYMBOL(drm_legacy_rmmap); +void drm_legacy_master_rmmaps(struct drm_device *dev, struct drm_master *master) +{ + struct drm_map_list *r_list, *list_temp; + + if (drm_core_check_feature(dev, DRIVER_MODESET)) + return; + + mutex_lock(&dev->struct_mutex); + list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head) { + if (r_list->master == master) { + drm_legacy_rmmap_locked(dev, r_list->map); + r_list = NULL; + } + } + mutex_unlock(&dev->struct_mutex); +} + /* The rmmap ioctl appears to be unnecessary. All mappings are torn down on * the last close of the device, and this is necessary for cleanup when things * exit uncleanly. Therefore, having userland manually remove mappings seems diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 55273f8f3acb..e1fb52d4f72c 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -121,19 +121,11 @@ static void drm_master_destroy(struct kref *kref) { struct drm_master *master = container_of(kref, struct drm_master, refcount); struct drm_device *dev = master->minor->dev; - struct drm_map_list *r_list, *list_temp; if (dev->driver->master_destroy) dev->driver->master_destroy(dev, master); - mutex_lock(&dev->struct_mutex); - list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head) { - if (r_list->master == master) { - drm_legacy_rmmap_locked(dev, r_list->map); - r_list = NULL; - } - } - mutex_unlock(&dev->struct_mutex); + drm_legacy_master_rmmaps(dev, master); idr_destroy(&master->magic_map); kfree(master->unique); diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index 3e698038dc7b..a5ef2c7e40f8 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -154,8 +154,10 @@ struct drm_map_list { int drm_legacy_addmap(struct drm_device *d, resource_size_t offset, unsigned int size, enum drm_map_type type, enum drm_map_flags flags, struct drm_local_map **map_p); -int drm_legacy_rmmap(struct drm_device *d, struct drm_local_map *map); +void drm_legacy_rmmap(struct drm_device *d, struct drm_local_map *map); int drm_legacy_rmmap_locked(struct drm_device *d, struct drm_local_map *map); +void drm_legacy_master_rmmaps(struct drm_device *dev, + struct drm_master *master); struct drm_local_map *drm_legacy_getsarea(struct drm_device *dev); int drm_legacy_mmap(struct file *filp, struct vm_area_struct *vma); -- cgit v1.2.3 From 1d2ac403ae3bfde7c50328ee0d39d3fb3d8d9823 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 26 Apr 2016 19:29:41 +0200 Subject: drm: Protect dev->filelist with its own mutex amdgpu gained dev->struct_mutex usage, and that's because it's walking the dev->filelist list. Protect that list with it's own lock to take one more step towards getting rid of struct_mutex usage in drivers once and for all. While doing the conversion I noticed that 2 debugfs files in i915 completely lacked appropriate locking. Fix that up too. v2: don't forget to switch to drm_gem_object_unreference_unlocked. Cc: Alex Deucher Reviewed-by: Alex Deucher Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461691808-12414-9-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 10 +++++----- drivers/gpu/drm/drm_drv.c | 1 + drivers/gpu/drm/drm_fops.c | 9 ++++++--- drivers/gpu/drm/drm_info.c | 4 ++-- drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++++++++-- include/drm/drmP.h | 1 + 6 files changed, 25 insertions(+), 12 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index fa6a27bff298..a087b9638cde 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -93,7 +93,7 @@ void amdgpu_gem_force_release(struct amdgpu_device *adev) struct drm_device *ddev = adev->ddev; struct drm_file *file; - mutex_lock(&ddev->struct_mutex); + mutex_lock(&ddev->filelist_mutex); list_for_each_entry(file, &ddev->filelist, lhead) { struct drm_gem_object *gobj; @@ -103,13 +103,13 @@ void amdgpu_gem_force_release(struct amdgpu_device *adev) spin_lock(&file->table_lock); idr_for_each_entry(&file->object_idr, gobj, handle) { WARN_ONCE(1, "And also active allocations!\n"); - drm_gem_object_unreference(gobj); + drm_gem_object_unreference_unlocked(gobj); } idr_destroy(&file->object_idr); spin_unlock(&file->table_lock); } - mutex_unlock(&ddev->struct_mutex); + mutex_unlock(&ddev->filelist_mutex); } /* @@ -769,7 +769,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data) struct drm_file *file; int r; - r = mutex_lock_interruptible(&dev->struct_mutex); + r = mutex_lock_interruptible(&dev->filelist_mutex); if (r) return r; @@ -793,7 +793,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data) spin_unlock(&file->table_lock); } - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&dev->filelist_mutex); return 0; } diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index e1fb52d4f72c..bff89226a344 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -590,6 +590,7 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver, spin_lock_init(&dev->buf_lock); spin_lock_init(&dev->event_lock); mutex_init(&dev->struct_mutex); + mutex_init(&dev->filelist_mutex); mutex_init(&dev->ctxlist_mutex); mutex_init(&dev->master_mutex); diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index c3d0aaac0669..7af7f8bcb355 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -297,9 +297,9 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor) } mutex_unlock(&dev->master_mutex); - mutex_lock(&dev->struct_mutex); + mutex_lock(&dev->filelist_mutex); list_add(&priv->lhead, &dev->filelist); - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&dev->filelist_mutex); #ifdef __alpha__ /* @@ -447,8 +447,11 @@ int drm_release(struct inode *inode, struct file *filp) DRM_DEBUG("open_count = %d\n", dev->open_count); - mutex_lock(&dev->struct_mutex); + mutex_lock(&dev->filelist_mutex); list_del(&file_priv->lhead); + mutex_unlock(&dev->filelist_mutex); + + mutex_lock(&dev->struct_mutex); if (file_priv->magic) idr_remove(&file_priv->master->magic_map, file_priv->magic); mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c index cbb4fc0fc969..5d469b2f26f4 100644 --- a/drivers/gpu/drm/drm_info.c +++ b/drivers/gpu/drm/drm_info.c @@ -174,7 +174,7 @@ int drm_clients_info(struct seq_file *m, void *data) /* dev->filelist is sorted youngest first, but we want to present * oldest first (i.e. kernel, servers, clients), so walk backwardss. */ - mutex_lock(&dev->struct_mutex); + mutex_lock(&dev->filelist_mutex); list_for_each_entry_reverse(priv, &dev->filelist, lhead) { struct task_struct *task; @@ -190,7 +190,7 @@ int drm_clients_info(struct seq_file *m, void *data) priv->magic); rcu_read_unlock(); } - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&dev->filelist_mutex); return 0; } diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 85933940fe9c..c298312f83f6 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -498,6 +498,10 @@ static int i915_gem_object_info(struct seq_file *m, void* data) seq_putc(m, '\n'); print_batch_pool_stats(m, dev_priv); + + mutex_unlock(&dev->struct_mutex); + + mutex_lock(&dev->filelist_mutex); list_for_each_entry_reverse(file, &dev->filelist, lhead) { struct file_stats stats; struct task_struct *task; @@ -518,8 +522,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data) print_file_stats(m, task ? task->comm : "", stats); rcu_read_unlock(); } - - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&dev->filelist_mutex); return 0; } @@ -2325,6 +2328,7 @@ static int i915_ppgtt_info(struct seq_file *m, void *data) else if (INTEL_INFO(dev)->gen >= 6) gen6_ppgtt_info(m, dev); + mutex_lock(&dev->filelist_mutex); list_for_each_entry_reverse(file, &dev->filelist, lhead) { struct drm_i915_file_private *file_priv = file->driver_priv; struct task_struct *task; @@ -2339,6 +2343,7 @@ static int i915_ppgtt_info(struct seq_file *m, void *data) idr_for_each(&file_priv->context_idr, per_file_ctx, (void *)(unsigned long)m); } + mutex_unlock(&dev->filelist_mutex); out_put: intel_runtime_pm_put(dev_priv); @@ -2374,6 +2379,8 @@ static int i915_rps_boost_info(struct seq_file *m, void *data) intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit), intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit), intel_gpu_freq(dev_priv, dev_priv->rps.max_freq)); + + mutex_lock(&dev->filelist_mutex); spin_lock(&dev_priv->rps.client_lock); list_for_each_entry_reverse(file, &dev->filelist, lhead) { struct drm_i915_file_private *file_priv = file->driver_priv; @@ -2396,6 +2403,7 @@ static int i915_rps_boost_info(struct seq_file *m, void *data) list_empty(&dev_priv->rps.mmioflips.link) ? "" : ", active"); seq_printf(m, "Kernel boosts: %d\n", dev_priv->rps.boosts); spin_unlock(&dev_priv->rps.client_lock); + mutex_unlock(&dev->filelist_mutex); return 0; } diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 5de4cff05ac9..7901768bb47c 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -769,6 +769,7 @@ struct drm_device { atomic_t buf_alloc; /**< Buffer allocation in progress */ /*@} */ + struct mutex filelist_mutex; struct list_head filelist; /** \name Memory management */ -- cgit v1.2.3