[Mesa-dev] [PATCH] panfrost: Jobs must be per context, not per screen
Rohan Garg
rohan.garg at collabora.com
Thu Aug 29 12:51:52 UTC 2019
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.garg at 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.rosenzweig at 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
More information about the mesa-dev
mailing list