[Intel-gfx] [PATCH 3/6] drm/i915: Only track live rings for retiring
We don't need to track every ring for its lifetime as they are managed by the contexts/engines. What we do want to track are the live rings so that we can sporadically clean up requests if userspace falls behind. We can simply restrict the gt->rings list to being only gt->live_rings. v2: s/live/active/ for consistency with gt.active_requests Suggested-by: Tvrtko Ursulin Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_drv.h | 3 ++- drivers/gpu/drm/i915/i915_gem.c | 6 -- drivers/gpu/drm/i915/i915_request.c | 10 -- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- drivers/gpu/drm/i915/selftests/mock_engine.c | 4 drivers/gpu/drm/i915/selftests/mock_gem_device.c | 5 +++-- 7 files changed, 18 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1837c01d44d0..54351cace362 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2060,7 +2060,8 @@ struct drm_i915_private { struct i915_gem_timeline global_timeline; struct list_head timelines; - struct list_head rings; + + struct list_head active_rings; u32 active_requests; u32 request_serial; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 906e2395c245..56c79df5ebce 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -141,6 +141,7 @@ static u32 __i915_gem_park(struct drm_i915_private *i915) { lockdep_assert_held(&i915->drm.struct_mutex); GEM_BUG_ON(i915->gt.active_requests); + GEM_BUG_ON(!list_empty(&i915->gt.active_rings)); if (!i915->gt.awake) return I915_EPOCH_INVALID; @@ -5599,9 +5600,10 @@ int i915_gem_init_early(struct drm_i915_private *dev_priv) if (!dev_priv->priorities) goto err_dependencies; - mutex_lock(&dev_priv->drm.struct_mutex); - INIT_LIST_HEAD(&dev_priv->gt.rings); INIT_LIST_HEAD(&dev_priv->gt.timelines); + INIT_LIST_HEAD(&dev_priv->gt.active_rings); + + mutex_lock(&dev_priv->drm.struct_mutex); err = i915_gem_timeline_init__global(dev_priv); mutex_unlock(&dev_priv->drm.struct_mutex); if (err) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index aa84b5447fd5..6301fb0d2319 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -322,6 +322,7 @@ static void advance_ring(struct i915_request *request) * noops - they are safe to be replayed on a reset. */ tail = READ_ONCE(request->tail); + list_del(&ring->active_link); } else { tail = request->postfix; } @@ -1084,6 +1085,8 @@ void __i915_request_add(struct i915_request *request, bool flush_caches) i915_gem_active_set(&timeline->last_request, request); list_add_tail(&request->ring_link, &ring->request_list); + if (list_is_first(&request->ring_link, &ring->request_list)) + list_add(&ring->active_link, &request->i915->gt.active_rings); request->emitted_jiffies = jiffies; /* @@ -1406,14 +1409,17 @@ static void ring_retire_requests(struct intel_ring *ring) void i915_retire_requests(struct drm_i915_private *i915) { - struct intel_ring *ring, *next; + struct intel_ring *ring, *tmp; lockdep_assert_held(&i915->drm.struct_mutex); if (!i915->gt.active_requests) return; - list_for_each_entry_safe(ring, next, &i915->gt.rings, link) + /* An outstanding request must be on a still active ring somewhere */ + GEM_BUG_ON(list_empty(&i915->gt.active_rings)); + + list_for_each_entry_safe(ring, tmp, &i915->gt.active_rings, active_link) ring_retire_requests(ring); } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 75dca28782ee..3d02b2c779e7 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1149,8 +1149,6 @@ intel_engine_create_ring(struct intel_engine_cs *engine, int size) } ring->vma = vma; - list_add(&ring->link, &engine->i915->gt.rings); - return ring; } @@ -1162,8 +1160,6 @@ intel_ring_free(struct intel_ring *ring) i915_vma_close(ring->vma); __i915_gem_object_release_unless_active(obj); - list_del(&ring->link); - kfree(ring); } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index d816f8dea245..e8d17bcd9bee 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -129,7 +12
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Only track live rings for retiring
On 23/04/2018 11:36, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-04-23 11:25:54) On 23/04/2018 11:13, Chris Wilson wrote: We don't need to track every ring for its lifetime as they are managed by the contexts/engines. What we do want to track are the live rings so that we can sporadically clean up requests if userspace falls behind. We can simply restrict the gt->rings list to being only gt->live_rings. Suggested-by: Tvrtko Ursulin Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gem.c | 3 ++- drivers/gpu/drm/i915/i915_request.c | 6 +- drivers/gpu/drm/i915/i915_utils.h| 6 ++ drivers/gpu/drm/i915/intel_ringbuffer.c | 4 drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- drivers/gpu/drm/i915/selftests/mock_engine.c | 4 drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 +- 8 files changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 73936be90aed..a7787c2cb53c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2060,7 +2060,7 @@ struct drm_i915_private { struct i915_gem_timeline global_timeline; struct list_head timelines; - struct list_head rings; + struct list_head live_rings; u32 active_requests; /** diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 906e2395c245..0097a77fae8d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -141,6 +141,7 @@ static u32 __i915_gem_park(struct drm_i915_private *i915) { lockdep_assert_held(&i915->drm.struct_mutex); GEM_BUG_ON(i915->gt.active_requests); + GEM_BUG_ON(!list_empty(&i915->gt.live_rings)); if (!i915->gt.awake) return I915_EPOCH_INVALID; @@ -5600,7 +5601,7 @@ int i915_gem_init_early(struct drm_i915_private *dev_priv) goto err_dependencies; mutex_lock(&dev_priv->drm.struct_mutex); - INIT_LIST_HEAD(&dev_priv->gt.rings); + INIT_LIST_HEAD(&dev_priv->gt.live_rings); INIT_LIST_HEAD(&dev_priv->gt.timelines); err = i915_gem_timeline_init__global(dev_priv); mutex_unlock(&dev_priv->drm.struct_mutex); diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 0bf949ec9f1a..534b8d684cef 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -316,6 +316,7 @@ static void advance_ring(struct i915_request *request) * noops - they are safe to be replayed on a reset. */ tail = READ_ONCE(request->tail); + list_del(&ring->live); } else { tail = request->postfix; } @@ -1046,6 +1047,8 @@ void __i915_request_add(struct i915_request *request, bool flush_caches) i915_gem_active_set(&timeline->last_request, request); list_add_tail(&request->ring_link, &ring->request_list); + if (list_is_first(&request->ring_link, &ring->request_list)) + list_add(&ring->live, &request->i915->gt.live_rings); If you re-order the two list adds you could use list_empty and wouldn't have to add list_is_first. list_is_first tallies nicely with the list_is_last used before the corresponding list_del. Yes but to me that's minor, basically immaterial as argument whether or not to add our own list helper. request->emitted_jiffies = jiffies; /* @@ -1375,7 +1378,8 @@ void i915_retire_requests(struct drm_i915_private *i915) if (!i915->gt.active_requests) return; - list_for_each_entry_safe(ring, next, &i915->gt.rings, link) + GEM_BUG_ON(list_empty(&i915->gt.live_rings)); Maybe blank line here since the assert is not logically associated with the list but with the !i915.active_requests? I was thinking list when I wrote it. It's small enough we can argue both and both be right. Hm obviosuly it is not an error to call i915_retire_requests with nothing active (early return). So I even briefly wanted to suggest to make it 100% explicit and have the assert at the top of the function as: GEM_BUG_ON(!!i915->gt.active_requests ^ !!list_empty(..)); Unless I messed it up, the idea is to check those two are always in the same state. + list_for_each_entry_safe(ring, next, &i915->gt.live_rings, live) ring_retire_requests(ring); } diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index 0695717522ea..00165ad55fb3 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -120,6 +120,12 @@ static inline u64 ptr_to_u64(const void *ptr) #include +static inline int list_is_first(const st
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Only track live rings for retiring
Quoting Tvrtko Ursulin (2018-04-23 11:25:54) > > On 23/04/2018 11:13, Chris Wilson wrote: > > We don't need to track every ring for its lifetime as they are managed > > by the contexts/engines. What we do want to track are the live rings so > > that we can sporadically clean up requests if userspace falls behind. We > > can simply restrict the gt->rings list to being only gt->live_rings. > > > > Suggested-by: Tvrtko Ursulin > > Signed-off-by: Chris Wilson > > Cc: Tvrtko Ursulin > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > drivers/gpu/drm/i915/i915_gem.c | 3 ++- > > drivers/gpu/drm/i915/i915_request.c | 6 +- > > drivers/gpu/drm/i915/i915_utils.h| 6 ++ > > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 > > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- > > drivers/gpu/drm/i915/selftests/mock_engine.c | 4 > > drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 +- > > 8 files changed, 16 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 73936be90aed..a7787c2cb53c 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2060,7 +2060,7 @@ struct drm_i915_private { > > > > struct i915_gem_timeline global_timeline; > > struct list_head timelines; > > - struct list_head rings; > > + struct list_head live_rings; > > u32 active_requests; > > > > /** > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c > > index 906e2395c245..0097a77fae8d 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -141,6 +141,7 @@ static u32 __i915_gem_park(struct drm_i915_private > > *i915) > > { > > lockdep_assert_held(&i915->drm.struct_mutex); > > GEM_BUG_ON(i915->gt.active_requests); > > + GEM_BUG_ON(!list_empty(&i915->gt.live_rings)); > > > > if (!i915->gt.awake) > > return I915_EPOCH_INVALID; > > @@ -5600,7 +5601,7 @@ int i915_gem_init_early(struct drm_i915_private > > *dev_priv) > > goto err_dependencies; > > > > mutex_lock(&dev_priv->drm.struct_mutex); > > - INIT_LIST_HEAD(&dev_priv->gt.rings); > > + INIT_LIST_HEAD(&dev_priv->gt.live_rings); > > INIT_LIST_HEAD(&dev_priv->gt.timelines); > > err = i915_gem_timeline_init__global(dev_priv); > > mutex_unlock(&dev_priv->drm.struct_mutex); > > diff --git a/drivers/gpu/drm/i915/i915_request.c > > b/drivers/gpu/drm/i915/i915_request.c > > index 0bf949ec9f1a..534b8d684cef 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -316,6 +316,7 @@ static void advance_ring(struct i915_request *request) > >* noops - they are safe to be replayed on a reset. > >*/ > > tail = READ_ONCE(request->tail); > > + list_del(&ring->live); > > } else { > > tail = request->postfix; > > } > > @@ -1046,6 +1047,8 @@ void __i915_request_add(struct i915_request *request, > > bool flush_caches) > > i915_gem_active_set(&timeline->last_request, request); > > > > list_add_tail(&request->ring_link, &ring->request_list); > > + if (list_is_first(&request->ring_link, &ring->request_list)) > > + list_add(&ring->live, &request->i915->gt.live_rings); > > If you re-order the two list adds you could use list_empty and wouldn't > have to add list_is_first. list_is_first tallies nicely with the list_is_last used before the corresponding list_del. > > > request->emitted_jiffies = jiffies; > > > > /* > > @@ -1375,7 +1378,8 @@ void i915_retire_requests(struct drm_i915_private > > *i915) > > if (!i915->gt.active_requests) > > return; > > > > - list_for_each_entry_safe(ring, next, &i915->gt.rings, link) > > + GEM_BUG_ON(list_empty(&i915->gt.live_rings)); > > Maybe blank line here since the assert is not logically associated with > the list but with the !i915.active_requests? I was thinking list when I wrote it. It's small enough we can argue both and both be right. > > > + list_for_each_entry_safe(ring, next, &i915->gt.live_rings, live) > > ring_retire_requests(ring); > > } > > > > diff --git a/drivers/gpu/drm/i915/i915_utils.h > > b/drivers/gpu/drm/i915/i915_utils.h > > index 0695717522ea..00165ad55fb3 100644 > > --- a/drivers/gpu/drm/i915/i915_utils.h > > +++ b/drivers/gpu/drm/i915/i915_utils.h > > @@ -120,6 +120,12 @@ static inline u64 ptr_to_u64(const void *ptr) > > > > #include > > > > +static inline int list_is_first(const struct list_head *list, > > + const struct list_head *head) > > Return bool if you decide you prefer to keep list_is_first
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Only track live rings for retiring
On 23/04/2018 11:13, Chris Wilson wrote: We don't need to track every ring for its lifetime as they are managed by the contexts/engines. What we do want to track are the live rings so that we can sporadically clean up requests if userspace falls behind. We can simply restrict the gt->rings list to being only gt->live_rings. Suggested-by: Tvrtko Ursulin Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gem.c | 3 ++- drivers/gpu/drm/i915/i915_request.c | 6 +- drivers/gpu/drm/i915/i915_utils.h| 6 ++ drivers/gpu/drm/i915/intel_ringbuffer.c | 4 drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- drivers/gpu/drm/i915/selftests/mock_engine.c | 4 drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 +- 8 files changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 73936be90aed..a7787c2cb53c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2060,7 +2060,7 @@ struct drm_i915_private { struct i915_gem_timeline global_timeline; struct list_head timelines; - struct list_head rings; + struct list_head live_rings; u32 active_requests; /** diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 906e2395c245..0097a77fae8d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -141,6 +141,7 @@ static u32 __i915_gem_park(struct drm_i915_private *i915) { lockdep_assert_held(&i915->drm.struct_mutex); GEM_BUG_ON(i915->gt.active_requests); + GEM_BUG_ON(!list_empty(&i915->gt.live_rings)); if (!i915->gt.awake) return I915_EPOCH_INVALID; @@ -5600,7 +5601,7 @@ int i915_gem_init_early(struct drm_i915_private *dev_priv) goto err_dependencies; mutex_lock(&dev_priv->drm.struct_mutex); - INIT_LIST_HEAD(&dev_priv->gt.rings); + INIT_LIST_HEAD(&dev_priv->gt.live_rings); INIT_LIST_HEAD(&dev_priv->gt.timelines); err = i915_gem_timeline_init__global(dev_priv); mutex_unlock(&dev_priv->drm.struct_mutex); diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 0bf949ec9f1a..534b8d684cef 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -316,6 +316,7 @@ static void advance_ring(struct i915_request *request) * noops - they are safe to be replayed on a reset. */ tail = READ_ONCE(request->tail); + list_del(&ring->live); } else { tail = request->postfix; } @@ -1046,6 +1047,8 @@ void __i915_request_add(struct i915_request *request, bool flush_caches) i915_gem_active_set(&timeline->last_request, request); list_add_tail(&request->ring_link, &ring->request_list); + if (list_is_first(&request->ring_link, &ring->request_list)) + list_add(&ring->live, &request->i915->gt.live_rings); If you re-order the two list adds you could use list_empty and wouldn't have to add list_is_first. request->emitted_jiffies = jiffies; /* @@ -1375,7 +1378,8 @@ void i915_retire_requests(struct drm_i915_private *i915) if (!i915->gt.active_requests) return; - list_for_each_entry_safe(ring, next, &i915->gt.rings, link) + GEM_BUG_ON(list_empty(&i915->gt.live_rings)); Maybe blank line here since the assert is not logically associated with the list but with the !i915.active_requests? + list_for_each_entry_safe(ring, next, &i915->gt.live_rings, live) ring_retire_requests(ring); } diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index 0695717522ea..00165ad55fb3 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -120,6 +120,12 @@ static inline u64 ptr_to_u64(const void *ptr) #include +static inline int list_is_first(const struct list_head *list, + const struct list_head *head) Return bool if you decide you prefer to keep list_is_first? +{ + return head->next == list; +} + static inline void __list_del_many(struct list_head *head, struct list_head *first) { diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 792a2ca95872..3453e7426f6b 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1150,8 +1150,6 @@ intel_engine_create_ring(struct intel_engine_cs *engine, int size) } ring->vma = vma; - list_add(&ring->link, &engine->i915->gt.rings); - return ring; } @@ -1163,8 +1161,
[Intel-gfx] [PATCH 3/6] drm/i915: Only track live rings for retiring
We don't need to track every ring for its lifetime as they are managed by the contexts/engines. What we do want to track are the live rings so that we can sporadically clean up requests if userspace falls behind. We can simply restrict the gt->rings list to being only gt->live_rings. Suggested-by: Tvrtko Ursulin Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gem.c | 3 ++- drivers/gpu/drm/i915/i915_request.c | 6 +- drivers/gpu/drm/i915/i915_utils.h| 6 ++ drivers/gpu/drm/i915/intel_ringbuffer.c | 4 drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- drivers/gpu/drm/i915/selftests/mock_engine.c | 4 drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 +- 8 files changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 73936be90aed..a7787c2cb53c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2060,7 +2060,7 @@ struct drm_i915_private { struct i915_gem_timeline global_timeline; struct list_head timelines; - struct list_head rings; + struct list_head live_rings; u32 active_requests; /** diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 906e2395c245..0097a77fae8d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -141,6 +141,7 @@ static u32 __i915_gem_park(struct drm_i915_private *i915) { lockdep_assert_held(&i915->drm.struct_mutex); GEM_BUG_ON(i915->gt.active_requests); + GEM_BUG_ON(!list_empty(&i915->gt.live_rings)); if (!i915->gt.awake) return I915_EPOCH_INVALID; @@ -5600,7 +5601,7 @@ int i915_gem_init_early(struct drm_i915_private *dev_priv) goto err_dependencies; mutex_lock(&dev_priv->drm.struct_mutex); - INIT_LIST_HEAD(&dev_priv->gt.rings); + INIT_LIST_HEAD(&dev_priv->gt.live_rings); INIT_LIST_HEAD(&dev_priv->gt.timelines); err = i915_gem_timeline_init__global(dev_priv); mutex_unlock(&dev_priv->drm.struct_mutex); diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 0bf949ec9f1a..534b8d684cef 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -316,6 +316,7 @@ static void advance_ring(struct i915_request *request) * noops - they are safe to be replayed on a reset. */ tail = READ_ONCE(request->tail); + list_del(&ring->live); } else { tail = request->postfix; } @@ -1046,6 +1047,8 @@ void __i915_request_add(struct i915_request *request, bool flush_caches) i915_gem_active_set(&timeline->last_request, request); list_add_tail(&request->ring_link, &ring->request_list); + if (list_is_first(&request->ring_link, &ring->request_list)) + list_add(&ring->live, &request->i915->gt.live_rings); request->emitted_jiffies = jiffies; /* @@ -1375,7 +1378,8 @@ void i915_retire_requests(struct drm_i915_private *i915) if (!i915->gt.active_requests) return; - list_for_each_entry_safe(ring, next, &i915->gt.rings, link) + GEM_BUG_ON(list_empty(&i915->gt.live_rings)); + list_for_each_entry_safe(ring, next, &i915->gt.live_rings, live) ring_retire_requests(ring); } diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index 0695717522ea..00165ad55fb3 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -120,6 +120,12 @@ static inline u64 ptr_to_u64(const void *ptr) #include +static inline int list_is_first(const struct list_head *list, + const struct list_head *head) +{ + return head->next == list; +} + static inline void __list_del_many(struct list_head *head, struct list_head *first) { diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 792a2ca95872..3453e7426f6b 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1150,8 +1150,6 @@ intel_engine_create_ring(struct intel_engine_cs *engine, int size) } ring->vma = vma; - list_add(&ring->link, &engine->i915->gt.rings); - return ring; } @@ -1163,8 +1161,6 @@ intel_ring_free(struct intel_ring *ring) i915_vma_close(ring->vma); __i915_gem_object_release_unless_active(obj); - list_del(&ring->link); - kfree(ring); } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index d816f8dea245..fd5a