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

[Iago Toral]

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.

> 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.

> > 
> >     /* 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
> >