[Intel-gfx] [PATCH 05/23] Revert "drm/i915/gem: Split eb_vma into its own allocation"

2020-05-20 Thread Maarten Lankhorst
This reverts commit 0f1dd02295f35dcdcbaafcbcbbec0753884ab974.
This conflicts with the ww mutex handling, which needs to drop
the references after gpu submission anyway, because otherwise we
may risk unlocking a BO after first freeing it.

Signed-off-by: Maarten Lankhorst 
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 124 +++---
 1 file changed, 51 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index c795f3688eb4..3bcaf9af590a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -40,11 +40,6 @@ struct eb_vma {
u32 handle;
 };
 
-struct eb_vma_array {
-   struct kref kref;
-   struct eb_vma vma[];
-};
-
 enum {
FORCE_CPU_RELOC = 1,
FORCE_GTT_RELOC,
@@ -57,6 +52,7 @@ enum {
 #define __EXEC_OBJECT_NEEDS_MAPBIT(29)
 #define __EXEC_OBJECT_NEEDS_BIAS   BIT(28)
 #define __EXEC_OBJECT_INTERNAL_FLAGS   (~0u << 28) /* all of the above */
+#define __EXEC_OBJECT_RESERVED (__EXEC_OBJECT_HAS_PIN | 
__EXEC_OBJECT_HAS_FENCE)
 
 #define __EXEC_HAS_RELOC   BIT(31)
 #define __EXEC_INTERNAL_FLAGS  (~0u << 31)
@@ -289,7 +285,6 @@ struct i915_execbuffer {
 */
int lut_size;
struct hlist_head *buckets; /** ht for relocation handles */
-   struct eb_vma_array *array;
 };
 
 static int eb_parse(struct i915_execbuffer *eb);
@@ -301,62 +296,8 @@ static inline bool eb_use_cmdparser(const struct 
i915_execbuffer *eb)
 eb->args->batch_len);
 }
 
-static struct eb_vma_array *eb_vma_array_create(unsigned int count)
-{
-   struct eb_vma_array *arr;
-
-   arr = kvmalloc(struct_size(arr, vma, count), GFP_KERNEL | __GFP_NOWARN);
-   if (!arr)
-   return NULL;
-
-   kref_init(>kref);
-   arr->vma[0].vma = NULL;
-
-   return arr;
-}
-
-static inline void eb_unreserve_vma(struct eb_vma *ev)
-{
-   struct i915_vma *vma = ev->vma;
-
-   if (unlikely(ev->flags & __EXEC_OBJECT_HAS_FENCE))
-   __i915_vma_unpin_fence(vma);
-
-   if (ev->flags & __EXEC_OBJECT_HAS_PIN)
-   __i915_vma_unpin(vma);
-
-   ev->flags &= ~(__EXEC_OBJECT_HAS_PIN |
-  __EXEC_OBJECT_HAS_FENCE);
-}
-
-static void eb_vma_array_destroy(struct kref *kref)
-{
-   struct eb_vma_array *arr = container_of(kref, typeof(*arr), kref);
-   struct eb_vma *ev = arr->vma;
-
-   while (ev->vma) {
-   eb_unreserve_vma(ev);
-   i915_vma_put(ev->vma);
-   ev++;
-   }
-
-   kvfree(arr);
-}
-
-static void eb_vma_array_put(struct eb_vma_array *arr)
-{
-   kref_put(>kref, eb_vma_array_destroy);
-}
-
 static int eb_create(struct i915_execbuffer *eb)
 {
-   /* Allocate an extra slot for use by the command parser + sentinel */
-   eb->array = eb_vma_array_create(eb->buffer_count + 2);
-   if (!eb->array)
-   return -ENOMEM;
-
-   eb->vma = eb->array->vma;
-
if (!(eb->args->flags & I915_EXEC_HANDLE_LUT)) {
unsigned int size = 1 + ilog2(eb->buffer_count);
 
@@ -390,10 +331,8 @@ static int eb_create(struct i915_execbuffer *eb)
break;
} while (--size);
 
-   if (unlikely(!size)) {
-   eb_vma_array_put(eb->array);
+   if (unlikely(!size))
return -ENOMEM;
-   }
 
eb->lut_size = size;
} else {
@@ -504,6 +443,26 @@ eb_pin_vma(struct i915_execbuffer *eb,
return !eb_vma_misplaced(entry, vma, ev->flags);
 }
 
+static inline void __eb_unreserve_vma(struct i915_vma *vma, unsigned int flags)
+{
+   GEM_BUG_ON(!(flags & __EXEC_OBJECT_HAS_PIN));
+
+   if (unlikely(flags & __EXEC_OBJECT_HAS_FENCE))
+   __i915_vma_unpin_fence(vma);
+
+   __i915_vma_unpin(vma);
+}
+
+static inline void
+eb_unreserve_vma(struct eb_vma *ev)
+{
+   if (!(ev->flags & __EXEC_OBJECT_HAS_PIN))
+   return;
+
+   __eb_unreserve_vma(ev->vma, ev->flags);
+   ev->flags &= ~__EXEC_OBJECT_RESERVED;
+}
+
 static int
 eb_validate_vma(struct i915_execbuffer *eb,
struct drm_i915_gem_exec_object2 *entry,
@@ -946,13 +905,31 @@ eb_get_vma(const struct i915_execbuffer *eb, unsigned 
long handle)
}
 }
 
