[PATCH i-g-t] lib/intel_allocator: protect binding for multithreaded tests

[Francois Dugast]

On Thu, Apr 03, 2025 at 10:34:38AM +0200, Zbigniew Kempczyński wrote:
> Upcoming render-copy stress test reveals problem where multiple
> threads enter binding code. Before binding/unbinding threads calls
> alloc/free which collects operations to be executed in single vm_bind
> ioctl. If first thread which fetches all binding operations from the
> allocator will be interleaved by second thread which in turn sees no
> operations are necessary to complete, it will start to execute jobs
> without properly bounded offsets in the vm (it thinks addresses are
> already there as there's no operations in the allocator).
> 
> Avoid this situation by migrating mutex lock to the place where thread
> performing binding will finish its job. This will ensure any other
> threads will wait until all binding operations are completed.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Cc: Francois Dugast <francois.dugast at intel.com>

I confirm this solves the problem seen with another series [1] on LNL:

    (xe_render_copy:8244) [thread:8970] intel_ctx-CRITICAL: Test assertion failure function intel_ctx_xe_exec, file ../lib/intel_ctx.c:467:
    (xe_render_copy:8244) [thread:8970] intel_ctx-CRITICAL: Failed assertion: __intel_ctx_xe_exec(ctx, ahnd, bb_offset) == 0
    (xe_render_copy:8244) [thread:8970] intel_ctx-CRITICAL: error: -125 != 0

So:
Reviewed-by: Francois Dugast <francois.dugast at intel.com>

[1] https://patchwork.freedesktop.org/patch/646197/?series=145831&rev=7#comment_1182552

Thanks,
Francois

> ---
>  lib/intel_allocator.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/intel_allocator.c b/lib/intel_allocator.c
> index 341db2a502..d22c70f1f7 100644
> --- a/lib/intel_allocator.c
> +++ b/lib/intel_allocator.c
> @@ -1535,10 +1535,11 @@ static void __xe_op_bind(struct ahnd_info *ainfo, uint32_t sync_in, uint32_t syn
>  			igt_map_remove(ainfo->bind_map, &obj->handle,
>  				       map_entry_free_func);
>  	}
> -	pthread_mutex_unlock(&ainfo->bind_map_mutex);
>  
>  	xe_bind_unbind_async(ainfo->fd, ainfo->vm, 0, &obj_list, sync_in, sync_out);
>  
> +	pthread_mutex_unlock(&ainfo->bind_map_mutex);
> +
>  	igt_list_for_each_entry_safe(entry, tmp, &obj_list, link) {
>  		igt_list_del(&entry->link);
>  		free(entry);
> -- 
> 2.34.1
>