[PATCH i-g-t 05/32] lib/intel_bufops: clarify buffer ownership rules
Chris Wilson
chris at chris-wilson.co.uk
Wed Sep 16 13:40:33 UTC 2020
From: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
To avoid intel_buf handle leakage we need to clarify of buffer ownership
rules. So we currently have:
1. intel_buf_init(), intel_buf_create() which create handle and take
ownership of such handle.
2. intel_buf_init_using_handle(), intel_buf_create_using_handle() which
take bo handle from the caller and doesn't take ownership.
intel_buf_close()/intel_buf_destroy() will honour ownership and skip
closing handle if buffer is not owned.
To take/release buffer ownership intel_buf_set_ownership() can be used.
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
Cc: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
lib/intel_bufops.c | 75 +++++++++++++++++++++++++++++++++++++++++-----
lib/intel_bufops.h | 14 +++++++++
2 files changed, 82 insertions(+), 7 deletions(-)
diff --git a/lib/intel_bufops.c b/lib/intel_bufops.c
index 714e1aaac..57aa2a401 100644
--- a/lib/intel_bufops.c
+++ b/lib/intel_bufops.c
@@ -786,13 +786,13 @@ static void __intel_buf_init(struct buf_ops *bops,
* @buf: pointer to intel_buf structure to be filled
* @width: surface width
* @height: surface height
- * @bpp: bits-per-pixel (8 / 16 / 32)
+ * @bpp: bits-per-pixel (8 / 16 / 32 / 64)
* @alignment: alignment of the stride for linear surfaces
* @tiling: surface tiling
* @compression: surface compression type
*
* Function creates new BO within intel_buf structure and fills all
- * structure fields.
+ * structure fields. Takes bo handle ownership.
*
* Note. For X / Y if GPU supports fences HW tiling is configured.
*/
@@ -803,6 +803,8 @@ void intel_buf_init(struct buf_ops *bops,
{
__intel_buf_init(bops, 0, buf, width, height, bpp, alignment,
tiling, compression);
+
+ intel_buf_set_ownership(buf, true);
}
/**
@@ -810,14 +812,17 @@ void intel_buf_init(struct buf_ops *bops,
* @bops: pointer to buf_ops
* @buf: pointer to intel_buf structure
*
- * Function closes gem BO inside intel_buf.
+ * Function closes gem BO inside intel_buf if bo is owned by intel_buf.
+ * For handle passed from the caller intel_buf doesn't take ownership and
+ * doesn't close it in close()/destroy() paths.
*/
void intel_buf_close(struct buf_ops *bops, struct intel_buf *buf)
{
igt_assert(bops);
igt_assert(buf);
- gem_close(bops->fd, buf->handle);
+ if (buf->is_owner)
+ gem_close(bops->fd, buf->handle);
}
/**
@@ -827,7 +832,7 @@ void intel_buf_close(struct buf_ops *bops, struct intel_buf *buf)
* @buf: pointer to intel_buf structure to be filled
* @width: surface width
* @height: surface height
- * @bpp: bits-per-pixel (8 / 16 / 32)
+ * @bpp: bits-per-pixel (8 / 16 / 32 / 64)
* @alignment: alignment of the stride for linear surfaces
* @tiling: surface tiling
* @compression: surface compression type
@@ -836,8 +841,8 @@ void intel_buf_close(struct buf_ops *bops, struct intel_buf *buf)
* (with all its metadata - width, height, ...). Useful if BO was created
* outside.
*
- * Note: intel_buf_close() can be used to close the BO handle, but caller
- * must be aware to not close the BO twice.
+ * Note: intel_buf_close() can be used because intel_buf is aware it is not
+ * buffer owner so it won't close it underneath.
*/
void intel_buf_init_using_handle(struct buf_ops *bops,
uint32_t handle,
@@ -849,6 +854,19 @@ void intel_buf_init_using_handle(struct buf_ops *bops,
req_tiling, compression);
}
+/**
+ * intel_buf_create
+ * @bops: pointer to buf_ops
+ * @width: surface width
+ * @height: surface height
+ * @bpp: bits-per-pixel (8 / 16 / 32 / 64)
+ * @alignment: alignment of the stride for linear surfaces
+ * @tiling: surface tiling
+ * @compression: surface compression type
+ *
+ * Function creates intel_buf with created BO handle. Takes ownership of the
+ * buffer.
+ */
struct intel_buf *intel_buf_create(struct buf_ops *bops,
int width, int height,
int bpp, int alignment,
@@ -867,12 +885,55 @@ struct intel_buf *intel_buf_create(struct buf_ops *bops,
return buf;
}
+/**
+ * intel_buf_create_using_handle
+ * @bops: pointer to buf_ops
+ * @handle: BO handle created by the caller
+ * @width: surface width
+ * @height: surface height
+ * @bpp: bits-per-pixel (8 / 16 / 32 / 64)
+ * @alignment: alignment of the stride for linear surfaces
+ * @tiling: surface tiling
+ * @compression: surface compression type
+ *
+ * Function creates intel_buf with passed BO handle from the caller. Doesn't
+ * take ownership of the buffer. close()/destroy() paths doesn't close
+ * passed handle unless buffer will take ownership using set_ownership().
+ */
+struct intel_buf *intel_buf_create_using_handle(struct buf_ops *bops,
+ uint32_t handle,
+ int width, int height,
+ int bpp, int alignment,
+ uint32_t req_tiling,
+ uint32_t compression)
+{
+ struct intel_buf *buf;
+
+ igt_assert(bops);
+
+ buf = calloc(1, sizeof(*buf));
+ igt_assert(buf);
+
+ intel_buf_init_using_handle(bops, handle, buf, width, height, bpp,
+ alignment, req_tiling, compression);
+
+ return buf;
+}
+
+/**
+ * intel_buf_destroy
+ * @buf: intel_buf
+ *
+ * Function frees intel_buf memory. It closes bo handle if intel_buf has
+ * buffer ownership.
+ */
void intel_buf_destroy(struct intel_buf *buf)
{
igt_assert(buf);
igt_assert(buf->ptr == NULL);
intel_buf_close(buf->bops, buf);
+
free(buf);
}
diff --git a/lib/intel_bufops.h b/lib/intel_bufops.h
index 0a8536ba2..8debe7f22 100644
--- a/lib/intel_bufops.h
+++ b/lib/intel_bufops.h
@@ -11,6 +11,7 @@ struct buf_ops;
#define INTEL_BUF_NAME_MAXSIZE 32
struct intel_buf {
struct buf_ops *bops;
+ bool is_owner;
uint32_t handle;
uint32_t tiling;
uint32_t bpp;
@@ -107,6 +108,11 @@ void linear_to_intel_buf(struct buf_ops *bops, struct intel_buf *buf,
bool buf_ops_has_hw_fence(struct buf_ops *bops, uint32_t tiling);
bool buf_ops_has_tiling_support(struct buf_ops *bops, uint32_t tiling);
+static inline void intel_buf_set_ownership(struct intel_buf *buf, bool is_owner)
+{
+ buf->is_owner = is_owner;
+}
+
void intel_buf_init(struct buf_ops *bops, struct intel_buf *buf,
int width, int height, int bpp, int alignment,
uint32_t tiling, uint32_t compression);
@@ -122,6 +128,14 @@ struct intel_buf *intel_buf_create(struct buf_ops *bops,
int width, int height,
int bpp, int alignment,
uint32_t req_tiling, uint32_t compression);
+
+struct intel_buf *intel_buf_create_using_handle(struct buf_ops *bops,
+ uint32_t handle,
+ int width, int height,
+ int bpp, int alignment,
+ uint32_t req_tiling,
+ uint32_t compression);
+
void intel_buf_destroy(struct intel_buf *buf);
void *intel_buf_cpu_map(struct intel_buf *buf, bool write);
--
2.28.0
More information about the Intel-gfx-trybot
mailing list