Message ID | 20160202125805.GK15851@nuc-i3427.alporthouse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 02/02/16 12:58, Chris Wilson wrote: > On Tue, Feb 02, 2016 at 12:10:19PM +0000, Tvrtko Ursulin wrote: >> On 02/02/16 11:36, Chris Wilson wrote: >>> #define list_for_each_entry_check(pos, list, member, lock) \ >>> for (typeof(*lock) == typeof(struct mutex) ? assert_lockdep_held(lock) : assert_spin_locked(lock); \ >>> pos = list_first_entry(head, typeof(*pos), member); \ >>> &pos->member != (head); \ >>> pos = list_next_entry(pos, member)) >>> >>> #define i915_gem_object_for_each_vma(vma, obj) \ >>> list_for_each_entry_check(vma, &(obj)->vma_list, vma_link, &(obj)->base.dev->struct_mutex) >>> >>> could be lifted easily, and makes i915_gem_object_for_each_vma() easier >>> to understand (i.e. serves better as documentation). >> >> Don't know, needs buy-in from the relevant people, and depends on >> how useful to outside of i915 it would be. And if you can make >> lockdep_assert_held work in this context. > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 4004a7cf8db4..931684a74533 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -140,7 +140,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, obj_link) { > + i915_gem_object_for_each_vma(vma, obj) { > if (vma->pin_count > 0) > pin_count++; > } > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 31487aa11977..574e45ab43cb 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3565,4 +3565,36 @@ int remap_io_mapping(struct vm_area_struct *vma, > unsigned long addr, unsigned long pfn, unsigned long size, > struct io_mapping *iomap); > > +static __always_inline void __list_check_mutex(struct mutex *lock) > +{ > + lockdep_assert_held(lock); > +} > + > +static __always_inline void __list_check_spin(struct spinlock *lock) > +{ > + assert_spin_locked(lock); > +} > + > +static __always_inline void __list_check_bug(void *_lock) > +{ > + BUILD_BUG_ON("unknown lock type"); > +} > + > +#define __list_check(lock) \ > + ({ __builtin_types_compatible_p(typeof(*lock), struct mutex) ? \ > + __list_check_mutex((struct mutex *)lock) : \ > + __builtin_types_compatible_p(typeof(*lock), struct spinlock) ? \ > + __list_check_spin((struct spinlock *)lock) : \ > + __list_check_bug(lock); \ > + 0; }) > + > +#define list_for_each_entry_check(pos, head, member, lock) \ > + for (__list_check(lock), \ > + pos = list_first_entry(head, typeof(*pos), member); \ > + &pos->member != (head); \ > + pos = list_next_entry(pos, member)) > + > +#define i915_gem_object_for_each_vma(vma, obj) \ > + list_for_each_entry_check(vma, &(obj)->vma_list, obj_link, &(obj)->base.dev->struct_mutex) > + > #endif Nice! Want to take it over? ;) Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 4004a7cf8db4..931684a74533 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -140,7 +140,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, obj_link) { + i915_gem_object_for_each_vma(vma, obj) { if (vma->pin_count > 0) pin_count++; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 31487aa11977..574e45ab43cb 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3565,4 +3565,36 @@ int remap_io_mapping(struct vm_area_struct *vma, unsigned long addr, unsigned long pfn, unsigned long size, struct io_mapping *iomap); +static __always_inline void __list_check_mutex(struct mutex *lock) +{ + lockdep_assert_held(lock); +} + +static __always_inline void __list_check_spin(struct spinlock *lock) +{ + assert_spin_locked(lock); +} + +static __always_inline void __list_check_bug(void *_lock) +{ + BUILD_BUG_ON("unknown lock type"); +} + +#define __list_check(lock) \ + ({ __builtin_types_compatible_p(typeof(*lock), struct mutex) ? \ + __list_check_mutex((struct mutex *)lock) : \ + __builtin_types_compatible_p(typeof(*lock), struct spinlock) ? \ + __list_check_spin((struct spinlock *)lock) : \ + __list_check_bug(lock); \ + 0; }) + +#define list_for_each_entry_check(pos, head, member, lock) \ + for (__list_check(lock), \ + pos = list_first_entry(head, typeof(*pos), member); \ + &pos->member != (head); \ + pos = list_next_entry(pos, member)) + +#define i915_gem_object_for_each_vma(vma, obj) \ + list_for_each_entry_check(vma, &(obj)->vma_list, obj_link, &(obj)->base.dev->struct_mutex) + #endif