+static void eb_release_vmas(const struct i915_execbuffer *eb)
+{
+   const unsigned int count = eb->buffer_count;
+   unsigned int i;
+
+   for (i = 0; i < count; i++) {
+   struct eb_vma *ev = >vma[i];
+   struct i915_vma *vma = ev->vma;
+
+   if (!vma)
+   break;
+
+   eb->vma[i].vma = NULL;
+
+   if (ev->flags & __EXEC_OBJECT_HAS_PIN)
+   __eb_unreserve_vma(vma, ev->flags);
+
+   i915_vma_put(vma);
+   }
+}
+
 static void 

[Intel-gfx] [PATCH 05/23] Revert "drm/i915/gem: Split eb_vma into its own allocation"

2020-05-11 Thread Maarten Lankhorst
This reverts commit 0f1dd02295f35dcdcbaafcbcbbec0753884ab974.
This conflicts with the ww mutex handling, which needs to drop
the references after gpu submission anyway, because otherwise we
may risk unlocking a BO after first freeing it.

Signed-off-by: Maarten Lankhorst 
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 131 --
 1 file changed, 58 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index d928bde60d68..4b60405e0ec5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -40,11 +40,6 @@ struct eb_vma {
u32 handle;
 };
 
-struct eb_vma_array {
-   struct kref kref;
-   struct eb_vma vma[];
-};
-
 enum {
FORCE_CPU_RELOC = 1,
FORCE_GTT_RELOC,
@@ -57,6 +52,7 @@ enum {
 #define __EXEC_OBJECT_NEEDS_MAPBIT(29)
 #define __EXEC_OBJECT_NEEDS_BIAS   BIT(28)
 #define __EXEC_OBJECT_INTERNAL_FLAGS   (~0u << 28) /* all of the above */
+#define __EXEC_OBJECT_RESERVED (__EXEC_OBJECT_HAS_PIN | 
__EXEC_OBJECT_HAS_FENCE)
 
 #define __EXEC_HAS_RELOC   BIT(31)
 #define __EXEC_INTERNAL_FLAGS  (~0u << 31)
@@ -289,7 +285,6 @@ struct i915_execbuffer {
 */
int lut_size;
struct hlist_head *buckets; /** ht for relocation handles */
-   struct eb_vma_array *array;
 };
 
 static int eb_parse(struct i915_execbuffer *eb);
@@ -301,62 +296,8 @@ static inline bool eb_use_cmdparser(const struct 
i915_execbuffer *eb)
 eb->args->batch_len);
 }
 
