[Intel-gfx] [PATCH 5/6] drm/i915: Simplify batch pool cache search

2015-02-26 Thread Chris Wilson
Combining list_del() with the list_for_each() is actually safe.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem_batch_pool.c | 41 +-
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c 
b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
index f1db6416d859..c0ad1f6503d7 100644
--- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c
+++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
@@ -97,10 +97,9 @@ 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, *next;
+   struct drm_i915_gem_object *obj;
struct list_head *list;
-   int n;
+   int ret, n;
 
WARN_ON(!mutex_is_locked(&pool->dev->struct_mutex));
 
@@ -109,37 +108,33 @@ 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_safe(tmp, next, list, batch_pool_link) {
-   if (tmp->active)
+   list_for_each_entry(obj, list, batch_pool_link) {
+   if (obj->active)
break;
 
/* While we're looping, do some clean up */
-   if (tmp->madv == __I915_MADV_PURGED) {
-   list_del(&tmp->batch_pool_link);
-   drm_gem_object_unreference(&tmp->base);
+   if (obj->madv == __I915_MADV_PURGED) {
+   list_del(&obj->batch_pool_link);
+   drm_gem_object_unreference(&obj->base);
continue;
}
 
-   if (tmp->base.size >= size) {
-   obj = tmp;
-   break;
-   }
+   if (obj->base.size >= size)
+   goto out;
}
 
-   if (obj == NULL) {
-   int ret;
-
-   obj = i915_gem_alloc_object(pool->dev, size);
-   if (obj == NULL)
-   return ERR_PTR(-ENOMEM);
+   /* None found, allocate a fresh bo and backing storage */
+   obj = i915_gem_alloc_object(pool->dev, size);
+   if (obj == NULL)
+   return ERR_PTR(-ENOMEM);
 
-   ret = i915_gem_object_get_pages(obj);
-   if (ret)
-   return ERR_PTR(ret);
+   ret = i915_gem_object_get_pages(obj);
+   if (ret)
+   return ERR_PTR(ret);
 
-   obj->madv = I915_MADV_DONTNEED;
-   }
+   obj->madv = I915_MADV_DONTNEED;
 
+out:
list_move_tail(&obj->batch_pool_link, list);
i915_gem_object_pin_pages(obj);
return obj;
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Simplify batch pool cache search

2015-02-26 Thread Chris Wilson
On Thu, Feb 26, 2015 at 10:05:17AM +, Chris Wilson wrote:
> Combining list_del() with the list_for_each() is actually safe.

Sigh. It's not, the iterator disappears and so iter = iter->next is
unsafe (I was just thinking that the value of iter->next is left
untouched by list_del()). Time to go shopping.
-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