Luca,
Good catch.
Some comments inline:
Luca Barbieri wrote:
> + entry = list_first_entry(&bdev->ddestroy,
> + struct ttm_buffer_object, ddestroy);
> + kref_get(&entry->list_kref);
>
> - if (next != &bdev->ddestroy) {
> - nentry = list_entry(next, struct ttm_buffer_object,
> - ddestroy);
> + for (;;) {
> + struct ttm_buffer_object *nentry = NULL;
> +
> + if (!list_empty(&entry->ddestroy)
> + && entry->ddestroy.next != &bdev->ddestroy) {
> + nentry = list_entry(entry->ddestroy.next,
> + struct ttm_buffer_object, ddestroy);
>
I'm not sure it's considered ok to access the list_head members directly
in new code.
Would nentry=list_first_entry(&entry->ddestroy, ....) work?
> kref_get(&nentry->list_kref);
> }
>
else unlock and break? That would save an unnecessary call to
ttm_bo_cleanup_refs.
> - kref_get(&entry->list_kref);
>
> spin_unlock(&glob->lru_lock);
> ret = ttm_bo_cleanup_refs(entry, remove_all);
> kref_put(&entry->list_kref, ttm_bo_release_list);
> + entry = nentry;
>
Here nentry may have been removed from the list by another process,
which would trigger the unnecessary call, mentioned above.
/Thomas
_______________________________________________
Nouveau mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/nouveau
