| Message ID | 20120820143612.21905.68138.stgit@patser.local (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 20, 2012 at 10:36 AM, Maarten Lankhorst <maarten.lankhorst@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@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
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; };
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@canonical.com> --- drivers/gpu/drm/ttm/ttm_execbuf_util.c | 69 +++++++++++--------------------- 1 file changed, 24 insertions(+), 45 deletions(-)