diff mbox

[39/48] drm/i915: Do not allow buffers at offset 0

Message ID 1386367941-7131-39-git-send-email-benjamin.widawsky@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Widawsky Dec. 6, 2013, 10:11 p.m. UTC
This is primarily a band aid for an unexplainable error in
gem_reloc_vs_gpu/forked-faulting-reloc-thrashing. Essentially as soon as
a relocated buffer (which had a non-zero presumed offset) moved to
offset 0, something goes bad. Since I have been unable to solve this,
and potentially this is a good thing to do anyway, since many things can
accidentally write to offset 0, why not?

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Chris Wilson Dec. 12, 2013, 10:59 a.m. UTC | #1
On Fri, Dec 06, 2013 at 02:11:24PM -0800, Ben Widawsky wrote:
> This is primarily a band aid for an unexplainable error in
> gem_reloc_vs_gpu/forked-faulting-reloc-thrashing. Essentially as soon as
> a relocated buffer (which had a non-zero presumed offset) moved to
> offset 0, something goes bad. Since I have been unable to solve this,
> and potentially this is a good thing to do anyway, since many things can
> accidentally write to offset 0, why not?

A better hack would have been to exclude the first page from the vm->mm,
so that we didn't then introduce a change to ggtt.
-Chris
Daniel Vetter Dec. 18, 2013, 2:58 p.m. UTC | #2
On Thu, Dec 12, 2013 at 10:59:03AM +0000, Chris Wilson wrote:
> On Fri, Dec 06, 2013 at 02:11:24PM -0800, Ben Widawsky wrote:
> > This is primarily a band aid for an unexplainable error in
> > gem_reloc_vs_gpu/forked-faulting-reloc-thrashing. Essentially as soon as
> > a relocated buffer (which had a non-zero presumed offset) moved to
> > offset 0, something goes bad. Since I have been unable to solve this,
> > and potentially this is a good thing to do anyway, since many things can
> > accidentally write to offset 0, why not?
> 
> A better hack would have been to exclude the first page from the vm->mm,
> so that we didn't then introduce a change to ggtt.

Band aids need to come with much better explanations. Also, if we really
want a guard area it should be of sufficient size to avoid scribbling over
random gunk. With 4k screens that means about 32MB.

I'll smash a revert for this patch on top of the series.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e6d7b4c..a9cabff 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3280,9 +3280,11 @@  i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
 	WARN_ON(!list_is_singular(&obj->vma_list));
 
 search_free:
+	/* FIXME: Some tests are failing when they receive a reloc of 0. To
+	 * prevent this, we simply don't allow the 0th offset. */
 	ret = drm_mm_insert_node_in_range_generic(&vm->mm, &vma->node,
 						  size, alignment,
-						  obj->cache_level, 0, gtt_max,
+						  obj->cache_level, 1, gtt_max,
 						  DRM_MM_SEARCH_DEFAULT);
 	if (ret) {
 		ret = i915_gem_evict_something(dev, vm, size, alignment,