On Wed, Mar 8, 2017 at 12:24 AM, Iago Toral <ito...@igalia.com> wrote:
> 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? > I'm not sure what you mean. If blorp_emit calls blorp_emit_dwords which calls anv_batch_emit_dwords which handles OOM by setting anv_batch::status to VK_OUT_OF_DEVICE_MEMORY and returning NULL, then that should take care of almost everything. We would also need anv_cmd_buffer_alloc_dynamic_state and anv_cmd_buffer_alloc_blorp_binding_table to do the right thing. I really don't think this will require massive BLORP changes. > 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(-) > > > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
