[Mesa-dev] [PATCH v3 44/63] st/mesa: add infrastructure for storing bound texture/image handles

[James Legg]

On Fri, 2017-06-09 at 15:35 +0200, Samuel Pitoiset wrote:
> v2: - rename st_bound_handle to st_bound_handles
> 
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com> (v1)
> Reviewed-by: Marek Olšák <marek.olsak at amd.com> (v2)
> ---
>  src/mesa/state_tracker/st_context.c |  2 +
>  src/mesa/state_tracker/st_context.h | 11 ++++++
>  src/mesa/state_tracker/st_texture.c | 77
> +++++++++++++++++++++++++++++++++++++
>  src/mesa/state_tracker/st_texture.h |  5 +++
>  4 files changed, 95 insertions(+)

> diff --git a/src/mesa/state_tracker/st_texture.c
> b/src/mesa/state_tracker/st_texture.c
> index 7da111f39f..fdd727ec8e 100644
> --- a/src/mesa/state_tracker/st_texture.c
> +++ b/src/mesa/state_tracker/st_texture.c
> @@ -421,6 +421,83 @@ st_create_color_map_texture(struct gl_context
> *ctx)
>     return pt;
>  }
>  
> +/**
> + * Destroy bound texture handles for the given stage.
> + */
> +static void
> +st_destroy_bound_texture_handles_per_stage(struct st_context *st,
> +                                           enum pipe_shader_type
> shader)
> +{
> +   struct st_bound_handles *bound_handles = &st-
> >bound_texture_handles[shader];
> +   struct pipe_context *pipe = st->pipe;
> +   unsigned i;
> +
> +   if (likely(!bound_handles->num_handles))
> +      return;
> +
> +   for (i = 0; i < bound_handles->num_handles; i++) {
> +      uint64_t handle = bound_handles->handles[i];
> +
> +      pipe->make_texture_handle_resident(pipe, handle, false);
> +      pipe->delete_texture_handle(pipe, handle);
> +   }
> +   free(bound_handles->handles);
> +   bound_handles->num_handles = 0;
> +}

Perhaps set bound_handles->handles to NULL here, otherwise the address
of freed memory can be used by the realloc added in the following
patch.

> +
> +
> +/**
> + * Destroy all bound texture handles in the context.
> + */
> +void
> +st_destroy_bound_texture_handles(struct st_context *st)
> +{
> +   unsigned i;
> +
> +   for (i = 0; i < PIPE_SHADER_TYPES; i++) {
> +      st_destroy_bound_texture_handles_per_stage(st, i);
> +   }
> +}
> +
> +
> +/**
> + * Destroy bound image handles for the given stage.
> + */
> +static void
> +st_destroy_bound_image_handles_per_stage(struct st_context *st,
> +                                         enum pipe_shader_type
> shader)
> +{
> +   struct st_bound_handles *bound_handles = &st-
> >bound_image_handles[shader];
> +   struct pipe_context *pipe = st->pipe;
> +   unsigned i;
> +
> +   if (likely(!bound_handles->num_handles))
> +      return;
> +
> +   for (i = 0; i < bound_handles->num_handles; i++) {
> +      uint64_t handle = bound_handles->handles[i];
> +
> +      pipe->make_image_handle_resident(pipe, handle, GL_READ_WRITE,
> false);
> +      pipe->delete_image_handle(pipe, handle);
> +   }
> +   free(bound_handles->handles);
> +   bound_handles->num_handles = 0;

Same here.

Thanks,
James