-static struct eb_vma_array *eb_vma_array_create(unsigned int count)
-{
-   struct eb_vma_array *arr;
-
-   arr = kvmalloc(struct_size(arr, vma, count), GFP_KERNEL | __GFP_NOWARN);
-   if (!arr)
-   return NULL;
-
-   kref_init(>kref);
-   arr->vma[0].vma = NULL;
-
-   return arr;
-}
-
-static inline void eb_unreserve_vma(struct eb_vma *ev)
-{
-   struct i915_vma *vma = ev->vma;
-
-   if (unlikely(ev->flags & __EXEC_OBJECT_HAS_FENCE))
-   __i915_vma_unpin_fence(vma);
-
-   if (ev->flags & __EXEC_OBJECT_HAS_PIN)
-   __i915_vma_unpin(vma);
-
-   ev->flags &= ~(__EXEC_OBJECT_HAS_PIN |
-  __EXEC_OBJECT_HAS_FENCE);
-}
-
-static void eb_vma_array_destroy(struct kref *kref)
-{
-   struct eb_vma_array *arr = container_of(kref, typeof(*arr), kref);
-   struct eb_vma *ev = arr->vma;
-
-   while (ev->vma) {
-   eb_unreserve_vma(ev);
-   i915_vma_put(ev->vma);
-   ev++;
-   }
-
-   kvfree(arr);
-}
-
-static void eb_vma_array_put(struct eb_vma_array *arr)
-{
-   kref_put(>kref, eb_vma_array_destroy);
-}
-
 static int eb_create(struct i915_execbuffer *eb)
 {
-   /* Allocate an extra slot for use by the command parser + sentinel */
-   eb->array = eb_vma_array_create(eb->buffer_count + 2);
-   if (!eb->array)
-   return -ENOMEM;
-
-   eb->vma = eb->array->vma;
-
if (!(eb->args->flags & I915_EXEC_HANDLE_LUT)) {
unsigned int size = 1 + ilog2(eb->buffer_count);
 
@@ -390,10 +331,8 @@ static int eb_create(struct i915_execbuffer *eb)
break;
} while (--size);
 
-   if (unlikely(!size)) {
-   eb_vma_array_put(eb->array);
+   if (unlikely(!size))
return -ENOMEM;
-   }
 
eb->lut_size = size;
} else {
@@ -504,6 +443,26 @@ eb_pin_vma(struct i915_execbuffer *eb,
return !eb_vma_misplaced(entry, vma, ev->flags);
 }
 
+static inline void __eb_unreserve_vma(struct i915_vma *vma, unsigned int flags)
+{
+   GEM_BUG_ON(!(flags & __EXEC_OBJECT_HAS_PIN));
+
+   if (unlikely(flags & __EXEC_OBJECT_HAS_FENCE))
+   __i915_vma_unpin_fence(vma);
+
+   __i915_vma_unpin(vma);
+}
+
+static inline void
+eb_unreserve_vma(struct eb_vma *ev)
+{
+   if (!(ev->flags & __EXEC_OBJECT_HAS_PIN))
+   return;
+
+   __eb_unreserve_vma(ev->vma, ev->flags);
+   ev->flags &= ~__EXEC_OBJECT_RESERVED;
+}
+
 static int
 eb_validate_vma(struct i915_execbuffer *eb,
struct drm_i915_gem_exec_object2 *entry,
@@ -946,13 +905,31 @@ eb_get_vma(const struct i915_execbuffer *eb, unsigned 
long handle)
}
 }
 
+static void eb_release_vmas(const struct i915_execbuffer *eb)
+{
+   const unsigned int count = eb->buffer_count;
+   unsigned int i;
+
+   for (i = 0; i < count; i++) {
+   struct eb_vma *ev = >vma[i];
+   struct i915_vma *vma = ev->vma;
+
+   if (!vma)
+   break;
+
+   eb->vma[i].vma = NULL;
+
+   if (ev->flags & __EXEC_OBJECT_HAS_PIN)
+   __eb_unreserve_vma(vma, ev->flags);
+
+   i915_vma_put(vma);
+   }
+}
+
 static void