Re: [Intel-gfx] [PATCH 08/16] drm/i915/error: Do a better job of disambiguating VMAs
On Thu, Jul 17, 2014 at 10:51:23AM +0200, Daniel Vetter wrote:
On Fri, Jul 04, 2014 at 09:56:54AM -0700, Ben Widawsky wrote:
On Fri, Jul 04, 2014 at 08:57:08AM +0100, Chris Wilson wrote:
On Tue, Jul 01, 2014 at 11:17:43AM -0700, Ben Widawsky wrote:
Some of the original PPGTT patches in this area where unmerged, and this
left a lot of confusion in our error capture with regard to which vm/obj
we want to capture. There have been at least a couple of patches from
Chris, and myself to try to fix this up; so here is another shot. Nobody
running without full PPGTT is effected by this, and that is probably why
nobody has bothered to fix it yet.
Instead of using any of the global lists to find the VMAs we want to
capture, we use the union of the active, and the inactive list in the
VM. This allows us to replace our capture_bo with capture_vma, and know
all the VMAs we want to capture are valid.
I could have probably figured out a way to reuse mm_list. As we've had
bugs here before in the shrinker, I think the best way forward is to get
it working, and then optimize it later.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 1 +
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
drivers/gpu/drm/i915/i915_gpu_error.c | 39
++-
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a4153ee..88451dc 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2114,6 +2114,7 @@ static struct i915_vma
*__i915_gem_vma_create(struct drm_i915_gem_object *obj,
return ERR_PTR(-ENOMEM);
INIT_LIST_HEAD(vma-vma_link);
+ INIT_LIST_HEAD(vma-pin_capture_link);
INIT_LIST_HEAD(vma-mm_list);
INIT_LIST_HEAD(vma-exec_list);
vma-vm = vm;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8d6f7c1..1d75801 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -126,6 +126,8 @@ struct i915_vma {
struct list_head vma_link; /* Link in the object's VMA list */
+ struct list_head pin_capture_link; /* Link in the error capture
*/
+
/** This vma's place in the batchbuffer or on the eviction list
*/
struct list_head exec_list;
We already have a slot for temporary lists...
-Chris
I did mention that in the commit message, if I caught your meaning.
Chris is probably talking about exec_list which is our canonical temporary
list, mostly used by execbuf. But also in other places.
-Daniel
I think that was a typo on my part, I meant exec_list. In either case, I
think doing it this way and merging it later is the safest path.
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/16] drm/i915/error: Do a better job of disambiguating VMAs
On Fri, Jul 04, 2014 at 09:56:54AM -0700, Ben Widawsky wrote:
On Fri, Jul 04, 2014 at 08:57:08AM +0100, Chris Wilson wrote:
On Tue, Jul 01, 2014 at 11:17:43AM -0700, Ben Widawsky wrote:
Some of the original PPGTT patches in this area where unmerged, and this
left a lot of confusion in our error capture with regard to which vm/obj
we want to capture. There have been at least a couple of patches from
Chris, and myself to try to fix this up; so here is another shot. Nobody
running without full PPGTT is effected by this, and that is probably why
nobody has bothered to fix it yet.
Instead of using any of the global lists to find the VMAs we want to
capture, we use the union of the active, and the inactive list in the
VM. This allows us to replace our capture_bo with capture_vma, and know
all the VMAs we want to capture are valid.
I could have probably figured out a way to reuse mm_list. As we've had
bugs here before in the shrinker, I think the best way forward is to get
it working, and then optimize it later.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 1 +
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
drivers/gpu/drm/i915/i915_gpu_error.c | 39
++-
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a4153ee..88451dc 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2114,6 +2114,7 @@ static struct i915_vma
*__i915_gem_vma_create(struct drm_i915_gem_object *obj,
return ERR_PTR(-ENOMEM);
INIT_LIST_HEAD(vma-vma_link);
+ INIT_LIST_HEAD(vma-pin_capture_link);
INIT_LIST_HEAD(vma-mm_list);
INIT_LIST_HEAD(vma-exec_list);
vma-vm = vm;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8d6f7c1..1d75801 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -126,6 +126,8 @@ struct i915_vma {
struct list_head vma_link; /* Link in the object's VMA list */
+ struct list_head pin_capture_link; /* Link in the error capture */
+
/** This vma's place in the batchbuffer or on the eviction list */
struct list_head exec_list;
We already have a slot for temporary lists...
-Chris
I did mention that in the commit message, if I caught your meaning.
Chris is probably talking about exec_list which is our canonical temporary
list, mostly used by execbuf. But also in other places.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/16] drm/i915/error: Do a better job of disambiguating VMAs
On Tue, Jul 01, 2014 at 11:17:43AM -0700, Ben Widawsky wrote:
Some of the original PPGTT patches in this area where unmerged, and this
left a lot of confusion in our error capture with regard to which vm/obj
we want to capture. There have been at least a couple of patches from
Chris, and myself to try to fix this up; so here is another shot. Nobody
running without full PPGTT is effected by this, and that is probably why
nobody has bothered to fix it yet.
Instead of using any of the global lists to find the VMAs we want to
capture, we use the union of the active, and the inactive list in the
VM. This allows us to replace our capture_bo with capture_vma, and know
all the VMAs we want to capture are valid.
I could have probably figured out a way to reuse mm_list. As we've had
bugs here before in the shrinker, I think the best way forward is to get
it working, and then optimize it later.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 1 +
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
drivers/gpu/drm/i915/i915_gpu_error.c | 39
++-
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a4153ee..88451dc 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2114,6 +2114,7 @@ static struct i915_vma *__i915_gem_vma_create(struct
drm_i915_gem_object *obj,
return ERR_PTR(-ENOMEM);
INIT_LIST_HEAD(vma-vma_link);
+ INIT_LIST_HEAD(vma-pin_capture_link);
INIT_LIST_HEAD(vma-mm_list);
INIT_LIST_HEAD(vma-exec_list);
vma-vm = vm;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8d6f7c1..1d75801 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -126,6 +126,8 @@ struct i915_vma {
struct list_head vma_link; /* Link in the object's VMA list */
+ struct list_head pin_capture_link; /* Link in the error capture */
+
/** This vma's place in the batchbuffer or on the eviction list */
struct list_head exec_list;
We already have a slot for temporary lists...
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
