diff mbox

drm/mm: fix dump table BUG

Message ID 1366452491-14365-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter April 20, 2013, 10:08 a.m. UTC
In

commit 9e8944ab564f2e3dde90a518cd32048c58918608
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Nov 15 11:32:17 2012 +0000

    drm: Introduce an iterator over holes in the drm_mm range manager

helpers and iterators for hole handling have been introduced with some
debug BUG_ONs sprinkled over. Unfortunately this broke the mm dumper
which unconditionally tried to compute the size of the very first
hole.

Reported-by: Christopher Harvey <charvey@matrox.com>
Cc: Christopher Harvey <charvey@matrox.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_mm.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Chris Wilson April 20, 2013, 10:11 a.m. UTC | #1
On Sat, Apr 20, 2013 at 12:08:11PM +0200, Daniel Vetter wrote:
> In
> 
> commit 9e8944ab564f2e3dde90a518cd32048c58918608
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Thu Nov 15 11:32:17 2012 +0000
> 
>     drm: Introduce an iterator over holes in the drm_mm range manager
> 
> helpers and iterators for hole handling have been introduced with some
> debug BUG_ONs sprinkled over. Unfortunately this broke the mm dumper
> which unconditionally tried to compute the size of the very first
> hole.
> 
> Reported-by: Christopher Harvey <charvey@matrox.com>
> Cc: Christopher Harvey <charvey@matrox.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_mm.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index db1e2d6..a0ba3a1 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -759,14 +759,15 @@ 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;
> +	unsigned long hole_start, hole_end, hole_size = 0;
>  
> -	hole_start = drm_mm_hole_node_start(&mm->head_node);
> -	hole_end = drm_mm_hole_node_end(&mm->head_node);

Use __drm_mm_hole_node_start and __drm_mm_hole_node_end instead.
-Chris
Daniel Vetter April 20, 2013, 11:28 a.m. UTC | #2
On Sat, Apr 20, 2013 at 12:11 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Sat, Apr 20, 2013 at 12:08:11PM +0200, Daniel Vetter wrote:
>> In
>>
>> commit 9e8944ab564f2e3dde90a518cd32048c58918608
>> Author: Chris Wilson <chris@chris-wilson.co.uk>
>> Date:   Thu Nov 15 11:32:17 2012 +0000
>>
>>     drm: Introduce an iterator over holes in the drm_mm range manager
>>
>> helpers and iterators for hole handling have been introduced with some
>> debug BUG_ONs sprinkled over. Unfortunately this broke the mm dumper
>> which unconditionally tried to compute the size of the very first
>> hole.
>>
>> Reported-by: Christopher Harvey <charvey@matrox.com>
>> Cc: Christopher Harvey <charvey@matrox.com>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>  drivers/gpu/drm/drm_mm.c |   11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
>> index db1e2d6..a0ba3a1 100644
>> --- a/drivers/gpu/drm/drm_mm.c
>> +++ b/drivers/gpu/drm/drm_mm.c
>> @@ -759,14 +759,15 @@ 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;
>> +     unsigned long hole_start, hole_end, hole_size = 0;
>>
>> -     hole_start = drm_mm_hole_node_start(&mm->head_node);
>> -     hole_end = drm_mm_hole_node_end(&mm->head_node);
>
> Use __drm_mm_hole_node_start and __drm_mm_hole_node_end instead.

I've figured I could make the condition more symmetric with the one in
the loop, so that both check ->hole_follows.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Chris Wilson April 20, 2013, 3:54 p.m. UTC | #3
On Sat, Apr 20, 2013 at 01:28:53PM +0200, Daniel Vetter wrote:
> On Sat, Apr 20, 2013 at 12:11 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Sat, Apr 20, 2013 at 12:08:11PM +0200, Daniel Vetter wrote:
> >> In
> >>
> >> commit 9e8944ab564f2e3dde90a518cd32048c58918608
> >> Author: Chris Wilson <chris@chris-wilson.co.uk>
> >> Date:   Thu Nov 15 11:32:17 2012 +0000
> >>
> >>     drm: Introduce an iterator over holes in the drm_mm range manager
> >>
> >> helpers and iterators for hole handling have been introduced with some
> >> debug BUG_ONs sprinkled over. Unfortunately this broke the mm dumper
> >> which unconditionally tried to compute the size of the very first
> >> hole.
> >>
[snip]
> > Use __drm_mm_hole_node_start and __drm_mm_hole_node_end instead.
> 
> I've figured I could make the condition more symmetric with the one in
> the loop, so that both check ->hole_follows.

Your change log didn't explain your intentions to refactor the code, only
to avoid the extra BUG_ON...
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index db1e2d6..a0ba3a1 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -759,14 +759,15 @@  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;
+	unsigned long hole_start, hole_end, hole_size = 0;
 
-	hole_start = drm_mm_hole_node_start(&mm->head_node);
-	hole_end = drm_mm_hole_node_end(&mm->head_node);
-	hole_size = hole_end - hole_start;
-	if (hole_size)
+	if (mm->head_node.hole_follows) {
+		hole_start = drm_mm_hole_node_start(&mm->head_node);
+		hole_end = drm_mm_hole_node_end(&mm->head_node);
+		hole_size = hole_end - hole_start;
 		seq_printf(m, "0x%08lx-0x%08lx: 0x%08lx: free\n",
 				hole_start, hole_end, hole_size);
+	}
 	total_free += hole_size;
 
 	drm_mm_for_each_node(entry, mm) {