On 08/01/16 11:29, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursu...@intel.com>Purpose is to catch places which iterate the object VMA list without holding the big lock. Implemented by open coding list_for_each_entry to make the macro compatible with existing call sites. Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com> Cc: Daniel Vetter <daniel.vet...@ffwll.ch> --- drivers/gpu/drm/i915/i915_debugfs.c | 8 ++++---- drivers/gpu/drm/i915/i915_drv.h | 6 ++++++ drivers/gpu/drm/i915/i915_gem.c | 24 ++++++++++++------------ drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- drivers/gpu/drm/i915/i915_gem_shrinker.c | 2 +- drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++-- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 714a45cf8a51..d7c2a3201161 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -117,7 +117,7 @@ static u64 i915_gem_obj_total_ggtt_size(struct drm_i915_gem_object *obj) u64 size = 0; struct i915_vma *vma; - list_for_each_entry(vma, &obj->vma_list, vma_link) { + i915_gem_obj_for_each_vma(vma, obj) { if (i915_is_ggtt(vma->vm) && drm_mm_node_allocated(&vma->node)) size += vma->node.size; @@ -155,7 +155,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) obj->madv == I915_MADV_DONTNEED ? " purgeable" : ""); if (obj->base.name) seq_printf(m, " (name: %d)", obj->base.name); - list_for_each_entry(vma, &obj->vma_list, vma_link) { + i915_gem_obj_for_each_vma(vma, obj) { if (vma->pin_count > 0) pin_count++; } @@ -164,7 +164,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) seq_printf(m, " (display)"); if (obj->fence_reg != I915_FENCE_REG_NONE) seq_printf(m, " (fence: %d)", obj->fence_reg); - list_for_each_entry(vma, &obj->vma_list, vma_link) { + i915_gem_obj_for_each_vma(vma, obj) { seq_printf(m, " (%sgtt offset: %08llx, size: %08llx", i915_is_ggtt(vma->vm) ? "g" : "pp", vma->node.start, vma->node.size); @@ -342,7 +342,7 @@ static int per_file_stats(int id, void *ptr, void *data) stats->shared += obj->base.size; if (USES_FULL_PPGTT(obj->base.dev)) { - list_for_each_entry(vma, &obj->vma_list, vma_link) { + i915_gem_obj_for_each_vma(vma, obj) { struct i915_hw_ppgtt *ppgtt; if (!drm_mm_node_allocated(&vma->node)) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b77a5d84eac2..0406a020dfcc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2852,6 +2852,12 @@ struct drm_i915_gem_object *i915_gem_object_create_from_data( void i915_gem_free_object(struct drm_gem_object *obj); void i915_gem_vma_destroy(struct i915_vma *vma); +#define i915_gem_obj_for_each_vma(vma, obj) \ + for (WARN_ON_ONCE(!mutex_is_locked(&(obj)->base.dev->struct_mutex)), \ + vma = list_first_entry(&(obj)->vma_list, typeof(*vma), vma_link);\ + &vma->vma_link != (&(obj)->vma_list); \ + vma = list_next_entry(vma, vma_link)) +
Unfortunately error capture is not happy with this approach. Can't even see that error capture attempts to grab the mutex anywhere.
So what? Drop the idea or add a "doing error capture" flag somewhere? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
