[Intel-gfx] [PATCH libdrm v2] intel: error out on has_error in exec2

[Chris Wilson]

On Mon, Aug 31, 2015 at 07:14:12PM +0100, Emil Velikov wrote:
> Just like we do for the original exec()
> 
> v2: move bo_gem declaration to the top of the function.
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: intel-gfx at lists.freedesktop.org
> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> ---
>  intel/intel_bufmgr_gem.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index 7303903..5287419 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -2185,10 +2185,14 @@ do_exec2(drm_intel_bo *bo, int used, drm_intel_context *ctx,
>  	 unsigned int flags)
>  {
>  	drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bo->bufmgr;
> +	drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
>  	struct drm_i915_gem_execbuffer2 execbuf;
>  	int ret = 0;
>  	int i;
>  
> +	if (bo_gem->has_error)
> +		return -ENOMEM;
> +
>  	switch (flags & 0x7) {
>  	default:
>  		return -EINVAL;
> @@ -2259,8 +2263,7 @@ skip_execution:
>  		drm_intel_gem_dump_validation_list(bufmgr_gem);
>  
>  	for (i = 0; i < bufmgr_gem->exec_count; i++) {
> -		drm_intel_bo_gem *bo_gem =
> -			(drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
> +		bo_gem = (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];

Reusing bo_gem here is a little worrying as it would be very easy for
someone to add code to the end of the function thinking that bo_gem
still was the batch.

If we had

static inline drm_intel_bo_gem *to_bo_gem(drm_intel_bo *bo)
{
	return (drm_intel_bo_gem *)bo;
}

then we can start doing one offs like

if (to_bo_gem(bo)->has_error) return -ENOMEM;

and of course
	for (i = 0; i < bufmgr_gem->exec_count; i++) {
		drm_intel_bo_gem *bo_gem = to_bo_gem(bufmgr_gem->exec_bos[i]);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre