[Mesa-dev] [PATCH 08/16] anv/cmd_buffer: track error scenarios during command buffer recording

[Jason Ekstrand]

On Mon, Mar 6, 2017 at 11:15 PM, Iago Toral Quiroga <itoral at igalia.com>
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 track things such as "out of host memory" and use that information
> to avoid executing code that could lead to crashes (due to the fact that
> the
> command buffer could not allocate the memory it needs) and report the error
> when the client calls vkEndCommandBuffer().
>
> Notice this patch is not sufficient to track and report all possible cases
> of out of host memory situations that can be produced while recording
> command buffers. Later patches will fix some of the missing cases. Also,
> some allocations may occur deep into the driver code (for example during
> batch emissions) and are more difficult to track because at that point we
> don't even have access to the command buffer object. Dealing with these
> scenarios would require more changes but for now this is sufficient to make
> CTS happy.
>

Thinking about things a bit more, I don't think that is actually all that
hard...


> Fixes:
> dEQP-VK.api.out_of_host_memory.cmd_begin_render_pass
> ---
>  src/intel/vulkan/anv_cmd_buffer.c  |  5 +++++
>  src/intel/vulkan/anv_private.h     |  7 +++++++
>  src/intel/vulkan/genX_cmd_buffer.c | 22 +++++++++++++++++++---
>  3 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_cmd_buffer.c b/src/intel/vulkan/anv_cmd_
> buffer.c
> index cab1dd7..01447b0 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->error_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->error_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->error_status = result;
>     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 d3e32bc..46b51a9 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -1377,6 +1377,13 @@ struct anv_cmd_buffer {
>     VkCommandBufferLevel                         level;
>
>     struct anv_cmd_state                         state;
> +
> +   /**
> +    * 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.
> +    */
> +   VkResult                                     error_status;
>

If we put this in anv_batch, we can easily set the status from "deep" parts
of the driver.  Also, I'm not sure the "error" part of the name is needed.
"status" should be ok.

One other thing we may want to do is to add a anv_batch_set_error helper
and only take first error.  That way we can track OUT_OF_HOST_MEMORY vs.
OUT_OF_DEVICE_MEMORY.


>  };
>
>  VkResult anv_cmd_buffer_init_batch_bo_chain(struct anv_cmd_buffer
> *cmd_buffer);
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index b46a922..cbad144 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -401,8 +401,8 @@ genX(cmd_buffer_setup_attachments)(struct
> anv_cmd_buffer *cmd_buffer,
>                                        sizeof(state->attachments[0]),
>                                   8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
>     if (state->attachments == NULL) {
> -      /* FIXME: Propagate VK_ERROR_OUT_OF_HOST_MEMORY to
> vkEndCommandBuffer */
> -      return VK_ERROR_OUT_OF_HOST_MEMORY;
> +      /* Propagate VK_ERROR_OUT_OF_HOST_MEMORY to vkEndCommandBuffer */
> +      return cmd_buffer->error_status = VK_ERROR_OUT_OF_HOST_MEMORY;
>     }
>
>     bool need_null_state = false;
> @@ -615,6 +615,9 @@ genX(EndCommandBuffer)(
>  {
>     ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);
>
> +   if (cmd_buffer->error_status != VK_SUCCESS)
> +      return cmd_buffer->error_status;
> +
>     /* We want every command buffer to start with the PMA fix in a known
> state,
>      * so we disable it at the end of the command buffer.
>      */
> @@ -2470,7 +2473,14 @@ void genX(CmdBeginRenderPass)(
>     cmd_buffer->state.framebuffer = framebuffer;
>     cmd_buffer->state.pass = pass;
>     cmd_buffer->state.render_area = pRenderPassBegin->renderArea;
> -   genX(cmd_buffer_setup_attachments)(cmd_buffer, pass,
> pRenderPassBegin);
> +   VkResult result =
> +      genX(cmd_buffer_setup_attachments)(cmd_buffer, pass,
> pRenderPassBegin);
> +
> +   /* If we failed to setup the attachments we should not try to go
> further */
> +   if (result != VK_SUCCESS) {
> +      assert(cmd_buffer->error_status != VK_SUCCESS);
> +      return;
> +   }
>
>     genX(flush_pipeline_select_3d)(cmd_buffer);
>
> @@ -2483,6 +2493,9 @@ void genX(CmdNextSubpass)(
>  {
>     ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);
>
> +   if (cmd_buffer->error_status != VK_SUCCESS)
> +      return;
> +
>     assert(cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_PRIMARY);
>
>     anv_cmd_buffer_resolve_subpass(cmd_buffer);
> @@ -2499,6 +2512,9 @@ void genX(CmdEndRenderPass)(
>  {
>     ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);
>
> +   if (cmd_buffer->error_status != VK_SUCCESS)
> +      return;
> +
>     anv_cmd_buffer_resolve_subpass(cmd_buffer);
>
>     /* Perform transitions to the final layout after all writes have
> occurred.
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170307/72e3322b/attachment.html>