[Nouveau] [PATCH] drm/ttm: Fix race condition in ttm_bo_delayed_delete

Thomas Hellstrom thellstrom at vmware.com
Mon Jan 18 11:40:57 PST 2010


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



More information about the Nouveau mailing list