On 31 August 2015 at 19:26, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > 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@xxxxxxxxxxxxxxxxxx> >> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> Signed-off-by: Emil Velikov <emil.l.velikov@xxxxxxxxx> >> --- >> 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. > Doesn't this concert apply to drm_intel_gem_bo_exec() as well ? > 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]); How about we do this as a follow up patch (4.1/17) that covers both functions ? Thanks again, Emil _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx