[PATCH] drm/ttm: cleanup ttm_execbuf_util.c slightly more

[Jerome Glisse]

On Mon, Aug 20, 2012 at 10:36 AM, Maarten Lankhorst
<maarten.lankhorst at canonical.com> wrote:
> The removed member is unneeded, an extra pass is done after all
> buffers have been reserved. The behavior stays the same even without
> the removed member, but this makes the code slightly more readable.
>
> Depends on previous 2 execbuf-util patches.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>

NAK this one modify lru list handling in wrong way, we need to track
lru per object no way around it.

Cheers,
Jerome

> ---
>  drivers/gpu/drm/ttm/ttm_execbuf_util.c |   69 +++++++++++---------------------
>  1 file changed, 24 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> index 4e7b596..a545bc9 100644
> --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> @@ -32,7 +32,7 @@
>  #include <linux/sched.h>
>  #include <linux/module.h>
>
> -static void ttm_eu_backoff_reservation_locked(struct list_head *list)
> +static void ttm_eu_backoff_reservation_locked(struct list_head *list, bool removed)
>  {
>         struct ttm_validate_buffer *entry;
>
> @@ -41,43 +41,13 @@ static void ttm_eu_backoff_reservation_locked(struct list_head *list)
>                 if (!entry->reserved)
>                         continue;
>
> -               if (entry->removed) {
> -                       ttm_bo_add_to_lru(bo);
> -                       entry->removed = false;
> -
> -               }
>                 entry->reserved = false;
> -               atomic_set(&bo->reserved, 0);
> -               wake_up_all(&bo->event_queue);
> -       }
> -}
> -
> -static void ttm_eu_del_from_lru_locked(struct list_head *list)
> -{
> -       struct ttm_validate_buffer *entry;
> -
> -       list_for_each_entry(entry, list, head) {
> -               struct ttm_buffer_object *bo = entry->bo;
> -               if (!entry->reserved)
> -                       continue;
>
> -               if (!entry->removed) {
> -                       entry->put_count = ttm_bo_del_from_lru(bo);
> -                       entry->removed = true;
> -               }
> -       }
> -}
> -
> -static void ttm_eu_list_ref_sub(struct list_head *list)
> -{
> -       struct ttm_validate_buffer *entry;
> -
> -       list_for_each_entry(entry, list, head) {
> -               struct ttm_buffer_object *bo = entry->bo;
> -
> -               if (entry->put_count) {
> -                       ttm_bo_list_ref_sub(bo, entry->put_count, true);
> -                       entry->put_count = 0;
> +               if (removed) {
> +                       ttm_bo_unreserve_locked(bo);
> +               } else {
> +                       atomic_set(&bo->reserved, 0);
> +                       wake_up_all(&bo->event_queue);
>                 }
>         }
>  }
> @@ -93,7 +63,7 @@ void ttm_eu_backoff_reservation(struct list_head *list)
>         entry = list_first_entry(list, struct ttm_validate_buffer, head);
>         glob = entry->bo->glob;
>         spin_lock(&glob->lru_lock);
> -       ttm_eu_backoff_reservation_locked(list);
> +       ttm_eu_backoff_reservation_locked(list, true);
>         spin_unlock(&glob->lru_lock);
>  }
>  EXPORT_SYMBOL(ttm_eu_backoff_reservation);
> @@ -122,8 +92,6 @@ int ttm_eu_reserve_buffers(struct list_head *list)
>
>         list_for_each_entry(entry, list, head) {
>                 entry->reserved = false;
> -               entry->put_count = 0;
> -               entry->removed = false;
>         }
>
>         entry = list_first_entry(list, struct ttm_validate_buffer, head);
> @@ -141,26 +109,37 @@ retry:
>                 case 0:
>                         break;
>                 case -EAGAIN:
> -                       ttm_eu_backoff_reservation_locked(list);
> +                       ttm_eu_backoff_reservation_locked(list, false);
>                         spin_unlock(&glob->lru_lock);
> -                       ttm_eu_list_ref_sub(list);
>                         ret = ttm_bo_wait_unreserved(bo, true);
>                         if (unlikely(ret != 0))
>                                 return ret;
>                         goto retry;
>                 default:
> -                       ttm_eu_backoff_reservation_locked(list);
> +                       ttm_eu_backoff_reservation_locked(list, false);
>                         spin_unlock(&glob->lru_lock);
> -                       ttm_eu_list_ref_sub(list);
>                         return ret;
>                 }
>
>                 entry->reserved = true;
>         }
>
> -       ttm_eu_del_from_lru_locked(list);
> +       list_for_each_entry(entry, list, head) {
> +               struct ttm_buffer_object *bo = entry->bo;
> +
> +               entry->put_count = ttm_bo_del_from_lru(bo);
> +       }
> +
>         spin_unlock(&glob->lru_lock);
> -       ttm_eu_list_ref_sub(list);
> +
> +       list_for_each_entry(entry, list, head) {
> +               struct ttm_buffer_object *bo = entry->bo;
> +
> +               if (entry->put_count) {
> +                       ttm_bo_list_ref_sub(bo, entry->put_count, true);
> +                       entry->put_count = 0;
> +               }
> +       }
>
>         return 0;
>  }
> diff --git a/include/drm/ttm/ttm_execbuf_util.h b/include/drm/ttm/ttm_execbuf_util.h
> index 26cc7f9..0ef7c95 100644
> --- a/include/drm/ttm/ttm_execbuf_util.h
> +++ b/include/drm/ttm/ttm_execbuf_util.h
> @@ -42,7 +42,6 @@
>   * @new_sync_obj_arg: New sync_obj_arg for @bo, to be used once
>   * adding a new sync object.
>   * @reserved:       Indicates whether @bo has been reserved for validation.
> - * @removed:        Indicates whether @bo has been removed from lru lists.
>   * @put_count:      Number of outstanding references on bo::list_kref.
>   * @old_sync_obj:   Pointer to a sync object about to be unreferenced
>   */
> @@ -52,7 +51,6 @@ struct ttm_validate_buffer {
>         struct ttm_buffer_object *bo;
>         void *new_sync_obj_arg;
>         bool reserved;
> -       bool removed;
>         int put_count;
>         void *old_sync_obj;
>  };
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel