fdo-mirrors/mesa
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/mesa/mesa.git synced 2026-05-05 00:58:05 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

panfrost: Fix reference counting with batch->resources

Browse source 
Refactor accesses to batch->resources to happen through safe helpers
that update the appropriate bookkeeping. This makes it obvious that (in
particular) reference counts are updated when they should be.

The functional change is that we are now correctly unreferencing
resources during shadowing, fixing a leak of shadowed resources.

Closes: #7362
Fixes: 2d8f28df73 ("panfrost: Replace resource shadowing flush")
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reported-by: Mastodon, apparently
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19753>
This commit is contained in:
Alyssa Rosenzweig 2022-11-15 11:16:15 -05:00 • committed by Marge Bot
parent ba9bdd5642
commit 42212a9bfd
2 changed files with 74 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

116
src/gallium/drivers/panfrost/pan_job.c
View file

@ -99,6 +99,74 @@ panfrost_batch_init(struct panfrost_context *ctx,
screen->vtbl.init_batch(batch);
}
/*
* Safe helpers for manipulating batch->resources follow. In addition to
* wrapping the underlying set operations, these update the required
* bookkeeping for resource tracking and reference counting.
*/
static bool
panfrost_batch_uses_resource(struct panfrost_batch *batch,
struct panfrost_resource *rsrc)
{
return _mesa_set_search(batch->resources, rsrc) != NULL;
}
static void
panfrost_batch_add_resource(struct panfrost_batch *batch,
struct panfrost_resource *rsrc)
{
bool found = false;
_mesa_set_search_or_add(batch->resources, rsrc, &found);
if (!found) {
/* Cache number of batches accessing a resource */
rsrc->track.nr_users++;
/* Reference the resource on the batch */
pipe_reference(NULL, &rsrc->base.reference);
}
}
static void
panfrost_batch_remove_resource_internal(struct panfrost_context *ctx,
struct panfrost_resource *rsrc)
{
struct hash_entry *writer = _mesa_hash_table_search(ctx->writers, rsrc);
if (writer) {
_mesa_hash_table_remove(ctx->writers, writer);
rsrc->track.nr_writers--;
}
rsrc->track.nr_users--;
pipe_resource_reference((struct pipe_resource **) &rsrc, NULL);
}
static void
panfrost_batch_remove_resource_if_present(struct panfrost_context *ctx,
struct panfrost_batch *batch,
struct panfrost_resource *rsrc)
{
struct set_entry *ent = _mesa_set_search(batch->resources, rsrc);
if (ent != NULL) {
panfrost_batch_remove_resource_internal(ctx, rsrc);
_mesa_set_remove(batch->resources, ent);
}
}
static void
panfrost_batch_destroy_resources(struct panfrost_context *ctx,
struct panfrost_batch *batch)
{
set_foreach(batch->resources, entry) {
struct panfrost_resource *rsrc = (void *) entry->key;
panfrost_batch_remove_resource_internal(ctx, rsrc);
}
_mesa_set_destroy(batch->resources, NULL);
}
static void
panfrost_batch_cleanup(struct panfrost_context *ctx, struct panfrost_batch *batch)
{
@ -122,20 +190,7 @@ panfrost_batch_cleanup(struct panfrost_context *ctx, struct panfrost_batch *batc
panfrost_bo_unreference(bo);
}
set_foreach(batch->resources, entry) {
struct panfrost_resource *rsrc = (void *) entry->key;
if (_mesa_hash_table_search(ctx->writers, rsrc)) {
_mesa_hash_table_remove_key(ctx->writers, rsrc);
rsrc->track.nr_writers--;
}
rsrc->track.nr_users--;
pipe_resource_reference((struct pipe_resource **) &rsrc, NULL);
}
_mesa_set_destroy(batch->resources, NULL);
panfrost_batch_destroy_resources(ctx, batch);
panfrost_pool_cleanup(&batch->pool);
panfrost_pool_cleanup(&batch->invisible_pool);
@ -239,17 +294,8 @@ panfrost_batch_update_access(struct panfrost_batch *batch,
uint32_t batch_idx = panfrost_batch_idx(batch);
struct hash_entry *entry = _mesa_hash_table_search(ctx->writers, rsrc);
struct panfrost_batch *writer = entry ? entry->data : NULL;
bool found = false;
_mesa_set_search_or_add(batch->resources, rsrc, &found);
if (!found) {
/* Cache number of batches accessing a resource */
rsrc->track.nr_users++;
/* Reference the resource on the batch */
pipe_reference(NULL, &rsrc->base.reference);
}
panfrost_batch_add_resource(batch, rsrc);
/* Flush users if required */
if (writes || ((writer != NULL) && (writer != batch))) {
@ -262,7 +308,7 @@ panfrost_batch_update_access(struct panfrost_batch *batch,
continue;
/* Submit if it's a user */
if (_mesa_set_search(batch->resources, rsrc))
if (panfrost_batch_uses_resource(batch, rsrc))
panfrost_batch_submit(ctx, batch);
}
}
@ -363,30 +409,14 @@ panfrost_resource_swap_bo(struct panfrost_context *ctx,
struct panfrost_resource *rsrc,
struct panfrost_bo *newbo)
{
/* Any batch writing this resource is writing to the old BO, not the
* new BO. After swapping the resource's backing BO, there will be no
* writers of the updated resource. Existing writers still hold a
* reference to the old BO for reference counting.
*/
struct hash_entry *writer = _mesa_hash_table_search(ctx->writers, rsrc);
if (writer) {
_mesa_hash_table_remove(ctx->writers, writer);
rsrc->track.nr_writers--;
}
/* Likewise, any batch reading this resource is reading the old BO, and
* after swapping will not be reading this resource.
*/
unsigned i;
foreach_batch(ctx, i) {
struct panfrost_batch *batch = &ctx->batches.slots[i];
struct set_entry *ent = _mesa_set_search(batch->resources, rsrc);
if (!ent)
continue;
_mesa_set_remove(batch->resources, ent);
rsrc->track.nr_users--;
panfrost_batch_remove_resource_if_present(ctx, batch, rsrc);
}
/* Swap the pointers, dropping a reference to the old BO which is no
@ -891,7 +921,7 @@ panfrost_flush_batches_accessing_rsrc(struct panfrost_context *ctx,
foreach_batch(ctx, i) {
struct panfrost_batch *batch = &ctx->batches.slots[i];
if (!_mesa_set_search(batch->resources, rsrc))
if (!panfrost_batch_uses_resource(batch, rsrc))
continue;
perf_debug_ctx(ctx, "Flushing user due to: %s", reason);

2
src/gallium/drivers/panfrost/pan_job.h
View file

@ -192,7 +192,7 @@ struct panfrost_batch {
struct pan_tristate sprite_coord_origin;
struct pan_tristate first_provoking_vertex;
/* Referenced resources */
/* Referenced resources, holds a pipe_reference. */
struct set *resources;
};