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 Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau