Jobs _must_ only be shared across the same context, having the last_job tracked in a screen causes use-after-free issues and memory corruptions.
Signed-off-by: Rohan Garg <rohan.g...@collabora.com> --- src/gallium/drivers/panfrost/pan_allocate.c | 2 ++ src/gallium/drivers/panfrost/pan_bo_cache.c | 16 +++++++++++----- src/gallium/drivers/panfrost/pan_context.c | 10 +++++----- src/gallium/drivers/panfrost/pan_context.h | 6 ++++++ src/gallium/drivers/panfrost/pan_drm.c | 6 +++--- src/gallium/drivers/panfrost/pan_job.c | 2 ++ src/gallium/drivers/panfrost/pan_screen.c | 5 ++--- src/gallium/drivers/panfrost/pan_screen.h | 10 ++++------ 8 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_allocate.c b/src/gallium/drivers/panfrost/pan_allocate.c index f549c864c70..fb8b18fe718 100644 --- a/src/gallium/drivers/panfrost/pan_allocate.c +++ b/src/gallium/drivers/panfrost/pan_allocate.c @@ -74,6 +74,7 @@ panfrost_allocate_transient(struct panfrost_context *ctx, size_t sz) unsigned offset = 0; bool update_offset = false; + pthread_mutex_lock(&screen->transient_lock); bool has_current = batch->transient_indices.size; bool fits_in_current = (batch->transient_offset + sz) < TRANSIENT_SLAB_SIZE; @@ -131,6 +132,7 @@ panfrost_allocate_transient(struct panfrost_context *ctx, size_t sz) if (update_offset) batch->transient_offset = offset + sz; + pthread_mutex_unlock(&screen->transient_lock); return ret; diff --git a/src/gallium/drivers/panfrost/pan_bo_cache.c b/src/gallium/drivers/panfrost/pan_bo_cache.c index 9dd6b694b72..f2f49437a89 100644 --- a/src/gallium/drivers/panfrost/pan_bo_cache.c +++ b/src/gallium/drivers/panfrost/pan_bo_cache.c @@ -24,6 +24,7 @@ * Alyssa Rosenzweig <alyssa.rosenzw...@collabora.com> */ #include <xf86drm.h> +#include <pthread.h> #include "drm-uapi/panfrost_drm.h" #include "pan_screen.h" @@ -84,7 +85,9 @@ panfrost_bo_cache_fetch( struct panfrost_screen *screen, size_t size, uint32_t flags) { + pthread_mutex_lock(&screen->bo_cache_lock); struct list_head *bucket = pan_bucket(screen, size); + struct panfrost_bo *bo = NULL; /* Iterate the bucket looking for something suitable */ list_for_each_entry_safe(struct panfrost_bo, entry, bucket, link) { @@ -106,12 +109,13 @@ panfrost_bo_cache_fetch( continue; } /* Let's go! */ - return entry; + bo = entry; + break; } } + pthread_mutex_unlock(&screen->bo_cache_lock); - /* We didn't find anything */ - return NULL; + return bo; } /* Tries to add a BO to the cache. Returns if it was @@ -122,6 +126,7 @@ panfrost_bo_cache_put( struct panfrost_screen *screen, struct panfrost_bo *bo) { + pthread_mutex_lock(&screen->bo_cache_lock); struct list_head *bucket = pan_bucket(screen, bo->size); struct drm_panfrost_madvise madv; @@ -133,6 +138,7 @@ panfrost_bo_cache_put( /* Add us to the bucket */ list_addtail(&bo->link, bucket); + pthread_mutex_unlock(&screen->bo_cache_lock); return true; } @@ -147,6 +153,7 @@ void panfrost_bo_cache_evict_all( struct panfrost_screen *screen) { + pthread_mutex_lock(&screen->bo_cache_lock); for (unsigned i = 0; i < ARRAY_SIZE(screen->bo_cache); ++i) { struct list_head *bucket = &screen->bo_cache[i]; @@ -155,7 +162,6 @@ panfrost_bo_cache_evict_all( panfrost_drm_release_bo(screen, entry, false); } } - - return; + pthread_mutex_unlock(&screen->bo_cache_lock); } diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c index fa9c92af9f6..94ee9b5bdb2 100644 --- a/src/gallium/drivers/panfrost/pan_context.c +++ b/src/gallium/drivers/panfrost/pan_context.c @@ -1329,9 +1329,6 @@ panfrost_submit_frame(struct panfrost_context *ctx, bool flush_immediate, struct pipe_fence_handle **fence, struct panfrost_job *job) { - struct pipe_context *gallium = (struct pipe_context *) ctx; - struct panfrost_screen *screen = pan_screen(gallium->screen); - panfrost_job_submit(ctx, job); /* If visual, we can stall a frame */ @@ -1339,8 +1336,8 @@ panfrost_submit_frame(struct panfrost_context *ctx, bool flush_immediate, if (!flush_immediate) panfrost_drm_force_flush_fragment(ctx, fence); - screen->last_fragment_flushed = false; - screen->last_job = job; + ctx->last_fragment_flushed = false; + ctx->last_job = job; /* If readback, flush now (hurts the pipelined performance) */ if (flush_immediate) @@ -2856,6 +2853,9 @@ panfrost_create_context(struct pipe_screen *screen, void *priv, unsigned flags) assert(ctx->blitter); assert(ctx->blitter_wallpaper); + ctx->last_fragment_flushed = true; + ctx->last_job = NULL; + /* Prepare for render! */ panfrost_job_init(ctx); diff --git a/src/gallium/drivers/panfrost/pan_context.h b/src/gallium/drivers/panfrost/pan_context.h index 4c1580b3393..9f96e983a86 100644 --- a/src/gallium/drivers/panfrost/pan_context.h +++ b/src/gallium/drivers/panfrost/pan_context.h @@ -203,6 +203,12 @@ struct panfrost_context { bool is_t6xx; uint32_t out_sync; + + /* While we're busy building up the job for frame N, the GPU is + * still busy executing frame N-1. So hold a reference to + * yesterjob */ + int last_fragment_flushed; + struct panfrost_job *last_job; }; /* Corresponds to the CSO */ diff --git a/src/gallium/drivers/panfrost/pan_drm.c b/src/gallium/drivers/panfrost/pan_drm.c index c3693bff56a..f89bc1d26eb 100644 --- a/src/gallium/drivers/panfrost/pan_drm.c +++ b/src/gallium/drivers/panfrost/pan_drm.c @@ -349,12 +349,12 @@ panfrost_drm_force_flush_fragment(struct panfrost_context *ctx, struct pipe_context *gallium = (struct pipe_context *) ctx; struct panfrost_screen *screen = pan_screen(gallium->screen); - if (!screen->last_fragment_flushed) { + if (!ctx->last_fragment_flushed) { drmSyncobjWait(screen->fd, &ctx->out_sync, 1, INT64_MAX, 0, NULL); - screen->last_fragment_flushed = true; + ctx->last_fragment_flushed = true; /* The job finished up, so we're safe to clean it up now */ - panfrost_free_job(ctx, screen->last_job); + panfrost_free_job(ctx, ctx->last_job); } if (fence) { diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c index 4d8ec2eadc9..72afcdbf358 100644 --- a/src/gallium/drivers/panfrost/pan_job.c +++ b/src/gallium/drivers/panfrost/pan_job.c @@ -67,10 +67,12 @@ panfrost_free_job(struct panfrost_context *ctx, struct panfrost_job *job) /* Free up the transient BOs we're sitting on */ struct panfrost_screen *screen = pan_screen(ctx->base.screen); + pthread_mutex_lock(&screen->transiant_lock); util_dynarray_foreach(&job->transient_indices, unsigned, index) { /* Mark it free */ BITSET_SET(screen->free_transient, *index); } + pthread_mutex_unlock(&screen->transiant_lock); /* Unreference the polygon list */ panfrost_bo_unreference(ctx->base.screen, job->polygon_list); diff --git a/src/gallium/drivers/panfrost/pan_screen.c b/src/gallium/drivers/panfrost/pan_screen.c index 36c91a1572e..07f60ffee52 100644 --- a/src/gallium/drivers/panfrost/pan_screen.c +++ b/src/gallium/drivers/panfrost/pan_screen.c @@ -639,8 +639,10 @@ panfrost_create_screen(int fd, struct renderonly *ro) return NULL; } + pthread_mutex_init(&screen->transiant_lock, NULL); util_dynarray_init(&screen->transient_bo, screen); + pthread_mutex_init(&screen->bo_cache_lock, NULL); for (unsigned i = 0; i < ARRAY_SIZE(screen->bo_cache); ++i) list_inithead(&screen->bo_cache[i]); @@ -665,9 +667,6 @@ panfrost_create_screen(int fd, struct renderonly *ro) screen->base.fence_finish = panfrost_fence_finish; screen->base.set_damage_region = panfrost_resource_set_damage_region; - screen->last_fragment_flushed = true; - screen->last_job = NULL; - panfrost_resource_screen_init(screen); return &screen->base; diff --git a/src/gallium/drivers/panfrost/pan_screen.h b/src/gallium/drivers/panfrost/pan_screen.h index 35fb8de2628..f83cd7562ab 100644 --- a/src/gallium/drivers/panfrost/pan_screen.h +++ b/src/gallium/drivers/panfrost/pan_screen.h @@ -104,6 +104,8 @@ struct panfrost_screen { struct renderonly *ro; + pthread_mutex_t transient_lock; + /* Transient memory management is based on borrowing fixed-size slabs * off the screen (loaning them out to the batch). Dynamic array * container of panfrost_bo */ @@ -113,17 +115,13 @@ struct panfrost_screen { /* Set of free transient BOs */ BITSET_DECLARE(free_transient, MAX_TRANSIENT_SLABS); + pthread_mutex_t bo_cache_lock; + /* The BO cache is a set of buckets with power-of-two sizes ranging * from 2^12 (4096, the page size) to 2^(12 + MAX_BO_CACHE_BUCKETS). * Each bucket is a linked list of free panfrost_bo objects. */ struct list_head bo_cache[NR_BO_CACHE_BUCKETS]; - - /* While we're busy building up the job for frame N, the GPU is - * still busy executing frame N-1. So hold a reference to - * yesterjob */ - int last_fragment_flushed; - struct panfrost_job *last_job; }; static inline struct panfrost_screen * -- 2.17.1 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev