[Mesa-dev] [PATCH 03/16] anv: don't crash if we fail to grow the reloc list

[Jason Ekstrand]

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

> On Tue, 2017-03-07 at 08:44 -0800, Jason Ekstrand wrote:
> > On Mon, Mar 6, 2017 at 11:15 PM, Iago Toral Quiroga <itoral at igalia.co
> > m> wrote:
> > > ---
> > >  src/intel/vulkan/anv_batch_chain.c | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/anv_batch_chain.c
> > > b/src/intel/vulkan/anv_batch_chain.c
> > > index 3f6039e..4cbfb4d 100644
> > > --- a/src/intel/vulkan/anv_batch_chain.c
> > > +++ b/src/intel/vulkan/anv_batch_chain.c
> > > @@ -151,8 +151,9 @@ anv_reloc_list_add(struct anv_reloc_list *list,
> > >     const uint32_t domain =
> > >        target_bo->is_winsys_bo ? I915_GEM_DOMAIN_RENDER : 0;
> > >
> > > -   anv_reloc_list_grow(list, alloc, 1);
> > > -   /* TODO: Handle failure */
> > > +   VkResult result = anv_reloc_list_grow(list, alloc, 1);
> > > +   if (result != VK_SUCCESS)
> > > +      return 0;
> > This prevents crashes, yes, but it doesn't flag the failure anywhere
> > so it gets lost and you now have  command buffer that references
> > something but doesn't have a relocaiton.
> >
> > I think the right thing to do would be to add an
> > anv_batch_emit_surface_reloc helper and then we have a more central
> > place to handle it.
>
> Actually, we already have anv_batch_emit_reloc() which simply calls
> anv_reloc_list_add, so I think we can use that.
>

No, that emits a relocation on the batch.  Emitting a relocation on the
surface state BO is a bit different.


> > Would it make sense to put the VkResult status in the anv_batch?
> > This can get called from either batch building or pipeline building.
>
> Yes, I think this is a good idea since the batch associated with the
> command buffer is the only thing we have available during batch
> emissions.
>

Yeah, if we did that, we could handle OOM even in things such as
anv_batch_emit()


> > >
> > >     /* XXX: Can we use I915_EXEC_HANDLE_LUT? */
> > >     index = list->num_relocs++;
> > > @@ -174,8 +175,9 @@ anv_reloc_list_append(struct anv_reloc_list
> > > *list,
> > >                        const VkAllocationCallbacks *alloc,
> > >                        struct anv_reloc_list *other, uint32_t
> > > offset)
> > >  {
> > > -   anv_reloc_list_grow(list, alloc, other->num_relocs);
> > > -   /* TODO: Handle failure */
> > > +   VkResult result = anv_reloc_list_grow(list, alloc, other-
> > > >num_relocs);
> > > +   if (result != VK_SUCCESS)
> > > +      return;
> > Similarly, this one should handle failure.
> >
> > >
> > >     memcpy(&list->relocs[list->num_relocs], &other->relocs[0],
> > >            other->num_relocs * sizeof(other->relocs[0]));
> > > --
> > > 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/fc68b170/attachment.html>