| Message ID | 20170303180335.18857-1-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 03, 2017 at 06:03:34PM +0000, Matthew Auld wrote: > It looks like we were incorrectly comparing vma->node against itself > instead of the target node, when evicting for a node on systems where we > need guard pages between regions with different cache domains. As a > consequence we can end up trying to needlessly evict neighbouring nodes, > even if they have the same cache domain, and if they were pinned we > would fail the eviction. > > Fixes: 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a helper") > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> /o\ -Chris
On pe, 2017-03-03 at 18:18 +0000, Chris Wilson wrote: > On Fri, Mar 03, 2017 at 06:03:34PM +0000, Matthew Auld wrote: > > > > It looks like we were incorrectly comparing vma->node against itself > > instead of the target node, when evicting for a node on systems where we > > need guard pages between regions with different cache domains. As a > > consequence we can end up trying to needlessly evict neighbouring nodes, > > even if they have the same cache domain, and if they were pinned we > > would fail the eviction. > > > > Fixes: 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a helper") > > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> /o\ > -Chris Good catch, will merge the series once the comments on patch 2 are addressed. Regards, Joonas
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index a0de5734f7d0..2da3a94fc9f3 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -299,12 +299,12 @@ int i915_gem_evict_for_node(struct i915_address_space *vm, * those as well to make room for our guard pages. */ if (check_color) { - if (vma->node.start + vma->node.size == node->start) { - if (vma->node.color == node->color) + if (node->start + node->size == target->start) { + if (node->color == target->color) continue; } - if (vma->node.start == node->start + node->size) { - if (vma->node.color == node->color) + if (node->start == target->start + target->size) { + if (node->color == target->color) continue; } }
It looks like we were incorrectly comparing vma->node against itself instead of the target node, when evicting for a node on systems where we need guard pages between regions with different cache domains. As a consequence we can end up trying to needlessly evict neighbouring nodes, even if they have the same cache domain, and if they were pinned we would fail the eviction. Fixes: 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a helper") Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_gem_evict.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)