So, while tinkering with my Dove DRM driver, I tripped over a BUG_ON()
in drm_mm_hole_node_start(), which was called from drm_mm_dump_table():
int drm_mm_dump_table(struct seq_file *m, struct drm_mm *mm)
{
struct drm_mm_node *entry;
unsigned long total_used = 0, total_free = 0, total = 0;
unsigned long hole_start, hole_end, hole_size;
hole_start = drm_mm_hole_node_start(&mm->head_node);
hole_end = drm_mm_hole_node_end(&mm->head_node);
drm_mm_hole_node_start() does this:
static inline unsigned long drm_mm_hole_node_start(struct drm_mm_node *hole_node)
{
BUG_ON(!hole_node->hole_follows);
return __drm_mm_hole_node_start(hole_node);
}
This appears to indicate that it is required that the head node should
always have a "hole" following it.
As this used to work, I dug in the git history, and found commit 6b9d89b4
(drm: Add colouring to the range allocator), in particular this change:
static void drm_mm_insert_helper(struct drm_mm_node *hole_node,
struct drm_mm_node *node,
- unsigned long size, unsigned alignment)
+ unsigned long size, unsigned alignment,
+ unsigned long color)
{
...
- if (!tmp) {
+ if (alignment) {
+ unsigned tmp = adj_start % alignment;
+ if (tmp)
+ adj_start += alignment - tmp;
+ }
+
+ if (adj_start == hole_start) {
hole_node->hole_follows = 0;
- list_del_init(&hole_node->hole_stack);
- } else
- wasted = alignment - tmp;
+ list_del(&hole_node->hole_stack);
+ }
Now, when we do an allocation, it is typically done as follows:
free = drm_mm_search_free(mm, size, align, 0);
if (free)
linear = drm_mm_get_block(free, size, align);
If nothing has been allocated in the mm, 'free' will be &mm->head_node.
The result is that drm_mm_get_block calls down into drm_mm_insert_helper
with hole_node == &mm->head_node. This I confirmed via:
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index db1e2d6..7241be8 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -125,6 +125,7 @@ static void drm_mm_insert_helper(struct drm_mm_node *hole_node,
}
if (adj_start == hole_start) {
+ BUG_ON(hole_node == &mm->head_node);
hole_node->hole_follows = 0;
list_del(&hole_node->hole_stack);
}
which triggers during booting. Hence, we end up setting the head_node
to indicate no hole following - which violates the conditions of other
parts of the code in drm_mm.c.
It looks like this change was made quite a while ago - back in July
2012, so I assume that while the rest of the allocator is fine with this,
it's just the debugging code which is now broken. However, I suspect
that DRM people may wish to either back out the coloring change or
carefully review the change for correctness throughout the drm_mm
code.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
