| Message ID | 20170126122959.18223-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Chris Wilson <chris@chris-wilson.co.uk> writes: > Since obj->active_count is only updated upon retirement, if we see an > active object in the batch pool, double check that is still active > before deciding to allocate a new object. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem_batch_pool.c | 29 ++++++++++++++--------------- > 1 file changed, 14 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c > index 853b615fa574..22b7ee2c9ef6 100644 > --- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c > +++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c > @@ -96,8 +96,7 @@ struct drm_i915_gem_object * > i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, > size_t size) > { > - struct drm_i915_gem_object *obj = NULL; > - struct drm_i915_gem_object *tmp; > + struct drm_i915_gem_object *obj; > struct list_head *list; > int n, ret; > > @@ -112,26 +111,26 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, > n = ARRAY_SIZE(pool->cache_list) - 1; > list = &pool->cache_list[n]; > > - list_for_each_entry(tmp, list, batch_pool_link) { > + list_for_each_entry(obj, list, batch_pool_link) { > /* The batches are strictly LRU ordered */ > - if (i915_gem_object_is_active(tmp)) > - break; > + if (i915_gem_object_is_active(obj)) { > + i915_gem_retire_requests(pool->engine->i915); > + if (i915_gem_object_is_active(obj)) > + break; > + } > > - GEM_BUG_ON(!reservation_object_test_signaled_rcu(tmp->resv, > + GEM_BUG_ON(!reservation_object_test_signaled_rcu(obj->resv, > true)); > > - if (tmp->base.size >= size) { > - obj = tmp; This chunk here collides with the d07f0e59b ("drm/i915: Move GEM activity tracking..") Please rebase. -Mika > - break; > - } > + if (obj->base.size >= size) > + goto found; > } > > - if (obj == NULL) { > - obj = i915_gem_object_create_internal(pool->engine->i915, size); > - if (IS_ERR(obj)) > - return obj; > - } > + obj = i915_gem_object_create_internal(pool->engine->i915, size); > + if (IS_ERR(obj)) > + return obj; > > +found: > ret = i915_gem_object_pin_pages(obj); > if (ret) > return ERR_PTR(ret); > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jan 26, 2017 at 04:25:25PM +0200, Mika Kuoppala wrote: > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Since obj->active_count is only updated upon retirement, if we see an > > active object in the batch pool, double check that is still active > > before deciding to allocate a new object. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_gem_batch_pool.c | 29 ++++++++++++++--------------- > > 1 file changed, 14 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c > > index 853b615fa574..22b7ee2c9ef6 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c > > +++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c > > @@ -96,8 +96,7 @@ struct drm_i915_gem_object * > > i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, > > size_t size) > > { > > - struct drm_i915_gem_object *obj = NULL; > > - struct drm_i915_gem_object *tmp; > > + struct drm_i915_gem_object *obj; > > struct list_head *list; > > int n, ret; > > > > @@ -112,26 +111,26 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, > > n = ARRAY_SIZE(pool->cache_list) - 1; > > list = &pool->cache_list[n]; > > > > - list_for_each_entry(tmp, list, batch_pool_link) { > > + list_for_each_entry(obj, list, batch_pool_link) { > > /* The batches are strictly LRU ordered */ > > - if (i915_gem_object_is_active(tmp)) > > - break; > > + if (i915_gem_object_is_active(obj)) { > > + i915_gem_retire_requests(pool->engine->i915); > > + if (i915_gem_object_is_active(obj)) > > + break; > > + } > > > > - GEM_BUG_ON(!reservation_object_test_signaled_rcu(tmp->resv, > > + GEM_BUG_ON(!reservation_object_test_signaled_rcu(obj->resv, > > true)); > > > > - if (tmp->base.size >= size) { > > - obj = tmp; > > This chunk here collides with the > d07f0e59b ("drm/i915: Move GEM activity tracking..") That's already in the tree. It's the pruning that isn't yet in the tree. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c index 853b615fa574..22b7ee2c9ef6 100644 --- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c +++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c @@ -96,8 +96,7 @@ struct drm_i915_gem_object * i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, size_t size) { - struct drm_i915_gem_object *obj = NULL; - struct drm_i915_gem_object *tmp; + struct drm_i915_gem_object *obj; struct list_head *list; int n, ret; @@ -112,26 +111,26 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, n = ARRAY_SIZE(pool->cache_list) - 1; list = &pool->cache_list[n]; - list_for_each_entry(tmp, list, batch_pool_link) { + list_for_each_entry(obj, list, batch_pool_link) { /* The batches are strictly LRU ordered */ - if (i915_gem_object_is_active(tmp)) - break; + if (i915_gem_object_is_active(obj)) { + i915_gem_retire_requests(pool->engine->i915); + if (i915_gem_object_is_active(obj)) + break; + } - GEM_BUG_ON(!reservation_object_test_signaled_rcu(tmp->resv, + GEM_BUG_ON(!reservation_object_test_signaled_rcu(obj->resv, true)); - if (tmp->base.size >= size) { - obj = tmp; - break; - } + if (obj->base.size >= size) + goto found; } - if (obj == NULL) { - obj = i915_gem_object_create_internal(pool->engine->i915, size); - if (IS_ERR(obj)) - return obj; - } + obj = i915_gem_object_create_internal(pool->engine->i915, size); + if (IS_ERR(obj)) + return obj; +found: ret = i915_gem_object_pin_pages(obj); if (ret) return ERR_PTR(ret);
Since obj->active_count is only updated upon retirement, if we see an active object in the batch pool, double check that is still active before deciding to allocate a new object. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem_batch_pool.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)