[Mesa-dev] [PATCH 16/16] anv/cmd_buffer: handle out of device memory during binding table emission

[Jason Ekstrand]

On Wed, Mar 8, 2017 at 1:04 AM, Iago Toral <itoral at igalia.com> wrote:

> On Tue, 2017-03-07 at 13:25 -0800, Jason Ekstrand wrote:
> > On Mon, Mar 6, 2017 at 11:16 PM, Iago Toral Quiroga <itoral at igalia.co
> > m> wrote:
> > > ---
> > >  src/intel/vulkan/genX_cmd_buffer.c | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > > b/src/intel/vulkan/genX_cmd_buffer.c
> > > index 8b752c4..23d7493 100644
> > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > @@ -1112,7 +1112,7 @@ emit_binding_table(struct anv_cmd_buffer
> > > *cmd_buffer,
> > >     uint32_t *bt_map = bt_state->map;
> > >
> > >     if (bt_state->map == NULL)
> > > -      return VK_ERROR_OUT_OF_DEVICE_MEMORY;
> > > +      return cmd_buffer->error_status =
> > > VK_ERROR_OUT_OF_DEVICE_MEMORY;
> > >
> > >     if (stage == MESA_SHADER_COMPUTE &&
> > >         get_cs_prog_data(cmd_buffer->state.compute_pipeline)-
> > > >uses_num_work_groups) {
> > > @@ -1295,7 +1295,7 @@ emit_samplers(struct anv_cmd_buffer
> > > *cmd_buffer,
> > >     *state = anv_cmd_buffer_alloc_dynamic_state(cmd_buffer, size,
> > > 32);
> > >
> > >     if (state->map == NULL)
> > > -      return VK_ERROR_OUT_OF_DEVICE_MEMORY;
> > > +      return cmd_buffer->error_status =
> > > VK_ERROR_OUT_OF_DEVICE_MEMORY;
> > >
> > >     for (uint32_t s = 0; s < map->sampler_count; s++) {
> > >        struct anv_pipeline_binding *binding = &map-
> > > >sampler_to_descriptor[s];
> > > @@ -1345,6 +1345,7 @@ flush_descriptor_sets(struct anv_cmd_buffer
> > > *cmd_buffer)
> > >     if (result != VK_SUCCESS) {
> > >        assert(result == VK_ERROR_OUT_OF_DEVICE_MEMORY);
> > >
> > > +      cmd_buffer->error_status = VK_SUCCESS;
> > This doesn't seem right.  If we're setting it back to success in
> > anywhere other than cmd_buffer_reset, we're doing our error handling
> > wrong.
>
> The thing is that this seems to be a controlled error inside the
> driver, so we only really want to report an error if the fallback path
> below fails. The problem I see is that we might be stomping a previous
> error produced before this function, so maybe the right thing to do
> here is to simply avoid executing this function if we have seen an
> error for this command buffer before (that is, if error_status !=
> VK_SUCCESS by the time we call this function). That way, if we get here
> it is because we failed to allocate memory during binding table
> emission above and since that is an error we can recover from we can
> safely reset the error status for the fallback path below.
>

The other option would be to make emit_binding_table leave the command
buffer error state alone and wait to set it in flush_descriptor_sets.  Then
it could error all it wants and we could do a single re-try and if the
re-try fails, we set the error.


> > >        result = anv_cmd_buffer_new_binding_table_block(cmd_buffer);
> > >        assert(result == VK_SUCCESS);
> > >
> > > @@ -1817,6 +1818,8 @@ flush_compute_descriptor_set(struct
> > > anv_cmd_buffer *cmd_buffer)
> > >     result = emit_binding_table(cmd_buffer, MESA_SHADER_COMPUTE,
> > > &surfaces);
> > >     if (result != VK_SUCCESS) {
> > >        assert(result == VK_ERROR_OUT_OF_DEVICE_MEMORY);
> > > +
> > > +      cmd_buffer->error_status = VK_SUCCESS;
> > >        result = anv_cmd_buffer_new_binding_table_block(cmd_buffer);
> > >        assert(result == VK_SUCCESS);
> > >
> > > --
> > > 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/20170308/1cac614b/attachment.html>