Re: [Nouveau] [PATCH] drm/nouveau: fix nouveau_mm/nouveau_mm_node leak

2012-10-21 Thread Marcin Slusarz 
On Fri, Oct 19, 2012 at 04:05:14PM +1000, Ben Skeggs wrote:
 On Thu, Oct 11, 2012 at 11:53:09PM +0200, Marcin Slusarz wrote:
  Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
  ---
   drivers/gpu/drm/nouveau/core/core/gpuobj.c | 6 +-
   drivers/gpu/drm/nouveau/core/include/core/gpuobj.h | 3 +++
   2 files changed, 8 insertions(+), 1 deletion(-)
  
  diff --git a/drivers/gpu/drm/nouveau/core/core/gpuobj.c 
  b/drivers/gpu/drm/nouveau/core/core/gpuobj.c
  index c2a7608..48121d2 100644
  --- a/drivers/gpu/drm/nouveau/core/core/gpuobj.c
  +++ b/drivers/gpu/drm/nouveau/core/core/gpuobj.c
  @@ -39,8 +39,11 @@ nouveau_gpuobj_destroy(struct nouveau_gpuobj *gpuobj)
  nv_wo32(gpuobj, i, 0x);
  }
   
  +   if (gpuobj-node)
  +   nouveau_mm_free(gpuobj-node_heap, gpuobj-node);
  +
 if (gpuobj-node) {
   nouveau_mm_free(nv_gpuobj(gpuobj-parent)-heap,
   gpuobj-node);
 }
 
 Or something to that effect, instead of having to store the heap
 pointer again.

Oh, right.
(Somehow I assumed pargpu to be different object than parent when
I first read it.)

  if (gpuobj-heap.block_size)
  -   nouveau_mm_fini(gpuobj-heap);
  +   WARN_ON(nouveau_mm_fini(gpuobj-heap));
 Alright, I get this.  However, perhaps we should go the full hog here
 and make nouveau_mm_fini() directly do the WARN_ON() in this situation?
 
 There was, once upon a time, reasons for it not doing this, I don't
 believe they're valid anymore though.

OK.

Marcin
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau: fix nouveau_mm/nouveau_mm_node leak

2012-10-19 Thread Ben Skeggs 
On Thu, Oct 11, 2012 at 11:53:09PM +0200, Marcin Slusarz wrote:
 Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
 ---
  drivers/gpu/drm/nouveau/core/core/gpuobj.c | 6 +-
  drivers/gpu/drm/nouveau/core/include/core/gpuobj.h | 3 +++
  2 files changed, 8 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/nouveau/core/core/gpuobj.c 
 b/drivers/gpu/drm/nouveau/core/core/gpuobj.c
 index c2a7608..48121d2 100644
 --- a/drivers/gpu/drm/nouveau/core/core/gpuobj.c
 +++ b/drivers/gpu/drm/nouveau/core/core/gpuobj.c
 @@ -39,8 +39,11 @@ nouveau_gpuobj_destroy(struct nouveau_gpuobj *gpuobj)
   nv_wo32(gpuobj, i, 0x);
   }
  
 + if (gpuobj-node)
 + nouveau_mm_free(gpuobj-node_heap, gpuobj-node);
 +
if (gpuobj-node) {
nouveau_mm_free(nv_gpuobj(gpuobj-parent)-heap,
gpuobj-node);
}

Or something to that effect, instead of having to store the heap
pointer again.

   if (gpuobj-heap.block_size)
 - nouveau_mm_fini(gpuobj-heap);
 + WARN_ON(nouveau_mm_fini(gpuobj-heap));
Alright, I get this.  However, perhaps we should go the full hog here
and make nouveau_mm_fini() directly do the WARN_ON() in this situation?

There was, once upon a time, reasons for it not doing this, I don't
believe they're valid anymore though.

