[Mesa-dev] [PATCH v2 05/24] anv/cmd_buffer: add a status field to anv_batch

[Iago Toral]

On Wed, 2017-03-15 at 10:33 +0200, Pohjolainen, Topi wrote:
> On Fri, Mar 10, 2017 at 01:38:18PM +0100, Iago Toral Quiroga wrote:
> > 
> > The vkCmd*() functions do not report errors, instead, any errors
> > should be
> > reported by the time we call vkEndCommandBuffer(). This means that
> > we
> > need to make the driver robust against incosistent and/or
> > imcomplete
> > command  buffer states through the command recording process,
> > particularly,
> > avoid crashes due to access to memory that we failed to allocate
> > previously.
> > 
> > The strategy used to do this is to track the first error ocurred
> > while
> > recording a command buffer in the batch associated with it. We use
> > the
> > batch to track this information because the command buffer may not
> > be
> > visible to all parts of the driver that can produce errors we need
> > to be
> > aware of (such as allocation failures during batch emissions).
> > 
> > Later patches will use this error information to guard parts of the
> > driver
> > that may not be safe to execute.
> > 
> > v2: Move the field from the command buffer to the batch so we can
> > track
> >     errors from batch emissions (Jason)
> > ---
> >  src/intel/vulkan/anv_cmd_buffer.c | 5 +++++
> >  src/intel/vulkan/anv_private.h    | 9 +++++++++
> >  2 files changed, 14 insertions(+)
> > 
> > diff --git a/src/intel/vulkan/anv_cmd_buffer.c
> > b/src/intel/vulkan/anv_cmd_buffer.c
> > index cab1dd7..4a41bc0 100644
> > --- a/src/intel/vulkan/anv_cmd_buffer.c
> > +++ b/src/intel/vulkan/anv_cmd_buffer.c
> > @@ -117,6 +117,8 @@ anv_cmd_state_reset(struct anv_cmd_buffer
> > *cmd_buffer)
> >  {
> >     struct anv_cmd_state *state = &cmd_buffer->state;
> >  
> > +   cmd_buffer->batch.status = VK_SUCCESS;
> > +
> >     memset(&state->descriptors, 0, sizeof(state->descriptors));
> >     memset(&state->push_constants, 0, sizeof(state-
> > >push_constants));
> >     memset(state->binding_tables, 0, sizeof(state-
> > >binding_tables));
> > @@ -185,6 +187,8 @@ static VkResult anv_create_cmd_buffer(
> >     if (cmd_buffer == NULL)
> >        return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> >  
> > +   cmd_buffer->batch.status = VK_SUCCESS;
> > +
> >     cmd_buffer->_loader_data.loaderMagic = ICD_LOADER_MAGIC;
> >     cmd_buffer->device = device;
> >     cmd_buffer->pool = pool;
> > @@ -217,6 +221,7 @@ static VkResult anv_create_cmd_buffer(
> >     return VK_SUCCESS;
> >  
> >   fail:
> > +   cmd_buffer->batch.status = result;
> This looks unnecessary, cmd_buffer gets deallocated in the next line.

Oops, right, I'll remove this. Thanks!

> > 
> >     vk_free(&cmd_buffer->pool->alloc, cmd_buffer);
> >  
> >     return result;
> > diff --git a/src/intel/vulkan/anv_private.h
> > b/src/intel/vulkan/anv_private.h
> > index aa2b6d7..f5d7600 100644
> > --- a/src/intel/vulkan/anv_private.h
> > +++ b/src/intel/vulkan/anv_private.h
> > @@ -704,6 +704,15 @@ struct anv_batch {
> >      */
> >     VkResult (*extend_cb)(struct anv_batch *, void *);
> >     void *                                       user_data;
> > +
> > +   /**
> > +    * Current error status of the command buffer. Used to track
> > inconsistent
> > +    * or incomplete command buffer states that are the consequence
> > of run-time
> > +    * errors such as out of memory scenarios. We want to track
> > this in the
> > +    * batch because the command buffer object is not visible to
> > some parts
> > +    * of the driver.
> > +    */
> > +   VkResult                                     status;
> >  };
> >  
> >  void *anv_batch_emit_dwords(struct anv_batch *batch, int
> > num_dwords);