[Mesa-dev] [PATCH 00/16] anv: Handle out of memory situations better

[Iago Toral]

On Tue, 2017-03-07 at 10:31 +0200, Pohjolainen, Topi wrote:
> On Tue, Mar 07, 2017 at 08:15:45AM +0100, Iago Toral Quiroga wrote:
> > 
> > There are a number of work-in-progress CTS tests that check OOM
> > handling
> > and raised a number of issues that this series addresses.
> > 
> > Particularly noteworthy is the case of command buffer recording:
> > since the
> > various vkCmd*() functions do not return errors, it is expected
> > that drivers survive until vkEndCommanBuffer() and report any
> > errors
> > at that point. This requires that we are particularly careful with
> > out of
> > memory scenarios, since we need to make sure that vkCmd*() commands
> > do not
> > attempt to access memory that we have failed to allocate in a
> > previous step.
> > We achieve this by tracking errors generated during command buffer
> > recording
> > into the command buffer object and, generally, avoiding to execute
> > new
> > vkCmd*() functions if the command buffer recording has generated an
> > error
> > before. In general, I tried to  only guard execution of vkCmd*()
> > commands that
> > the tests showed needed to be guarded since most of the vkCmd*()
> > commands are
> > safe to execute in any scenario.
> > 
> > This series fixes all the issues raised with the new tests and a
> > few more that
> > I found by inspection but I am sure we could do better in more
> > places.
> > It is a first step though and the tests are still in development.
> > 
> > Iago Toral Quiroga (16):
> >   blorp: handle out of memory in blorp_emitn()
> >   blorp: handle out of memory without crashing in various batch
> >     emissions
> These two go sort of half way - blorp_exec() still ignores the return
> values of blorp_emit_vertex_buffers() and
> blorp_emit_vertex_elements().
> Neither does blorp_emit_depth_stencil_state() returning zero make
> blorp_exec() to take any further action. 

Right, I focused on avoiding the crashes only because there are a lot
of other places where we do the same thing. Specifically, the
blorp_emit() macro that we use all over the place, suffers from this
same issue: it prevents the crash by avoiding the loop body to execute
if we failed to allocate memory, but it does nothing to report the
allocation failure back to its callers, so I figured it would not make
sense to do something different for blorp_emitn().

I am fine with being more robust here and try to address the problem
more thoroughly if you think it makes sense, but if we are going to do
that, then I think we would need to look beyond this macro and do a lot
more changes. Do you want me to work on that?

In any case, even if we agree on doing this, since these patches put
blorp_emitn() at the same level as blorp_emit() and they solve some
problems, would it be okay to land this series first and tackle that in
a second series?

Iago

> > 
> >   anv: don't crash if we fail to grow the reloc list
> >   anv: handle out of memory situations in anv_batch_emit_dwords()
> >   anv: do not try to ref/unref NULL shaders
> >   anv/blorp: return early if we failed to create the shader binary
> >   anv/cmd_buffer: return a VkResult in cmd_buffer_setup_attachments
> >   anv/cmd_buffer: track error scenarios during command buffer
> > recording
> >   anv: handle out of memory scenarios in anv_batch_emit_batch()
> >   anv/cmd_buffer: handle out of memory during vkCmdPushConstants
> >   anv: handle out of memory situations during queue submissions
> >   anv: ensure that we don't ever try to adjust relocations more
> > than
> >     once
> >   anv/cmd_buffer: skip vkCmdDraw*() on broken command buffers
> >   anv/cmd_buffer: skip vkCmdDispatch() on broken command buffers
> >   anv/cmd_buffer: skip vkCmdExecuteCommands() on broken command
> > buffers
> >   anv/cmd_buffer: handle out of device memory during binding table
> >     emission
> > 
> >  src/intel/blorp/blorp_genX_exec.h  | 25 ++++++++++-----
> >  src/intel/vulkan/anv_batch_chain.c | 45 +++++++++++++++++++-------
> > -
> >  src/intel/vulkan/anv_blorp.c       |  3 ++
> >  src/intel/vulkan/anv_cmd_buffer.c  | 21 +++++++++++--
> >  src/intel/vulkan/anv_private.h     | 19 ++++++++++++
> >  src/intel/vulkan/genX_cmd_buffer.c | 63
> > ++++++++++++++++++++++++++++++++------
> >  6 files changed, 142 insertions(+), 34 deletions(-)
> >