If you want to do this, that'd be great.  Bonus points for being in a
separate patch :)

  
   nouveau_object_destroy(gpuobj-base);
  }
 @@ -114,6 +117,7 @@ nouveau_gpuobj_create_(struct nouveau_object *parent,
 max(align, (u32)1), gpuobj-node);
   if (ret)
   return ret;
 + gpuobj-node_heap = heap;
  
   gpuobj-addr += gpuobj-node-offset;
   }
 diff --git a/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h 
 b/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h
 index d09adf1..f65bf5b 100644
 --- a/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h
 +++ b/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h
 @@ -16,7 +16,10 @@ struct nouveau_vm;
  struct nouveau_gpuobj {
   struct nouveau_object base;
   struct nouveau_object *parent;
 +
 + struct nouveau_mm *node_heap;
   struct nouveau_mm_node *node;
 +
   struct nouveau_mm heap;
  
   u32 flags;
 -- 
 1.7.12
 
 ___
 Nouveau mailing list
 Nouveau@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/nouveau
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau: fix nouveau_mm/nouveau_mm_node leak

2012-10-16 Thread Marcin Slusarz 
On Thu, Oct 11, 2012 at 11:53:09PM +0200, Marcin Slusarz wrote:
 Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
 ---
  drivers/gpu/drm/nouveau/core/core/gpuobj.c | 6 +-
  drivers/gpu/drm/nouveau/core/include/core/gpuobj.h | 3 +++
  2 files changed, 8 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/nouveau/core/core/gpuobj.c 
 b/drivers/gpu/drm/nouveau/core/core/gpuobj.c
 index c2a7608..48121d2 100644
 --- a/drivers/gpu/drm/nouveau/core/core/gpuobj.c
 +++ b/drivers/gpu/drm/nouveau/core/core/gpuobj.c
 @@ -39,8 +39,11 @@ nouveau_gpuobj_destroy(struct nouveau_gpuobj *gpuobj)
   nv_wo32(gpuobj, i, 0x);
   }
  
 + if (gpuobj-node)
 + nouveau_mm_free(gpuobj-node_heap, gpuobj-node);
 +
   if (gpuobj-heap.block_size)
 - nouveau_mm_fini(gpuobj-heap);
 + WARN_ON(nouveau_mm_fini(gpuobj-heap));
  
   nouveau_object_destroy(gpuobj-base);
  }
 @@ -114,6 +117,7 @@ nouveau_gpuobj_create_(struct nouveau_object *parent,
 max(align, (u32)1), gpuobj-node);
   if (ret)
   return ret;
 + gpuobj-node_heap = heap;
  
   gpuobj-addr += gpuobj-node-offset;
   }
 diff --git a/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h 
 b/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h
 index d09adf1..f65bf5b 100644
 --- a/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h
 +++ b/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h
 @@ -16,7 +16,10 @@ struct nouveau_vm;
  struct nouveau_gpuobj {
   struct nouveau_object base;
   struct nouveau_object *parent;
 +
 + struct nouveau_mm *node_heap;
   struct nouveau_mm_node *node;
 +
   struct nouveau_mm heap;
  
   u32 flags;
 -- 

What's wrong with this patch?
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH] drm/nouveau: fix nouveau_mm/nouveau_mm_node leak

2012-10-11 Thread Marcin Slusarz 
Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
---
 drivers/gpu/drm/nouveau/core/core/gpuobj.c | 6 +-
 drivers/gpu/drm/nouveau/core/include/core/gpuobj.h | 3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/core/core/gpuobj.c 
b/drivers/gpu/drm/nouveau/core/core/gpuobj.c
index c2a7608..48121d2 100644
--- a/drivers/gpu/drm/nouveau/core/core/gpuobj.c
+++ b/drivers/gpu/drm/nouveau/core/core/gpuobj.c
@@ -39,8 +39,11 @@ nouveau_gpuobj_destroy(struct nouveau_gpuobj *gpuobj)
nv_wo32(gpuobj, i, 0x);
}
 
+   if (gpuobj-node)
+   nouveau_mm_free(gpuobj-node_heap, gpuobj-node);
+
if (gpuobj-heap.block_size)
-   nouveau_mm_fini(gpuobj-heap);
+   WARN_ON(nouveau_mm_fini(gpuobj-heap));
 
nouveau_object_destroy(gpuobj-base);
 }
@@ -114,6 +117,7 @@ nouveau_gpuobj_create_(struct nouveau_object *parent,
  max(align, (u32)1), gpuobj-node);
if (ret)
return ret;
+   gpuobj-node_heap = heap;
 
gpuobj-addr += gpuobj-node-offset;
}
diff --git a/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h 
b/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h
index d09adf1..f65bf5b 100644
--- a/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h
+++ b/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h
@@ -16,7 +16,10 @@ struct nouveau_vm;
 struct nouveau_gpuobj {
struct nouveau_object base;
struct nouveau_object *parent;
+
+   struct nouveau_mm *node_heap;
struct nouveau_mm_node *node;
+
struct nouveau_mm heap;
 
u32 flags;
-- 
1.7.12

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau