[Mesa-dev] [PATCH 3/3] r600g, radeonsi: attempt to fix racy multi-context apps calling BufferData

[Michel Dänzer]

On Son, 2014-03-09 at 02:24 +0100, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75061

[...]

> @@ -136,15 +137,23 @@ bool r600_init_resource(struct r600_common_screen *rscreen,
>  		res->domains = RADEON_DOMAIN_VRAM;
>  	}
>  
> -	/* Allocate the resource. */
> -	res->buf = rscreen->ws->buffer_create(rscreen->ws, size, alignment,
> -                                              use_reusable_pool,
> -                                              res->domains);
> -	if (!res->buf) {
> +	/* Allocate a new resource. */
> +	new_buf = rscreen->ws->buffer_create(rscreen->ws, size, alignment,
> +					     use_reusable_pool,
> +					     res->domains);
> +	if (!new_buf) {
>  		return false;
>  	}
>  
> +	/* Replace the pointer such that if res->buf wasn't NULL, it won't be
> +	 * NULL. This should prevent crashes with multiple contexts using
> +	 * the same buffer where one of the contexts invalidates it while
> +	 * the others are using it. */
> +	old_buf = res->buf;
> +	res->buf = new_buf; /* this should be atomic */
>  	res->cs_buf = rscreen->ws->buffer_get_cs_handle(res->buf);
> +	pb_reference(&old_buf, NULL);
> +

This removes the window where res->buf is NULL, but it leaves a small
window where res->cs_buf doesn't match res->buf. In case that might
still cause subtle issues, the window should be minimal like this:

	old_buf = res->buf;
	res->cs_buf = rscreen->ws->buffer_get_cs_handle(new_buf);
	res->buf = new_buf; /* this should be atomic */


You mentioned on IRC that there is a known bug in patch 1. Patch 2 is

Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer