list_for_each_entry_check() is a variant of list_for_each_entry() modelled
after the ideas for RCU checking, that first evaluates an expression
before iterating. This can be used to both document and verify the
appropriate locks are held for the list. In order to make the macro more
versatile, list_for_each_entry_check() can be passed either a spinlock
or a struct mutex to verify.
Also provided is __list_for_each_entry_check() which allows the caller
to control the expression executed before list iteration for more
complex locking behaviour.
Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
---
First draft to ensure a bit of polish before invoking the overlords.
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 4 +++
include/drm/drm_crtc.h | 21 ++++++-------
include/linux/list_check.h | 62 +++++++++++++++++++++++++++++++++++++
4 files changed, 77 insertions(+), 12 deletions(-)
create mode 100644 include/linux/list_check.h
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..429f26469eca 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -51,6 +51,7 @@
#include <linux/backlight.h>
#include <linux/hash.h>
#include <linux/intel-iommu.h>
+#include <linux/list_check.h>
#include <linux/kref.h>
#include <linux/pm_qos.h>
#include "intel_guc.h"
@@ -3565,4 +3566,7 @@ int remap_io_mapping(struct vm_area_struct *vma,
unsigned long addr, unsigned long pfn, unsigned long size,
struct io_mapping *iomap);
+#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
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c65a212db77e..ca34a18f42d8 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -30,6 +30,7 @@
#include <linux/types.h>
#include <linux/idr.h>
#include <linux/fb.h>
+#include <linux/list_check.h>
#include <linux/hdmi.h>
#include <linux/media-bus-format.h>
#include <uapi/drm/drm_mode.h>
@@ -2541,7 +2542,7 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
#define drm_for_each_crtc(crtc, dev) \
list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
-static inline void
+static inline int
assert_drm_connector_list_read_locked(struct drm_mode_config *mode_config)
{
/*
@@ -2553,23 +2554,21 @@ assert_drm_connector_list_read_locked(struct drm_mode_config *mode_config)
*/
WARN_ON(!mutex_is_locked(&mode_config->mutex) &&
!drm_modeset_is_locked(&mode_config->connection_mutex));
+
+ return 1;
}
#define drm_for_each_connector(connector, dev) \
- for (assert_drm_connector_list_read_locked(&(dev)->mode_config), \
- connector = list_first_entry(&(dev)->mode_config.connector_list, \
- struct drm_connector, head); \
- &connector->head != (&(dev)->mode_config.connector_list); \
- connector = list_next_entry(connector, head))
+ __list_for_each_entry_check(connector, \
+ &(dev)->mode_config.connector_list, \
+ head, \
+ assert_drm_connector_list_read_locked(&(dev)->mode_config))
#define drm_for_each_encoder(encoder, dev) \
list_for_each_entry(encoder, &(dev)->mode_config.encoder_list, head)
#define drm_for_each_fb(fb, dev) \
- for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.fb_lock)), \
- fb = list_first_entry(&(dev)->mode_config.fb_list, \
- struct drm_framebuffer, head); \
- &fb->head != (&(dev)->mode_config.fb_list); \
- fb = list_next_entry(fb, head))
+ list_for_each_entry_check(fb, &(dev)->mode_config.fb_list, head, \
+ &(dev)->mode_config.fb_lock)
#endif /* __DRM_CRTC_H__ */
diff --git a/include/linux/list_check.h b/include/linux/list_check.h
new file mode 100644
index 000000000000..3f23c98b7b24
--- /dev/null
+++ b/include/linux/list_check.h
@@ -0,0 +1,62 @@
+#ifndef _LINUX_LIST_CHECK_H
+#define _LINUX_LIST_CHECK_H
+
+#include <linux/bug.h>
+#include <linux/lockdep.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+
+/**
+ * __list_for_each_entry_check - iterate over list of given type
+ * @pos: the type * to use as a loop cursor.
+ * @head: the head for your list.
+ * @member: the name of the list_head within the struct.
+ * @expr: the expression to call before iterating
+ *
+ * Before iterating over the list, see list_for_each_entry(), evaluate @expr.
+ */
+#define __list_for_each_entry_check(pos, head, member, expr) \
+ for (expr, pos = list_first_entry(head, typeof(*pos), member); \
+ &pos->member != (head); \
+ pos = list_next_entry(pos, member))
+
+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), spinlock_t) ? \
+ __list_check_spin((spinlock_t *)lock) : \
+ __list_check_bug(lock); \
+ 1; })
+
+/**
+ * list_for_each_entry_check - iterate over list of given type
+ * @pos: the type * to use as a loop cursor.
+ * @head: the head for your list.
+ * @member: the name of the list_head within the struct.
+ * @lock: the lock guarding the list
+ *
+ * Before iterating over the list, see list_for_each_entry(), first
+ * verify that the @lock is held, either using lockdep_assert_held() for
+ * mutexes or assert_spin_locked() for spinlocks. The purpose of this macro
+ * is two-fold: one to document the locks guarding the list, and secondly
+ * to verify that invariant is held at runtime.
+ */
+#define list_for_each_entry_check(pos, head, member, lock) \
+ __list_for_each_entry_check(pos, head, member, __list_check(lock))
+
+#endif
--
2.7.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
