From patchwork Sun May 19 17:22:01 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 2592651 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id C1E5CDF2A2 for ; Mon, 20 May 2013 12:09:18 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CAF48E5D36 for ; Mon, 20 May 2013 05:09:18 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from caramon.arm.linux.org.uk (caramon.arm.linux.org.uk [78.32.30.218]) by gabe.freedesktop.org (Postfix) with ESMTP id 2F255E5DBF for ; Sun, 19 May 2013 10:23:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=caramon; h=Sender:Content-Type:MIME-Version:Message-ID:Subject:To:From:Date; bh=pFn4Z8y8IOUx+qB9s7wyr6N0OW4vVt6X35wqRcUQr7c=; b=CBc6aciFwy7R+MdBvETBrYJRilx949LMYU/L2vVFEC02ewDurTKs7wpfKr5ZZUEszrnRoxenoBjW/DxV5aVzNJq9UxQAUPLB4A87qN03VwLR3jwfxC8eLQozgi9nLoz2MgH9v2Z0K/ZZddGZVifeNTsqQBFao/0+hpuoHS7WtO8=; Received: from n2100.arm.linux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:47334) by caramon.arm.linux.org.uk with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.76) (envelope-from ) id 1Ue7J5-0001qq-I4; Sun, 19 May 2013 18:22:03 +0100 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1Ue7J4-0006fg-8u; Sun, 19 May 2013 18:22:02 +0100 Date: Sun, 19 May 2013 18:22:01 +0100 From: Russell King - ARM Linux To: David Airlie , dri-devel@lists.freedesktop.org, Chris Wilson Subject: BUG: drm_mm head_node gets broken? Message-ID: <20130519172201.GS18614@n2100.arm.linux.org.uk> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.19 (2009-01-05) X-Mailman-Approved-At: Mon, 20 May 2013 05:07:00 -0700 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org 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: 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. 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); }