[PATCH 1/5] dma-buf: cleanup dma_fence_unwrap selftest v2

[PATCH 1/5] dma-buf: cleanup dma_fence_unwrap selftest v2

[Christian König]

The selftests, fix the error handling, remove unused functions and stop
leaking memory in failed tests.

v2: fix the memory leak correctly.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/st-dma-fence-unwrap.c | 48 +++++++++++----------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c
index 039f016b57be..e20c5a7dcfe4 100644
--- a/drivers/dma-buf/st-dma-fence-unwrap.c
+++ b/drivers/dma-buf/st-dma-fence-unwrap.c
@@ -4,27 +4,19 @@
  * Copyright (C) 2022 Advanced Micro Devices, Inc.
  */
 
+#include <linux/dma-fence.h>
+#include <linux/dma-fence-array.h>
+#include <linux/dma-fence-chain.h>
 #include <linux/dma-fence-unwrap.h>
-#if 0
-#include <linux/kernel.h>
-#include <linux/kthread.h>
-#include <linux/mm.h>
-#include <linux/sched/signal.h>
-#include <linux/slab.h>
-#include <linux/spinlock.h>
-#include <linux/random.h>
-#endif
 
 #include "selftest.h"
 
 #define CHAIN_SZ (4 << 10)
 
-static inline struct mock_fence {
+struct mock_fence {
 	struct dma_fence base;
 	spinlock_t lock;
-} *to_mock_fence(struct dma_fence *f) {
-	return container_of(f, struct mock_fence, base);
-}
+};
 
 static const char *mock_name(struct dma_fence *f)
 {
@@ -45,7 +37,8 @@ static struct dma_fence *mock_fence(void)
 		return NULL;
 
 	spin_lock_init(&f->lock);
-	dma_fence_init(&f->base, &mock_ops, &f->lock, 0, 0);
+	dma_fence_init(&f->base, &mock_ops, &f->lock,
+		       dma_fence_context_alloc(1), 1);
 
 	return &f->base;
 }
@@ -59,7 +52,7 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...)
 
 	fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
 	if (!fences)
-		return NULL;
+		goto error_put;
 
 	va_start(valist, num_fences);
 	for (i = 0; i < num_fences; ++i)
@@ -70,13 +63,17 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...)
 				       dma_fence_context_alloc(1),
 				       1, false);
 	if (!array)
-		goto cleanup;
+		goto error_free;
 	return &array->base;
 
-cleanup:
-	for (i = 0; i < num_fences; ++i)
-		dma_fence_put(fences[i]);
+error_free:
 	kfree(fences);
+
+error_put:
+	va_start(valist, num_fences);
+	for (i = 0; i < num_fences; ++i)
+		dma_fence_put(va_arg(valist, typeof(*fences)));
+	va_end(valist);
 	return NULL;
 }
 
@@ -113,7 +110,6 @@ static int sanitycheck(void *arg)
 	if (!chain)
 		return -ENOMEM;
 
-	dma_fence_signal(f);
 	dma_fence_put(chain);
 	return err;
 }
@@ -154,10 +150,8 @@ static int unwrap_array(void *arg)
 		err = -EINVAL;
 	}
 
-	dma_fence_signal(f1);
-	dma_fence_signal(f2);
 	dma_fence_put(array);
-	return 0;
+	return err;
 }
 
 static int unwrap_chain(void *arg)
@@ -196,10 +190,8 @@ static int unwrap_chain(void *arg)
 		err = -EINVAL;
 	}
 
-	dma_fence_signal(f1);
-	dma_fence_signal(f2);
 	dma_fence_put(chain);
-	return 0;
+	return err;
 }
 
 static int unwrap_chain_array(void *arg)
@@ -242,10 +234,8 @@ static int unwrap_chain_array(void *arg)
 		err = -EINVAL;
 	}
 
-	dma_fence_signal(f1);
-	dma_fence_signal(f2);
 	dma_fence_put(chain);
-	return 0;
+	return err;
 }
 
 int dma_fence_unwrap(void)
-- 
2.25.1

[PATCH 2/5] dma-buf: cleanup dma_fence_unwrap implementation

[Christian König]

Move the code from the inline functions into exported functions.

Signed-off-by: Christian König <christian.koenig@amd.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/Makefile           |  2 +-
 drivers/dma-buf/dma-fence-unwrap.c | 59 ++++++++++++++++++++++++++++++
 include/linux/dma-fence-unwrap.h   | 52 ++------------------------
 3 files changed, 64 insertions(+), 49 deletions(-)
 create mode 100644 drivers/dma-buf/dma-fence-unwrap.c

diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index 4c9eb53ba3f8..70ec901edf2c 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
-	 dma-resv.o
+	 dma-fence-unwrap.o dma-resv.o
 obj-$(CONFIG_DMABUF_HEAPS)	+= dma-heap.o
 obj-$(CONFIG_DMABUF_HEAPS)	+= heaps/
 obj-$(CONFIG_SYNC_FILE)		+= sync_file.o
diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c
new file mode 100644
index 000000000000..711be125428c
--- /dev/null
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * dma-fence-util: misc functions for dma_fence objects
+ *
+ * Copyright (C) 2022 Advanced Micro Devices, Inc.
+ * Authors:
+ *	Christian König <christian.koenig@amd.com>
+ */
+
+#include <linux/dma-fence.h>
+#include <linux/dma-fence-array.h>
+#include <linux/dma-fence-chain.h>
+#include <linux/dma-fence-unwrap.h>
+
+/* Internal helper to start new array iteration, don't use directly */
+static struct dma_fence *
+__dma_fence_unwrap_array(struct dma_fence_unwrap *cursor)
+{
+	cursor->array = dma_fence_chain_contained(cursor->chain);
+	cursor->index = 0;
+	return dma_fence_array_first(cursor->array);
+}
+
+/**
+ * dma_fence_unwrap_first - return the first fence from fence containers
+ * @head: the entrypoint into the containers
+ * @cursor: current position inside the containers
+ *
+ * Unwraps potential dma_fence_chain/dma_fence_array containers and return the
+ * first fence.
+ */
+struct dma_fence *dma_fence_unwrap_first(struct dma_fence *head,
+					 struct dma_fence_unwrap *cursor)
+{
+	cursor->chain = dma_fence_get(head);
+	return __dma_fence_unwrap_array(cursor);
+}
+EXPORT_SYMBOL_GPL(dma_fence_unwrap_first);
+
+/**
+ * dma_fence_unwrap_next - return the next fence from a fence containers
+ * @cursor: current position inside the containers
+ *
+ * Continue unwrapping the dma_fence_chain/dma_fence_array containers and return
+ * the next fence from them.
+ */
+struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor)
+{
+	struct dma_fence *tmp;
+
+	++cursor->index;
+	tmp = dma_fence_array_next(cursor->array, cursor->index);
+	if (tmp)
+		return tmp;
+
+	cursor->chain = dma_fence_chain_walk(cursor->chain);
+	return __dma_fence_unwrap_array(cursor);
+}
+EXPORT_SYMBOL_GPL(dma_fence_unwrap_next);
diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
index 77e335a1bcac..e7c219da4ed7 100644
--- a/include/linux/dma-fence-unwrap.h
+++ b/include/linux/dma-fence-unwrap.h
@@ -1,7 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * fence-chain: chain fences together in a timeline
- *
  * Copyright (C) 2022 Advanced Micro Devices, Inc.
  * Authors:
  *	Christian König <christian.koenig@amd.com>
@@ -10,8 +8,7 @@
 #ifndef __LINUX_DMA_FENCE_UNWRAP_H
 #define __LINUX_DMA_FENCE_UNWRAP_H
 
-#include <linux/dma-fence-chain.h>
-#include <linux/dma-fence-array.h>
+struct dma_fence;
 
 /**
  * struct dma_fence_unwrap - cursor into the container structure
@@ -33,50 +30,9 @@ struct dma_fence_unwrap {
 	unsigned int index;
 };
 
-/* Internal helper to start new array iteration, don't use directly */
-static inline struct dma_fence *
-__dma_fence_unwrap_array(struct dma_fence_unwrap * cursor)
-{
-	cursor->array = dma_fence_chain_contained(cursor->chain);
-	cursor->index = 0;
-	return dma_fence_array_first(cursor->array);
-}
-
-/**
- * dma_fence_unwrap_first - return the first fence from fence containers
- * @head: the entrypoint into the containers
- * @cursor: current position inside the containers
- *
- * Unwraps potential dma_fence_chain/dma_fence_array containers and return the
- * first fence.
- */
-static inline struct dma_fence *
-dma_fence_unwrap_first(struct dma_fence *head, struct dma_fence_unwrap *cursor)
-{
-	cursor->chain = dma_fence_get(head);
-	return __dma_fence_unwrap_array(cursor);
-}
-
-/**
- * dma_fence_unwrap_next - return the next fence from a fence containers
- * @cursor: current position inside the containers
- *
- * Continue unwrapping the dma_fence_chain/dma_fence_array containers and return
- * the next fence from them.
- */
-static inline struct dma_fence *
-dma_fence_unwrap_next(struct dma_fence_unwrap *cursor)
-{
-	struct dma_fence *tmp;
-
-	++cursor->index;
-	tmp = dma_fence_array_next(cursor->array, cursor->index);
-	if (tmp)
-		return tmp;
-
-	cursor->chain = dma_fence_chain_walk(cursor->chain);
-	return __dma_fence_unwrap_array(cursor);
-}
+struct dma_fence *dma_fence_unwrap_first(struct dma_fence *head,
+					 struct dma_fence_unwrap *cursor);
+struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor);
 
 /**
  * dma_fence_unwrap_for_each - iterate over all fences in containers
-- 
2.25.1

[PATCH 3/5] dma-buf: return only unsignaled fences in dma_fence_unwrap_for_each v3

[Christian König]

dma_fence_chain containers cleanup signaled fences automatically, so
filter those out from arrays as well.

v2: fix missing walk over the array
v3: massively simplify the patch and actually update the description.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 include/linux/dma-fence-unwrap.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
index e7c219da4ed7..a4d342fef8e0 100644
--- a/include/linux/dma-fence-unwrap.h
+++ b/include/linux/dma-fence-unwrap.h
@@ -43,9 +43,13 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor);
  * Unwrap dma_fence_chain and dma_fence_array containers and deep dive into all
  * potential fences in them. If @head is just a normal fence only that one is
  * returned.
+ *
+ * Note that signalled fences are opportunistically filtered out, which
+ * means the iteration is potentially over no fence at all.
  */
 #define dma_fence_unwrap_for_each(fence, cursor, head)			\
 	for (fence = dma_fence_unwrap_first(head, cursor); fence;	\
-	     fence = dma_fence_unwrap_next(cursor))
+	     fence = dma_fence_unwrap_next(cursor))			\
+		if (!dma_fence_is_signaled(fence))
 
 #endif
-- 
2.25.1

[PATCH 4/5] dma-buf: generalize dma_fence unwrap & merging v2

[Christian König]

Introduce a dma_fence_unwrap_merge() macro which allows to unwrap fences
which potentially can be containers as well and then merge them back
together into a flat dma_fence_array.

v2: rename the function, add some more comments about how the wrapper is
    used, move filtering of signaled fences into the unwrap iterator,
    add complex selftest which covers more cases.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/dma-fence-unwrap.c    |  99 +++++++++++++++++++++
 drivers/dma-buf/st-dma-fence-unwrap.c | 109 +++++++++++++++++++++++
 drivers/dma-buf/sync_file.c           | 119 ++------------------------
 include/linux/dma-fence-unwrap.h      |  24 ++++++
 4 files changed, 238 insertions(+), 113 deletions(-)

diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c
index 711be125428c..0e51547bbabd 100644
--- a/drivers/dma-buf/dma-fence-unwrap.c
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -11,6 +11,7 @@
 #include <linux/dma-fence-array.h>
 #include <linux/dma-fence-chain.h>
 #include <linux/dma-fence-unwrap.h>
+#include <linux/slab.h>
 
 /* Internal helper to start new array iteration, don't use directly */
 static struct dma_fence *
@@ -57,3 +58,101 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor)
 	return __dma_fence_unwrap_array(cursor);
 }
 EXPORT_SYMBOL_GPL(dma_fence_unwrap_next);
+
+/* Implementation for the dma_fence_merge() marco, don't use directly */
+struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
+					   struct dma_fence **fences,
+					   struct dma_fence_unwrap *iter)
+{
+	struct dma_fence_array *result;
+	struct dma_fence *tmp, **array;
+	unsigned int i;
+	size_t count;
+
+	count = 0;
+	for (i = 0; i < num_fences; ++i) {
+		dma_fence_unwrap_for_each(tmp, &iter[i], fences[i])
+			++count;
+	}
+
+	if (count == 0)
+		return dma_fence_get_stub();
+
+	array = kmalloc_array(count, sizeof(*array), GFP_KERNEL);
+	if (!array)
+		return NULL;
+
+	/*
+	 * This trashes the input fence array and uses it as position for the
+	 * following merge loop. This works because the dma_fence_merge()
+	 * wrapper macro is creating this temporary array on the stack together
+	 * with the iterators.
+	 */
+	for (i = 0; i < num_fences; ++i)
+		fences[i] = dma_fence_unwrap_first(fences[i], &iter[i]);
+
+	count = 0;
+	do {
+		unsigned int sel;
+
+restart:
+		tmp = NULL;
+		for (i = 0; i < num_fences; ++i) {
+			struct dma_fence *next = fences[i];
+
+			if (!next)
+				continue;
+
+			/*
+			 * We can't guarantee that inpute fences are ordered by
+			 * context, but it is still quite likely when this
+			 * function is used multiple times. So attempt to order
+			 * the fences by context as we pass over them and merge
+			 * fences with the same context.
+			 */
+			if (!tmp || tmp->context > next->context) {
+				tmp = next;
+				sel = i;
+
+			} else if (tmp->context < next->context) {
+				continue;
+
+			} else if (dma_fence_is_later(tmp, next)) {
+				fences[i] = dma_fence_unwrap_next(&iter[i]);
+				goto restart;
+			} else {
+				fences[sel] = dma_fence_unwrap_next(&iter[sel]);
+				goto restart;
+			}
+		}
+
+		if (tmp) {
+			array[count++] = dma_fence_get(tmp);
+			fences[sel] = dma_fence_unwrap_next(&iter[sel]);
+		}
+	} while (tmp);
+
+	if (count == 0) {
+		tmp = dma_fence_get_stub();
+		goto return_tmp;
+	}
+
+	if (count == 1) {
+		tmp = array[0];
+		goto return_tmp;
+	}
+
+	result = dma_fence_array_create(count, array,
+					dma_fence_context_alloc(1),
+					1, false);
+	if (!result) {
+		tmp = NULL;
+		goto return_tmp;
+	}
+	return &result->base;
+
+return_tmp:
+	kfree(array);
+	return tmp;
+}
+EXPORT_SYMBOL_GPL(__dma_fence_unwrap_merge);
diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c
index e20c5a7dcfe4..4105d5ea8dde 100644
--- a/drivers/dma-buf/st-dma-fence-unwrap.c
+++ b/drivers/dma-buf/st-dma-fence-unwrap.c
@@ -238,6 +238,113 @@ static int unwrap_chain_array(void *arg)
 	return err;
 }
 
+static int unwrap_merge(void *arg)
+{
+	struct dma_fence *fence, *f1, *f2, *f3;
+	struct dma_fence_unwrap iter;
+	int err = 0;
+
+	f1 = mock_fence();
+	if (!f1)
+		return -ENOMEM;
+
+	f2 = mock_fence();
+	if (!f2) {
+		err = -ENOMEM;
+		goto error_put_f1;
+	}
+
+	f3 = dma_fence_unwrap_merge(f1, f2);
+	if (!f3) {
+		err = -ENOMEM;
+		goto error_put_f2;
+	}
+
+	dma_fence_unwrap_for_each(fence, &iter, f3) {
+		if (fence == f1) {
+			dma_fence_put(f1);
+			f1 = NULL;
+		} else if (fence == f2) {
+			dma_fence_put(f2);
+			f2 = NULL;
+		} else {
+			pr_err("Unexpected fence!\n");
+			err = -EINVAL;
+		}
+	}
+
+	if (f1 || f2) {
+		pr_err("Not all fences seen!\n");
+		err = -EINVAL;
+	}
+
+	dma_fence_put(f3);
+error_put_f2:
+	dma_fence_put(f2);
+error_put_f1:
+	dma_fence_put(f1);
+	return err;
+}
+
+static int unwrap_merge_complex(void *arg)
+{
+	struct dma_fence *fence, *f1, *f2, *f3, *f4, *f5;
+	struct dma_fence_unwrap iter;
+	int err = -ENOMEM;
+
+	f1 = mock_fence();
+	if (!f1)
+		return -ENOMEM;
+
+	f2 = mock_fence();
+	if (!f2)
+		goto error_put_f1;
+
+	f3 = dma_fence_unwrap_merge(f1, f2);
+	if (!f3)
+		goto error_put_f2;
+
+	/* The resulting array has the fences in reverse */
+	f4 = dma_fence_unwrap_merge(f2, f1);
+	if (!f4)
+		goto error_put_f3;
+
+	/* Signaled fences should be filtered, the two arrays merged. */
+	f5 = dma_fence_unwrap_merge(f3, f4, dma_fence_get_stub());
+	if (!f5)
+		goto error_put_f4;
+
+	err = 0;
+	dma_fence_unwrap_for_each(fence, &iter, f5) {
+		if (fence == f1) {
+			dma_fence_put(f1);
+			f1 = NULL;
+		} else if (fence == f2) {
+			dma_fence_put(f2);
+			f2 = NULL;
+		} else {
+			pr_err("Unexpected fence!\n");
+			err = -EINVAL;
+		}
+	}
+
+	if (f1 || f2) {
+		pr_err("Not all fences seen!\n");
+		err = -EINVAL;
+	}
+
+	dma_fence_put(f5);
+error_put_f4:
+	dma_fence_put(f4);
+error_put_f3:
+	dma_fence_put(f3);
+error_put_f2:
+	dma_fence_put(f2);
+error_put_f1:
+	dma_fence_put(f1);
+	return err;
+}
+
 int dma_fence_unwrap(void)
 {
 	static const struct subtest tests[] = {
@@ -245,6 +352,8 @@ int dma_fence_unwrap(void)
 		SUBTEST(unwrap_array),
 		SUBTEST(unwrap_chain),
 		SUBTEST(unwrap_chain_array),
+		SUBTEST(unwrap_merge),
+		SUBTEST(unwrap_merge_complex),
 	};
 
 	return subtests(tests, NULL);
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 0fe564539166..3ebec19a8e02 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -146,50 +146,6 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len)
 	return buf;
 }
 
-static int sync_file_set_fence(struct sync_file *sync_file,
-			       struct dma_fence **fences, int num_fences)
-{
-	struct dma_fence_array *array;
-
-	/*
-	 * The reference for the fences in the new sync_file and held
-	 * in add_fence() during the merge procedure, so for num_fences == 1
-	 * we already own a new reference to the fence. For num_fence > 1
-	 * we own the reference of the dma_fence_array creation.
-	 */
-
-	if (num_fences == 0) {
-		sync_file->fence = dma_fence_get_stub();
-		kfree(fences);
-
-	} else if (num_fences == 1) {
-		sync_file->fence = fences[0];
-		kfree(fences);
-
-	} else {
-		array = dma_fence_array_create(num_fences, fences,
-					       dma_fence_context_alloc(1),
-					       1, false);
-		if (!array)
-			return -ENOMEM;
-
-		sync_file->fence = &array->base;
-	}
-
-	return 0;
-}
-
-static void add_fence(struct dma_fence **fences,
-		      int *i, struct dma_fence *fence)
-{
-	fences[*i] = fence;
-
-	if (!dma_fence_is_signaled(fence)) {
-		dma_fence_get(fence);
-		(*i)++;
-	}
-}
-
 /**
  * sync_file_merge() - merge two sync_files
  * @name:	name of new fence
@@ -203,84 +159,21 @@ static void add_fence(struct dma_fence **fences,
 static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
 					 struct sync_file *b)
 {
-	struct dma_fence *a_fence, *b_fence, **fences;
-	struct dma_fence_unwrap a_iter, b_iter;
-	unsigned int index, num_fences;
 	struct sync_file *sync_file;
+	struct dma_fence *fence;
 
 	sync_file = sync_file_alloc();
 	if (!sync_file)
 		return NULL;
 
-	num_fences = 0;
-	dma_fence_unwrap_for_each(a_fence, &a_iter, a->fence)
-		++num_fences;
-	dma_fence_unwrap_for_each(b_fence, &b_iter, b->fence)
-		++num_fences;
-
-	if (num_fences > INT_MAX)
-		goto err_free_sync_file;
-
-	fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
-	if (!fences)
-		goto err_free_sync_file;
-
-	/*
-	 * We can't guarantee that fences in both a and b are ordered, but it is
-	 * still quite likely.
-	 *
-	 * So attempt to order the fences as we pass over them and merge fences
-	 * with the same context.
-	 */
-
-	index = 0;
-	for (a_fence = dma_fence_unwrap_first(a->fence, &a_iter),
-	     b_fence = dma_fence_unwrap_first(b->fence, &b_iter);
-	     a_fence || b_fence; ) {
-
-		if (!b_fence) {
-			add_fence(fences, &index, a_fence);
-			a_fence = dma_fence_unwrap_next(&a_iter);
-
-		} else if (!a_fence) {
-			add_fence(fences, &index, b_fence);
-			b_fence = dma_fence_unwrap_next(&b_iter);
-
-		} else if (a_fence->context < b_fence->context) {
-			add_fence(fences, &index, a_fence);
-			a_fence = dma_fence_unwrap_next(&a_iter);
-
-		} else if (b_fence->context < a_fence->context) {
-			add_fence(fences, &index, b_fence);
-			b_fence = dma_fence_unwrap_next(&b_iter);
-
-		} else if (__dma_fence_is_later(a_fence->seqno, b_fence->seqno,
-						a_fence->ops)) {
-			add_fence(fences, &index, a_fence);
-			a_fence = dma_fence_unwrap_next(&a_iter);
-			b_fence = dma_fence_unwrap_next(&b_iter);
-
-		} else {
-			add_fence(fences, &index, b_fence);
-			a_fence = dma_fence_unwrap_next(&a_iter);
-			b_fence = dma_fence_unwrap_next(&b_iter);
-		}
+	fence = dma_fence_unwrap_merge(a->fence, b->fence);
+	if (!fence) {
+		fput(sync_file->file);
+		return NULL;
 	}
-
-	if (sync_file_set_fence(sync_file, fences, index) < 0)
-		goto err_put_fences;
-
+	sync_file->fence = fence;
 	strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name));
 	return sync_file;
-
-err_put_fences:
-	while (index)
-		dma_fence_put(fences[--index]);
-	kfree(fences);
-
-err_free_sync_file:
-	fput(sync_file->file);
-	return NULL;
 }
 
 static int sync_file_release(struct inode *inode, struct file *file)
diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
index a4d342fef8e0..390de1ee9d35 100644
--- a/include/linux/dma-fence-unwrap.h
+++ b/include/linux/dma-fence-unwrap.h
@@ -52,4 +52,28 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor);
 	     fence = dma_fence_unwrap_next(cursor))			\
 		if (!dma_fence_is_signaled(fence))
 
+struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
+					   struct dma_fence **fences,
+					   struct dma_fence_unwrap *cursors);
+
+/**
+ * dma_fence_unwrap_merge - unwrap and merge fences
+ *
+ * All fences given as parameters are unwrapped and merged back together as flat
+ * dma_fence_array. Useful if multiple containers need to be merged together.
+ *
+ * Implemented as a macro to allocate the necessary arrays on the stack and
+ * account the stack frame size to the caller.
+ *
+ * Returns NULL on memory allocation failure, a dma_fence object representing
+ * all the given fences otherwise.
+ */
+#define dma_fence_unwrap_merge(...)					\
+	({								\
+		struct dma_fence *__f[] = { __VA_ARGS__ };		\
+		struct dma_fence_unwrap __c[ARRAY_SIZE(__f)];		\
+									\
+		__dma_fence_unwrap_merge(ARRAY_SIZE(__f), __f, __c);	\
+	})
+
 #endif
-- 
2.25.1

[PATCH 5/5] drm: use dma_fence_unwrap_merge() in drm_syncobj

[Christian König]

The unwrap merge function is now intended for this use case.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_syncobj.c | 57 +++++------------------------------
 1 file changed, 7 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 7e48dcd1bee4..bbad9e4696e7 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -184,6 +184,7 @@
  */
 
 #include <linux/anon_inodes.h>
+#include <linux/dma-fence-unwrap.h>
 #include <linux/file.h>
 #include <linux/fs.h>
 #include <linux/sched/signal.h>
@@ -853,57 +854,12 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
 					&args->handle);
 }
 
-
-/*
- * Try to flatten a dma_fence_chain into a dma_fence_array so that it can be
- * added as timeline fence to a chain again.
- */
-static int drm_syncobj_flatten_chain(struct dma_fence **f)
-{
-	struct dma_fence_chain *chain = to_dma_fence_chain(*f);
-	struct dma_fence *tmp, **fences;
-	struct dma_fence_array *array;
-	unsigned int count;
-
-	if (!chain)
-		return 0;
-
-	count = 0;
-	dma_fence_chain_for_each(tmp, &chain->base)
-		++count;
-
-	fences = kmalloc_array(count, sizeof(*fences), GFP_KERNEL);
-	if (!fences)
-		return -ENOMEM;
-
-	count = 0;
-	dma_fence_chain_for_each(tmp, &chain->base)
-		fences[count++] = dma_fence_get(tmp);
-
-	array = dma_fence_array_create(count, fences,
-				       dma_fence_context_alloc(1),
-				       1, false);
-	if (!array)
-		goto free_fences;
-
-	dma_fence_put(*f);
-	*f = &array->base;
-	return 0;
-
-free_fences:
-	while (count--)
-		dma_fence_put(fences[count]);
-
-	kfree(fences);
-	return -ENOMEM;
-}
-
 static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private,
 					    struct drm_syncobj_transfer *args)
 {
 	struct drm_syncobj *timeline_syncobj = NULL;
+	struct dma_fence *fence, *tmp;
 	struct dma_fence_chain *chain;
-	struct dma_fence *fence;
 	int ret;
 
 	timeline_syncobj = drm_syncobj_find(file_private, args->dst_handle);
@@ -912,13 +868,14 @@ static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private,
 	}
 	ret = drm_syncobj_find_fence(file_private, args->src_handle,
 				     args->src_point, args->flags,
-				     &fence);
+				     &tmp);
 	if (ret)
 		goto err_put_timeline;
 
-	ret = drm_syncobj_flatten_chain(&fence);
-	if (ret)
-		goto err_free_fence;
+	fence = dma_fence_unwrap_merge(tmp);
+	dma_fence_put(tmp);
+	if (!fence)
+		goto err_put_timeline;
 
 	chain = dma_fence_chain_alloc();
 	if (!chain) {
-- 
2.25.1

Re: [PATCH 1/5] dma-buf: cleanup dma_fence_unwrap selftest v2

[Christian König]

I had to send this out once more.

This time with the right mail addresses and a much simplified patch #3.

Christian.

Am 06.05.22 um 16:10 schrieb Christian König:
> The selftests, fix the error handling, remove unused functions and stop
> leaking memory in failed tests.
>
> v2: fix the memory leak correctly.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/dma-buf/st-dma-fence-unwrap.c | 48 +++++++++++----------------
>   1 file changed, 19 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c
> index 039f016b57be..e20c5a7dcfe4 100644
> --- a/drivers/dma-buf/st-dma-fence-unwrap.c
> +++ b/drivers/dma-buf/st-dma-fence-unwrap.c
> @@ -4,27 +4,19 @@
>    * Copyright (C) 2022 Advanced Micro Devices, Inc.
>    */
>   
> +#include <linux/dma-fence.h>
> +#include <linux/dma-fence-array.h>
> +#include <linux/dma-fence-chain.h>
>   #include <linux/dma-fence-unwrap.h>
> -#if 0
> -#include <linux/kernel.h>
> -#include <linux/kthread.h>
> -#include <linux/mm.h>
> -#include <linux/sched/signal.h>
> -#include <linux/slab.h>
> -#include <linux/spinlock.h>
> -#include <linux/random.h>
> -#endif
>   
>   #include "selftest.h"
>   
>   #define CHAIN_SZ (4 << 10)
>   
> -static inline struct mock_fence {
> +struct mock_fence {
>   	struct dma_fence base;
>   	spinlock_t lock;
> -} *to_mock_fence(struct dma_fence *f) {
> -	return container_of(f, struct mock_fence, base);
> -}
> +};
>   
>   static const char *mock_name(struct dma_fence *f)
>   {
> @@ -45,7 +37,8 @@ static struct dma_fence *mock_fence(void)
>   		return NULL;
>   
>   	spin_lock_init(&f->lock);
> -	dma_fence_init(&f->base, &mock_ops, &f->lock, 0, 0);
> +	dma_fence_init(&f->base, &mock_ops, &f->lock,
> +		       dma_fence_context_alloc(1), 1);
>   
>   	return &f->base;
>   }
> @@ -59,7 +52,7 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...)
>   
>   	fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
>   	if (!fences)
> -		return NULL;
> +		goto error_put;
>   
>   	va_start(valist, num_fences);
>   	for (i = 0; i < num_fences; ++i)
> @@ -70,13 +63,17 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...)
>   				       dma_fence_context_alloc(1),
>   				       1, false);
>   	if (!array)
> -		goto cleanup;
> +		goto error_free;
>   	return &array->base;
>   
> -cleanup:
> -	for (i = 0; i < num_fences; ++i)
> -		dma_fence_put(fences[i]);
> +error_free:
>   	kfree(fences);
> +
> +error_put:
> +	va_start(valist, num_fences);
> +	for (i = 0; i < num_fences; ++i)
> +		dma_fence_put(va_arg(valist, typeof(*fences)));
> +	va_end(valist);
>   	return NULL;
>   }
>   
> @@ -113,7 +110,6 @@ static int sanitycheck(void *arg)
>   	if (!chain)
>   		return -ENOMEM;
>   
> -	dma_fence_signal(f);
>   	dma_fence_put(chain);
>   	return err;
>   }
> @@ -154,10 +150,8 @@ static int unwrap_array(void *arg)
>   		err = -EINVAL;
>   	}
>   
> -	dma_fence_signal(f1);
> -	dma_fence_signal(f2);
>   	dma_fence_put(array);
> -	return 0;
> +	return err;
>   }
>   
>   static int unwrap_chain(void *arg)
> @@ -196,10 +190,8 @@ static int unwrap_chain(void *arg)
>   		err = -EINVAL;
>   	}
>   
> -	dma_fence_signal(f1);
> -	dma_fence_signal(f2);
>   	dma_fence_put(chain);
> -	return 0;
> +	return err;
>   }
>   
>   static int unwrap_chain_array(void *arg)
> @@ -242,10 +234,8 @@ static int unwrap_chain_array(void *arg)
>   		err = -EINVAL;
>   	}
>   
> -	dma_fence_signal(f1);
> -	dma_fence_signal(f2);
>   	dma_fence_put(chain);
> -	return 0;
> +	return err;
>   }
>   
>   int dma_fence_unwrap(void)

Re: [PATCH 1/5] dma-buf: cleanup dma_fence_unwrap selftest v2

[Christian König]

Hey Daniel,

a gentle ping on this here. Those patches come before my drm-exec work, 
so would be nice if we could get that reviewed first.

Thanks,
Christian.

Am 06.05.22 um 16:11 schrieb Christian König:
> I had to send this out once more.
>
> This time with the right mail addresses and a much simplified patch #3.
>
> Christian.
>
> Am 06.05.22 um 16:10 schrieb Christian König:
>> The selftests, fix the error handling, remove unused functions and stop
>> leaking memory in failed tests.
>>
>> v2: fix the memory leak correctly.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/dma-buf/st-dma-fence-unwrap.c | 48 +++++++++++----------------
>>   1 file changed, 19 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c 
>> b/drivers/dma-buf/st-dma-fence-unwrap.c
>> index 039f016b57be..e20c5a7dcfe4 100644
>> --- a/drivers/dma-buf/st-dma-fence-unwrap.c
>> +++ b/drivers/dma-buf/st-dma-fence-unwrap.c
>> @@ -4,27 +4,19 @@
>>    * Copyright (C) 2022 Advanced Micro Devices, Inc.
>>    */
>>   +#include <linux/dma-fence.h>
>> +#include <linux/dma-fence-array.h>
>> +#include <linux/dma-fence-chain.h>
>>   #include <linux/dma-fence-unwrap.h>
>> -#if 0
>> -#include <linux/kernel.h>
>> -#include <linux/kthread.h>
>> -#include <linux/mm.h>
>> -#include <linux/sched/signal.h>
>> -#include <linux/slab.h>
>> -#include <linux/spinlock.h>
>> -#include <linux/random.h>
>> -#endif
>>     #include "selftest.h"
>>     #define CHAIN_SZ (4 << 10)
>>   -static inline struct mock_fence {
>> +struct mock_fence {
>>       struct dma_fence base;
>>       spinlock_t lock;
>> -} *to_mock_fence(struct dma_fence *f) {
>> -    return container_of(f, struct mock_fence, base);
>> -}
>> +};
>>     static const char *mock_name(struct dma_fence *f)
>>   {
>> @@ -45,7 +37,8 @@ static struct dma_fence *mock_fence(void)
>>           return NULL;
>>         spin_lock_init(&f->lock);
>> -    dma_fence_init(&f->base, &mock_ops, &f->lock, 0, 0);
>> +    dma_fence_init(&f->base, &mock_ops, &f->lock,
>> +               dma_fence_context_alloc(1), 1);
>>         return &f->base;
>>   }
>> @@ -59,7 +52,7 @@ static struct dma_fence *mock_array(unsigned int 
>> num_fences, ...)
>>         fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
>>       if (!fences)
>> -        return NULL;
>> +        goto error_put;
>>         va_start(valist, num_fences);
>>       for (i = 0; i < num_fences; ++i)
>> @@ -70,13 +63,17 @@ static struct dma_fence *mock_array(unsigned int 
>> num_fences, ...)
>>                          dma_fence_context_alloc(1),
>>                          1, false);
>>       if (!array)
>> -        goto cleanup;
>> +        goto error_free;
>>       return &array->base;
>>   -cleanup:
>> -    for (i = 0; i < num_fences; ++i)
>> -        dma_fence_put(fences[i]);
>> +error_free:
>>       kfree(fences);
>> +
>> +error_put:
>> +    va_start(valist, num_fences);
>> +    for (i = 0; i < num_fences; ++i)
>> +        dma_fence_put(va_arg(valist, typeof(*fences)));
>> +    va_end(valist);
>>       return NULL;
>>   }
>>   @@ -113,7 +110,6 @@ static int sanitycheck(void *arg)
>>       if (!chain)
>>           return -ENOMEM;
>>   -    dma_fence_signal(f);
>>       dma_fence_put(chain);
>>       return err;
>>   }
>> @@ -154,10 +150,8 @@ static int unwrap_array(void *arg)
>>           err = -EINVAL;
>>       }
>>   -    dma_fence_signal(f1);
>> -    dma_fence_signal(f2);
>>       dma_fence_put(array);
>> -    return 0;
>> +    return err;
>>   }
>>     static int unwrap_chain(void *arg)
>> @@ -196,10 +190,8 @@ static int unwrap_chain(void *arg)
>>           err = -EINVAL;
>>       }
>>   -    dma_fence_signal(f1);
>> -    dma_fence_signal(f2);
>>       dma_fence_put(chain);
>> -    return 0;
>> +    return err;
>>   }
>>     static int unwrap_chain_array(void *arg)
>> @@ -242,10 +234,8 @@ static int unwrap_chain_array(void *arg)
>>           err = -EINVAL;
>>       }
>>   -    dma_fence_signal(f1);
>> -    dma_fence_signal(f2);
>>       dma_fence_put(chain);
>> -    return 0;
>> +    return err;
>>   }
>>     int dma_fence_unwrap(void)
>

Re: [PATCH 3/5] dma-buf: return only unsignaled fences in dma_fence_unwrap_for_each v3

[Daniel Vetter]

On Fri, May 06, 2022 at 04:10:07PM +0200, Christian König wrote:
> dma_fence_chain containers cleanup signaled fences automatically, so
> filter those out from arrays as well.
> 
> v2: fix missing walk over the array
> v3: massively simplify the patch and actually update the description.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  include/linux/dma-fence-unwrap.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
> index e7c219da4ed7..a4d342fef8e0 100644
> --- a/include/linux/dma-fence-unwrap.h
> +++ b/include/linux/dma-fence-unwrap.h
> @@ -43,9 +43,13 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor);
>   * Unwrap dma_fence_chain and dma_fence_array containers and deep dive into all
>   * potential fences in them. If @head is just a normal fence only that one is
>   * returned.
> + *
> + * Note that signalled fences are opportunistically filtered out, which
> + * means the iteration is potentially over no fence at all.
>   */
>  #define dma_fence_unwrap_for_each(fence, cursor, head)			\
>  	for (fence = dma_fence_unwrap_first(head, cursor); fence;	\
> -	     fence = dma_fence_unwrap_next(cursor))
> +	     fence = dma_fence_unwrap_next(cursor))			\
> +		if (!dma_fence_is_signaled(fence))
>  
>  #endif
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/5] dma-buf: cleanup dma_fence_unwrap selftest v2

[Daniel Vetter]

On Fri, May 06, 2022 at 04:10:05PM +0200, Christian König wrote:
> The selftests, fix the error handling, remove unused functions and stop
> leaking memory in failed tests.
> 
> v2: fix the memory leak correctly.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

I'm still a bit lost on all this (maybe add an explainer why you want to
drop dma_fence_signal() - it's just not necessary for test functionality).

But I think it's at least correct now.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I've seen you've resent it to get intel-gfx-ci to look at it, so assuming
that's all fine I think it's now all reviewed and ready for merging.
-Daniel
> ---
>  drivers/dma-buf/st-dma-fence-unwrap.c | 48 +++++++++++----------------
>  1 file changed, 19 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c
> index 039f016b57be..e20c5a7dcfe4 100644
> --- a/drivers/dma-buf/st-dma-fence-unwrap.c
> +++ b/drivers/dma-buf/st-dma-fence-unwrap.c
> @@ -4,27 +4,19 @@
>   * Copyright (C) 2022 Advanced Micro Devices, Inc.
>   */
>  
> +#include <linux/dma-fence.h>
> +#include <linux/dma-fence-array.h>
> +#include <linux/dma-fence-chain.h>
>  #include <linux/dma-fence-unwrap.h>
> -#if 0
> -#include <linux/kernel.h>
> -#include <linux/kthread.h>
> -#include <linux/mm.h>
> -#include <linux/sched/signal.h>
> -#include <linux/slab.h>
> -#include <linux/spinlock.h>
> -#include <linux/random.h>
> -#endif
>  
>  #include "selftest.h"
>  
>  #define CHAIN_SZ (4 << 10)
>  
> -static inline struct mock_fence {
> +struct mock_fence {
>  	struct dma_fence base;
>  	spinlock_t lock;
> -} *to_mock_fence(struct dma_fence *f) {
> -	return container_of(f, struct mock_fence, base);
> -}
> +};
>  
>  static const char *mock_name(struct dma_fence *f)
>  {
> @@ -45,7 +37,8 @@ static struct dma_fence *mock_fence(void)
>  		return NULL;
>  
>  	spin_lock_init(&f->lock);
> -	dma_fence_init(&f->base, &mock_ops, &f->lock, 0, 0);
> +	dma_fence_init(&f->base, &mock_ops, &f->lock,
> +		       dma_fence_context_alloc(1), 1);
>  
>  	return &f->base;
>  }
> @@ -59,7 +52,7 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...)
>  
>  	fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
>  	if (!fences)
> -		return NULL;
> +		goto error_put;
>  
>  	va_start(valist, num_fences);
>  	for (i = 0; i < num_fences; ++i)
> @@ -70,13 +63,17 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...)
>  				       dma_fence_context_alloc(1),
>  				       1, false);
>  	if (!array)
> -		goto cleanup;
> +		goto error_free;
>  	return &array->base;
>  
> -cleanup:
> -	for (i = 0; i < num_fences; ++i)
> -		dma_fence_put(fences[i]);
> +error_free:
>  	kfree(fences);
> +
> +error_put:
> +	va_start(valist, num_fences);
> +	for (i = 0; i < num_fences; ++i)
> +		dma_fence_put(va_arg(valist, typeof(*fences)));
> +	va_end(valist);
>  	return NULL;
>  }
>  
> @@ -113,7 +110,6 @@ static int sanitycheck(void *arg)
>  	if (!chain)
>  		return -ENOMEM;
>  
> -	dma_fence_signal(f);
>  	dma_fence_put(chain);
>  	return err;
>  }
> @@ -154,10 +150,8 @@ static int unwrap_array(void *arg)
>  		err = -EINVAL;
>  	}
>  
> -	dma_fence_signal(f1);
> -	dma_fence_signal(f2);
>  	dma_fence_put(array);
> -	return 0;
> +	return err;
>  }
>  
>  static int unwrap_chain(void *arg)
> @@ -196,10 +190,8 @@ static int unwrap_chain(void *arg)
>  		err = -EINVAL;
>  	}
>  
> -	dma_fence_signal(f1);
> -	dma_fence_signal(f2);
>  	dma_fence_put(chain);
> -	return 0;
> +	return err;
>  }
>  
>  static int unwrap_chain_array(void *arg)
> @@ -242,10 +234,8 @@ static int unwrap_chain_array(void *arg)
>  		err = -EINVAL;
>  	}
>  
> -	dma_fence_signal(f1);
> -	dma_fence_signal(f2);
>  	dma_fence_put(chain);
> -	return 0;
> +	return err;
>  }
>  
>  int dma_fence_unwrap(void)
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 3/5] dma-buf: return only unsignaled fences in dma_fence_unwrap_for_each v3

[Karolina Drobnik]

Hi Christian,

I'm sorry for digging this one out so late.

On 06.05.2022 16:10, Christian König wrote:
> dma_fence_chain containers cleanup signaled fences automatically, so
> filter those out from arrays as well.
> 
> v2: fix missing walk over the array
> v3: massively simplify the patch and actually update the description.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   include/linux/dma-fence-unwrap.h | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
> index e7c219da4ed7..a4d342fef8e0 100644
> --- a/include/linux/dma-fence-unwrap.h
> +++ b/include/linux/dma-fence-unwrap.h
> @@ -43,9 +43,13 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor);
>    * Unwrap dma_fence_chain and dma_fence_array containers and deep dive into all
>    * potential fences in them. If @head is just a normal fence only that one is
>    * returned.
> + *
> + * Note that signalled fences are opportunistically filtered out, which
> + * means the iteration is potentially over no fence at all.
>    */
>   #define dma_fence_unwrap_for_each(fence, cursor, head)			\
>   	for (fence = dma_fence_unwrap_first(head, cursor); fence;	\
> -	     fence = dma_fence_unwrap_next(cursor))
> +	     fence = dma_fence_unwrap_next(cursor))			\
> +		if (!dma_fence_is_signaled(fence))
>   
>   #endif

It looks like this particular patch affects merging Sync Fences, which 
is reflected by failing IGT test (igt@sw_sync)[1]. The failing subtests are:
   - sync_merge - merging different fences on the same timeline, neither
		 single nor merged fences are signaled

   - sync_merge_same - merging the fence with itself on the same
		 timeline, the fence didn't signal at all

   - sync_multi_timeline_wait - merging different fences on different
		 timelines; the subtest checks if counting fences of
		 various states works. Currently, it can only see 2
		 active fences, 0 signaling (should be 2 active,
		 1 signaling)

Reverting this commit on the top of drm-tip fixes the issue, but I'm not 
sure if it wouldn't impact other places in the code. Please let me know 
if I can be of any help.

All the best,
Karolina

---------------------
[1] - reproducible locally, but can be also seen in the CI:
https://intel-gfx-ci.01.org/tree/drm-tip/index.html?testfilter=igt@sw_sync

Re: [PATCH 3/5] dma-buf: return only unsignaled fences in dma_fence_unwrap_for_each v3

[Christian König]

Hi Karolina,

Am 11.07.22 um 11:44 schrieb Karolina Drobnik:
> Hi Christian,
>
> I'm sorry for digging this one out so late.
>
> On 06.05.2022 16:10, Christian König wrote:
>> dma_fence_chain containers cleanup signaled fences automatically, so
>> filter those out from arrays as well.
>>
>> v2: fix missing walk over the array
>> v3: massively simplify the patch and actually update the description.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   include/linux/dma-fence-unwrap.h | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/dma-fence-unwrap.h 
>> b/include/linux/dma-fence-unwrap.h
>> index e7c219da4ed7..a4d342fef8e0 100644
>> --- a/include/linux/dma-fence-unwrap.h
>> +++ b/include/linux/dma-fence-unwrap.h
>> @@ -43,9 +43,13 @@ struct dma_fence *dma_fence_unwrap_next(struct 
>> dma_fence_unwrap *cursor);
>>    * Unwrap dma_fence_chain and dma_fence_array containers and deep 
>> dive into all
>>    * potential fences in them. If @head is just a normal fence only 
>> that one is
>>    * returned.
>> + *
>> + * Note that signalled fences are opportunistically filtered out, which
>> + * means the iteration is potentially over no fence at all.
>>    */
>>   #define dma_fence_unwrap_for_each(fence, cursor, head)            \
>>       for (fence = dma_fence_unwrap_first(head, cursor); fence;    \
>> -         fence = dma_fence_unwrap_next(cursor))
>> +         fence = dma_fence_unwrap_next(cursor))            \
>> +        if (!dma_fence_is_signaled(fence))
>>     #endif
>
> It looks like this particular patch affects merging Sync Fences, which 
> is reflected by failing IGT test (igt@sw_sync)[1]. The failing 
> subtests are:
>   - sync_merge - merging different fences on the same timeline, neither
>          single nor merged fences are signaled
>
>   - sync_merge_same - merging the fence with itself on the same
>          timeline, the fence didn't signal at all
>
>   - sync_multi_timeline_wait - merging different fences on different
>          timelines; the subtest checks if counting fences of
>          various states works. Currently, it can only see 2
>          active fences, 0 signaling (should be 2 active,
>          1 signaling)
>
> Reverting this commit on the top of drm-tip fixes the issue, but I'm 
> not sure if it wouldn't impact other places in the code. Please let me 
> know if I can be of any help.


Thanks for letting me know. Not sure what's going on here, but I can 
take a look today if time permits.

Do you have a description how to easy reproduce this? E.g. how to run 
just those specific igts?

Thanks,
Christian.

>
> All the best,
> Karolina
>
> ---------------------
> [1] - reproducible locally, but can be also seen in the CI:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fintel-gfx-ci.01.org%2Ftree%2Fdrm-tip%2Findex.html%3Ftestfilter%3Digt%40sw_sync&amp;data=05%7C01%7Cchristian.koenig%40amd.com%7C2af59c808f664f0cf04908da6321e708%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637931294507736831%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=NTI04OpP7kMsCu%2BDWsJ0%2FRIVJGJbxy36tJBImD2MQDU%3D&amp;reserved=0 
>

Re: [PATCH 3/5] dma-buf: return only unsignaled fences in dma_fence_unwrap_for_each v3

[Karolina Drobnik]

Hi Christian,

On 11.07.2022 11:57, Christian König wrote:
> Hi Karolina,
> 
> Am 11.07.22 um 11:44 schrieb Karolina Drobnik:
>> Hi Christian,
>>
>> I'm sorry for digging this one out so late.
>>
>> On 06.05.2022 16:10, Christian König wrote:
>>> dma_fence_chain containers cleanup signaled fences automatically, so
>>> filter those out from arrays as well.
>>>
>>> v2: fix missing walk over the array
>>> v3: massively simplify the patch and actually update the description.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   include/linux/dma-fence-unwrap.h | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/dma-fence-unwrap.h 
>>> b/include/linux/dma-fence-unwrap.h
>>> index e7c219da4ed7..a4d342fef8e0 100644
>>> --- a/include/linux/dma-fence-unwrap.h
>>> +++ b/include/linux/dma-fence-unwrap.h
>>> @@ -43,9 +43,13 @@ struct dma_fence *dma_fence_unwrap_next(struct 
>>> dma_fence_unwrap *cursor);
>>>    * Unwrap dma_fence_chain and dma_fence_array containers and deep 
>>> dive into all
>>>    * potential fences in them. If @head is just a normal fence only 
>>> that one is
>>>    * returned.
>>> + *
>>> + * Note that signalled fences are opportunistically filtered out, which
>>> + * means the iteration is potentially over no fence at all.
>>>    */
>>>   #define dma_fence_unwrap_for_each(fence, cursor, head)            \
>>>       for (fence = dma_fence_unwrap_first(head, cursor); fence;    \
>>> -         fence = dma_fence_unwrap_next(cursor))
>>> +         fence = dma_fence_unwrap_next(cursor))            \
>>> +        if (!dma_fence_is_signaled(fence))
>>>     #endif
>>
>> It looks like this particular patch affects merging Sync Fences, which 
>> is reflected by failing IGT test (igt@sw_sync)[1]. The failing 
>> subtests are:
>>   - sync_merge - merging different fences on the same timeline, neither
>>          single nor merged fences are signaled
>>
>>   - sync_merge_same - merging the fence with itself on the same
>>          timeline, the fence didn't signal at all
>>
>>   - sync_multi_timeline_wait - merging different fences on different
>>          timelines; the subtest checks if counting fences of
>>          various states works. Currently, it can only see 2
>>          active fences, 0 signaling (should be 2 active,
>>          1 signaling)
>>
>> Reverting this commit on the top of drm-tip fixes the issue, but I'm 
>> not sure if it wouldn't impact other places in the code. Please let me 
>> know if I can be of any help.
> 
> 
> Thanks for letting me know. Not sure what's going on here, but I can 
> take a look today if time permits.

The reproduction with IGTs should be quite easy. You'll need to 
clone/download the IGT code and follow instructions for Building[1] the 
project (make sure you have meson and ninja installed):

   https://gitlab.freedesktop.org/drm/igt-gpu-tools

Once you have it up and running, go to <igt path>/build/tests, and run 
the subtests:

   ./sw_sync --run sync_merge
   ./sw_sync --run sync_merge_same
   ./sw_sync --run sync_multi_timeline_wait

You can run all the subtests with ./sw_sync, but I think these are the 
most relevant to you.

Many thanks,
Karolina

------------------
[1] - https://gitlab.freedesktop.org/drm/igt-gpu-tools#building

> Do you have a description how to easy reproduce this? E.g. how to run 
> just those specific igts?
> 
> Thanks,
> Christian.
> 
>>
>> All the best,
>> Karolina
>>

Re: [PATCH 3/5] dma-buf: return only unsignaled fences in dma_fence_unwrap_for_each v3

[Christian König]

Hi Karolina,

Am 11.07.22 um 14:17 schrieb Karolina Drobnik:
> Hi Christian,
>
> On 11.07.2022 11:57, Christian König wrote:
>> Hi Karolina,
>>
>> Am 11.07.22 um 11:44 schrieb Karolina Drobnik:
>>> Hi Christian,
>>>
>>> I'm sorry for digging this one out so late.
>>>
>>> On 06.05.2022 16:10, Christian König wrote:
>>>> dma_fence_chain containers cleanup signaled fences automatically, so
>>>> filter those out from arrays as well.
>>>>
>>>> v2: fix missing walk over the array
>>>> v3: massively simplify the patch and actually update the description.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>   include/linux/dma-fence-unwrap.h | 6 +++++-
>>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/linux/dma-fence-unwrap.h 
>>>> b/include/linux/dma-fence-unwrap.h
>>>> index e7c219da4ed7..a4d342fef8e0 100644
>>>> --- a/include/linux/dma-fence-unwrap.h
>>>> +++ b/include/linux/dma-fence-unwrap.h
>>>> @@ -43,9 +43,13 @@ struct dma_fence *dma_fence_unwrap_next(struct 
>>>> dma_fence_unwrap *cursor);
>>>>    * Unwrap dma_fence_chain and dma_fence_array containers and deep 
>>>> dive into all
>>>>    * potential fences in them. If @head is just a normal fence only 
>>>> that one is
>>>>    * returned.
>>>> + *
>>>> + * Note that signalled fences are opportunistically filtered out, 
>>>> which
>>>> + * means the iteration is potentially over no fence at all.
>>>>    */
>>>>   #define dma_fence_unwrap_for_each(fence, cursor, head)            \
>>>>       for (fence = dma_fence_unwrap_first(head, cursor); fence;    \
>>>> -         fence = dma_fence_unwrap_next(cursor))
>>>> +         fence = dma_fence_unwrap_next(cursor)) \
>>>> +        if (!dma_fence_is_signaled(fence))
>>>>     #endif
>>>
>>> It looks like this particular patch affects merging Sync Fences, 
>>> which is reflected by failing IGT test (igt@sw_sync)[1]. The failing 
>>> subtests are:
>>>   - sync_merge - merging different fences on the same timeline, neither
>>>          single nor merged fences are signaled
>>>
>>>   - sync_merge_same - merging the fence with itself on the same
>>>          timeline, the fence didn't signal at all
>>>
>>>   - sync_multi_timeline_wait - merging different fences on different
>>>          timelines; the subtest checks if counting fences of
>>>          various states works. Currently, it can only see 2
>>>          active fences, 0 signaling (should be 2 active,
>>>          1 signaling)
>>>
>>> Reverting this commit on the top of drm-tip fixes the issue, but I'm 
>>> not sure if it wouldn't impact other places in the code. Please let 
>>> me know if I can be of any help.
>>
>>
>> Thanks for letting me know. Not sure what's going on here, but I can 
>> take a look today if time permits.
>
> The reproduction with IGTs should be quite easy. You'll need to 
> clone/download the IGT code and follow instructions for Building[1] 
> the project (make sure you have meson and ninja installed):
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Figt-gpu-tools&amp;data=05%7C01%7Cchristian.koenig%40amd.com%7C9a9587aefd2d4ac2d86208da63375cb6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637931386683611766%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=4WsMutcFJ2HwBqld%2BTv9N1Tx6cbFMwJJZ6kjm5rbfoI%3D&amp;reserved=0
>
> Once you have it up and running, go to <igt path>/build/tests, and run 
> the subtests:
>
>   ./sw_sync --run sync_merge
>   ./sw_sync --run sync_merge_same
>   ./sw_sync --run sync_multi_timeline_wait
>
> You can run all the subtests with ./sw_sync, but I think these are the 
> most relevant to you.

Thanks, I've already managed to reproduce it.

Not sure what's going on here, but could be that the test case was never 
correct in the first place. Need to double check.

Thanks,
Christian.

>
> Many thanks,
> Karolina
>
> ------------------
> [1] - 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Figt-gpu-tools%23building&amp;data=05%7C01%7Cchristian.koenig%40amd.com%7C9a9587aefd2d4ac2d86208da63375cb6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637931386683611766%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=FV0Ao6ra8EOyr4cOs4N7mCmpOEUUObTrgyOrd0tvEV8%3D&amp;reserved=0
>
>> Do you have a description how to easy reproduce this? E.g. how to run 
>> just those specific igts?
>>
>> Thanks,
>> Christian.
>>
>>>
>>> All the best,
>>> Karolina
>>>

Re: [PATCH 3/5] dma-buf: return only unsignaled fences in dma_fence_unwrap_for_each v3

[Karolina Drobnik]

Hi Christian,

On 11.07.2022 14:25, Christian König wrote:
> Hi Karolina,
> 
> Am 11.07.22 um 14:17 schrieb Karolina Drobnik:
>> Hi Christian,
>>
>> On 11.07.2022 11:57, Christian König wrote:
>>> Hi Karolina,
>>>
>>> Am 11.07.22 um 11:44 schrieb Karolina Drobnik:
>>>> Hi Christian,
>>>>
>>>> I'm sorry for digging this one out so late.
>>>>
>>>> On 06.05.2022 16:10, Christian König wrote:
>>>>> dma_fence_chain containers cleanup signaled fences automatically, so
>>>>> filter those out from arrays as well.
>>>>>
>>>>> v2: fix missing walk over the array
>>>>> v3: massively simplify the patch and actually update the description.
>>>>>
>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>> ---
>>>>>   include/linux/dma-fence-unwrap.h | 6 +++++-
>>>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/include/linux/dma-fence-unwrap.h 
>>>>> b/include/linux/dma-fence-unwrap.h
>>>>> index e7c219da4ed7..a4d342fef8e0 100644
>>>>> --- a/include/linux/dma-fence-unwrap.h
>>>>> +++ b/include/linux/dma-fence-unwrap.h
>>>>> @@ -43,9 +43,13 @@ struct dma_fence *dma_fence_unwrap_next(struct 
>>>>> dma_fence_unwrap *cursor);
>>>>>    * Unwrap dma_fence_chain and dma_fence_array containers and deep 
>>>>> dive into all
>>>>>    * potential fences in them. If @head is just a normal fence only 
>>>>> that one is
>>>>>    * returned.
>>>>> + *
>>>>> + * Note that signalled fences are opportunistically filtered out, 
>>>>> which
>>>>> + * means the iteration is potentially over no fence at all.
>>>>>    */
>>>>>   #define dma_fence_unwrap_for_each(fence, cursor, head)            \
>>>>>       for (fence = dma_fence_unwrap_first(head, cursor); fence;    \
>>>>> -         fence = dma_fence_unwrap_next(cursor))
>>>>> +         fence = dma_fence_unwrap_next(cursor)) \
>>>>> +        if (!dma_fence_is_signaled(fence))
>>>>>     #endif
>>>>
>>>> It looks like this particular patch affects merging Sync Fences, 
>>>> which is reflected by failing IGT test (igt@sw_sync)[1]. The failing 
>>>> subtests are:
>>>>   - sync_merge - merging different fences on the same timeline, neither
>>>>          single nor merged fences are signaled
>>>>
>>>>   - sync_merge_same - merging the fence with itself on the same
>>>>          timeline, the fence didn't signal at all
>>>>
>>>>   - sync_multi_timeline_wait - merging different fences on different
>>>>          timelines; the subtest checks if counting fences of
>>>>          various states works. Currently, it can only see 2
>>>>          active fences, 0 signaling (should be 2 active,
>>>>          1 signaling)
>>>>
>>>> Reverting this commit on the top of drm-tip fixes the issue, but I'm 
>>>> not sure if it wouldn't impact other places in the code. Please let 
>>>> me know if I can be of any help.
>>>
>>>
>>> Thanks for letting me know. Not sure what's going on here, but I can 
>>> take a look today if time permits.
>>
>> The reproduction with IGTs should be quite easy. You'll need to 
>> clone/download the IGT code and follow instructions for Building[1] 
>> the project (make sure you have meson and ninja installed):
>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Figt-gpu-tools&amp;data=05%7C01%7Cchristian.koenig%40amd.com%7C9a9587aefd2d4ac2d86208da63375cb6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637931386683611766%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=4WsMutcFJ2HwBqld%2BTv9N1Tx6cbFMwJJZ6kjm5rbfoI%3D&amp;reserved=0 
>>
>>
>> Once you have it up and running, go to <igt path>/build/tests, and run 
>> the subtests:
>>
>>   ./sw_sync --run sync_merge
>>   ./sw_sync --run sync_merge_same
>>   ./sw_sync --run sync_multi_timeline_wait
>>
>> You can run all the subtests with ./sw_sync, but I think these are the 
>> most relevant to you.
> 
> Thanks, I've already managed to reproduce it.
> 
> Not sure what's going on here, but could be that the test case was never 
> correct in the first place. Need to double check.

That's also a possibility, but I couldn't verify it before writing to 
you, as it's not my area of expertise.

Thanks for taking a look at this.

All the best,
Karolina

> Thanks,
> Christian.
> 
>>
>> Many thanks,
>> Karolina
>>
>> ------------------
>> [1] - 
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Figt-gpu-tools%23building&amp;data=05%7C01%7Cchristian.koenig%40amd.com%7C9a9587aefd2d4ac2d86208da63375cb6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637931386683611766%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=FV0Ao6ra8EOyr4cOs4N7mCmpOEUUObTrgyOrd0tvEV8%3D&amp;reserved=0 
>>
>>
>>> Do you have a description how to easy reproduce this? E.g. how to run 
>>> just those specific igts?
>>>
>>> Thanks,
>>> Christian.
>>>
>>>>
>>>> All the best,
>>>> Karolina
>>>>
> 

Re: [PATCH 4/5] dma-buf: generalize dma_fence unwrap & merging v2

[Daniel Vetter]

On Wed, May 04, 2022 at 02:22:55PM +0200, Christian König wrote:
> Introduce a dma_fence_unwrap_merge() macro which allows to unwrap fences
> which potentially can be containers as well and then merge them back
> together into a flat dma_fence_array.
> 
> v2: rename the function, add some more comments about how the wrapper is
>     used, move filtering of signaled fences into the unwrap iterator,
>     add complex selftest which covers more cases.

Yeah this addresses everything. The only thing that leaves me puzzled is
the unconditional dma_fence_put() in the testcases for the fences we stuff
into containers, I thought we only need that for error cases but not the
success. But I'm confused about that already in a previous patch.

Aside from that Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>


> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/dma-buf/dma-fence-unwrap.c    |  99 +++++++++++++++++++++
>  drivers/dma-buf/st-dma-fence-unwrap.c | 109 +++++++++++++++++++++++
>  drivers/dma-buf/sync_file.c           | 119 ++------------------------
>  include/linux/dma-fence-unwrap.h      |  24 ++++++
>  4 files changed, 238 insertions(+), 113 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c
> index 7b0b91086ded..4ed5ea8807d7 100644
> --- a/drivers/dma-buf/dma-fence-unwrap.c
> +++ b/drivers/dma-buf/dma-fence-unwrap.c
> @@ -11,6 +11,7 @@
>  #include <linux/dma-fence-array.h>
>  #include <linux/dma-fence-chain.h>
>  #include <linux/dma-fence-unwrap.h>
> +#include <linux/slab.h>
>  
>  /* Internal helper to start new array iteration, don't use directly */
>  static struct dma_fence *
> @@ -66,3 +67,101 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor)
>  	return tmp;
>  }
>  EXPORT_SYMBOL_GPL(dma_fence_unwrap_next);
> +
> +/* Implementation for the dma_fence_merge() marco, don't use directly */
> +struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
> +					   struct dma_fence **fences,
> +					   struct dma_fence_unwrap *iter)
> +{
> +	struct dma_fence_array *result;
> +	struct dma_fence *tmp, **array;
> +	unsigned int i;
> +	size_t count;
> +
> +	count = 0;
> +	for (i = 0; i < num_fences; ++i) {
> +		dma_fence_unwrap_for_each(tmp, &iter[i], fences[i])
> +			++count;
> +	}
> +
> +	if (count == 0)
> +		return dma_fence_get_stub();
> +
> +	array = kmalloc_array(count, sizeof(*array), GFP_KERNEL);
> +	if (!array)
> +		return NULL;
> +
> +	/*
> +	 * This trashes the input fence array and uses it as position for the
> +	 * following merge loop. This works because the dma_fence_merge()
> +	 * wrapper macro is creating this temporary array on the stack together
> +	 * with the iterators.
> +	 */
> +	for (i = 0; i < num_fences; ++i)
> +		fences[i] = dma_fence_unwrap_first(fences[i], &iter[i]);
> +
> +	count = 0;
> +	do {
> +		unsigned int sel;
> +
> +restart:
> +		tmp = NULL;
> +		for (i = 0; i < num_fences; ++i) {
> +			struct dma_fence *next = fences[i];
> +
> +			if (!next)
> +				continue;
> +
> +			/*
> +			 * We can't guarantee that inpute fences are ordered by
> +			 * context, but it is still quite likely when this
> +			 * function is used multiple times. So attempt to order
> +			 * the fences by context as we pass over them and merge
> +			 * fences with the same context.
> +			 */
> +			if (!tmp || tmp->context > next->context) {
> +				tmp = next;
> +				sel = i;
> +
> +			} else if (tmp->context < next->context) {
> +				continue;
> +
> +			} else if (dma_fence_is_later(tmp, next)) {
> +				fences[i] = dma_fence_unwrap_next(&iter[i]);
> +				goto restart;
> +			} else {
> +				fences[sel] = dma_fence_unwrap_next(&iter[sel]);
> +				goto restart;
> +			}
> +		}
> +
> +		if (tmp) {
> +			array[count++] = dma_fence_get(tmp);
> +			fences[sel] = dma_fence_unwrap_next(&iter[sel]);
> +		}
> +	} while (tmp);
> +
> +	if (count == 0) {
> +		tmp = dma_fence_get_stub();
> +		goto return_tmp;
> +	}
> +
> +	if (count == 1) {
> +		tmp = array[0];
> +		goto return_tmp;
> +	}
> +
> +	result = dma_fence_array_create(count, array,
> +					dma_fence_context_alloc(1),
> +					1, false);
> +	if (!result) {
> +		tmp = NULL;
> +		goto return_tmp;
> +	}
> +	return &result->base;
> +
> +return_tmp:
> +	kfree(array);
> +	return tmp;
> +}
> +EXPORT_SYMBOL_GPL(__dma_fence_unwrap_merge);
> diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c
> index 59628add93f5..3a8aca651938 100644
> --- a/drivers/dma-buf/st-dma-fence-unwrap.c
> +++ b/drivers/dma-buf/st-dma-fence-unwrap.c
> @@ -240,6 +240,113 @@ static int unwrap_chain_array(void *arg)
>  	return err;
>  }
>  
> +static int unwrap_merge(void *arg)
> +{
> +	struct dma_fence *fence, *f1, *f2, *f3;
> +	struct dma_fence_unwrap iter;
> +	int err = 0;
> +
> +	f1 = mock_fence();
> +	if (!f1)
> +		return -ENOMEM;
> +
> +	f2 = mock_fence();
> +	if (!f2) {
> +		err = -ENOMEM;
> +		goto error_put_f1;
> +	}
> +
> +	f3 = dma_fence_unwrap_merge(f1, f2);
> +	if (!f3) {
> +		err = -ENOMEM;
> +		goto error_put_f2;
> +	}
> +
> +	dma_fence_unwrap_for_each(fence, &iter, f3) {
> +		if (fence == f1) {
> +			dma_fence_put(f1);
> +			f1 = NULL;
> +		} else if (fence == f2) {
> +			dma_fence_put(f2);
> +			f2 = NULL;
> +		} else {
> +			pr_err("Unexpected fence!\n");
> +			err = -EINVAL;
> +		}
> +	}
> +
> +	if (f1 || f2) {
> +		pr_err("Not all fences seen!\n");
> +		err = -EINVAL;
> +	}
> +
> +	dma_fence_put(f3);
> +error_put_f2:
> +	dma_fence_put(f2);
> +error_put_f1:
> +	dma_fence_put(f1);
> +	return err;
> +}
> +
> +static int unwrap_merge_complex(void *arg)
> +{
> +	struct dma_fence *fence, *f1, *f2, *f3, *f4, *f5;
> +	struct dma_fence_unwrap iter;
> +	int err = -ENOMEM;
> +
> +	f1 = mock_fence();
> +	if (!f1)
> +		return -ENOMEM;
> +
> +	f2 = mock_fence();
> +	if (!f2)
> +		goto error_put_f1;
> +
> +	f3 = dma_fence_unwrap_merge(f1, f2);
> +	if (!f3)
> +		goto error_put_f2;
> +
> +	/* The resulting array has the fences in reverse */
> +	f4 = dma_fence_unwrap_merge(f2, f1);
> +	if (!f4)
> +		goto error_put_f3;
> +
> +	/* Signaled fences should be filtered, the two arrays merged. */
> +	f5 = dma_fence_unwrap_merge(f3, f4, dma_fence_get_stub());
> +	if (!f5)
> +		goto error_put_f4;
> +
> +	err = 0;
> +	dma_fence_unwrap_for_each(fence, &iter, f5) {
> +		if (fence == f1) {
> +			dma_fence_put(f1);
> +			f1 = NULL;
> +		} else if (fence == f2) {
> +			dma_fence_put(f2);
> +			f2 = NULL;
> +		} else {
> +			pr_err("Unexpected fence!\n");
> +			err = -EINVAL;
> +		}
> +	}
> +
> +	if (f1 || f2) {
> +		pr_err("Not all fences seen!\n");
> +		err = -EINVAL;
> +	}
> +
> +	dma_fence_put(f5);
> +error_put_f4:
> +	dma_fence_put(f4);
> +error_put_f3:
> +	dma_fence_put(f3);
> +error_put_f2:
> +	dma_fence_put(f2);
> +error_put_f1:
> +	dma_fence_put(f1);
> +	return err;
> +}
> +
>  int dma_fence_unwrap(void)
>  {
>  	static const struct subtest tests[] = {
> @@ -247,6 +354,8 @@ int dma_fence_unwrap(void)
>  		SUBTEST(unwrap_array),
>  		SUBTEST(unwrap_chain),
>  		SUBTEST(unwrap_chain_array),
> +		SUBTEST(unwrap_merge),
> +		SUBTEST(unwrap_merge_complex),
>  	};
>  
>  	return subtests(tests, NULL);
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 0fe564539166..3ebec19a8e02 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -146,50 +146,6 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len)
>  	return buf;
>  }
>  
> -static int sync_file_set_fence(struct sync_file *sync_file,
> -			       struct dma_fence **fences, int num_fences)
> -{
> -	struct dma_fence_array *array;
> -
> -	/*
> -	 * The reference for the fences in the new sync_file and held
> -	 * in add_fence() during the merge procedure, so for num_fences == 1
> -	 * we already own a new reference to the fence. For num_fence > 1
> -	 * we own the reference of the dma_fence_array creation.
> -	 */
> -
> -	if (num_fences == 0) {
> -		sync_file->fence = dma_fence_get_stub();
> -		kfree(fences);
> -
> -	} else if (num_fences == 1) {
> -		sync_file->fence = fences[0];
> -		kfree(fences);
> -
> -	} else {
> -		array = dma_fence_array_create(num_fences, fences,
> -					       dma_fence_context_alloc(1),
> -					       1, false);
> -		if (!array)
> -			return -ENOMEM;
> -
> -		sync_file->fence = &array->base;
> -	}
> -
> -	return 0;
> -}
> -
> -static void add_fence(struct dma_fence **fences,
> -		      int *i, struct dma_fence *fence)
> -{
> -	fences[*i] = fence;
> -
> -	if (!dma_fence_is_signaled(fence)) {
> -		dma_fence_get(fence);
> -		(*i)++;
> -	}
> -}
> -
>  /**
>   * sync_file_merge() - merge two sync_files
>   * @name:	name of new fence
> @@ -203,84 +159,21 @@ static void add_fence(struct dma_fence **fences,
>  static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
>  					 struct sync_file *b)
>  {
> -	struct dma_fence *a_fence, *b_fence, **fences;
> -	struct dma_fence_unwrap a_iter, b_iter;
> -	unsigned int index, num_fences;
>  	struct sync_file *sync_file;
> +	struct dma_fence *fence;
>  
>  	sync_file = sync_file_alloc();
>  	if (!sync_file)
>  		return NULL;
>  
> -	num_fences = 0;
> -	dma_fence_unwrap_for_each(a_fence, &a_iter, a->fence)
> -		++num_fences;
> -	dma_fence_unwrap_for_each(b_fence, &b_iter, b->fence)
> -		++num_fences;
> -
> -	if (num_fences > INT_MAX)
> -		goto err_free_sync_file;
> -
> -	fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
> -	if (!fences)
> -		goto err_free_sync_file;
> -
> -	/*
> -	 * We can't guarantee that fences in both a and b are ordered, but it is
> -	 * still quite likely.
> -	 *
> -	 * So attempt to order the fences as we pass over them and merge fences
> -	 * with the same context.
> -	 */
> -
> -	index = 0;
> -	for (a_fence = dma_fence_unwrap_first(a->fence, &a_iter),
> -	     b_fence = dma_fence_unwrap_first(b->fence, &b_iter);
> -	     a_fence || b_fence; ) {
> -
> -		if (!b_fence) {
> -			add_fence(fences, &index, a_fence);
> -			a_fence = dma_fence_unwrap_next(&a_iter);
> -
> -		} else if (!a_fence) {
> -			add_fence(fences, &index, b_fence);
> -			b_fence = dma_fence_unwrap_next(&b_iter);
> -
> -		} else if (a_fence->context < b_fence->context) {
> -			add_fence(fences, &index, a_fence);
> -			a_fence = dma_fence_unwrap_next(&a_iter);
> -
> -		} else if (b_fence->context < a_fence->context) {
> -			add_fence(fences, &index, b_fence);
> -			b_fence = dma_fence_unwrap_next(&b_iter);
> -
> -		} else if (__dma_fence_is_later(a_fence->seqno, b_fence->seqno,
> -						a_fence->ops)) {
> -			add_fence(fences, &index, a_fence);
> -			a_fence = dma_fence_unwrap_next(&a_iter);
> -			b_fence = dma_fence_unwrap_next(&b_iter);
> -
> -		} else {
> -			add_fence(fences, &index, b_fence);
> -			a_fence = dma_fence_unwrap_next(&a_iter);
> -			b_fence = dma_fence_unwrap_next(&b_iter);
> -		}
> +	fence = dma_fence_unwrap_merge(a->fence, b->fence);
> +	if (!fence) {
> +		fput(sync_file->file);
> +		return NULL;
>  	}
> -
> -	if (sync_file_set_fence(sync_file, fences, index) < 0)
> -		goto err_put_fences;
> -
> +	sync_file->fence = fence;
>  	strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name));
>  	return sync_file;
> -
> -err_put_fences:
> -	while (index)
> -		dma_fence_put(fences[--index]);
> -	kfree(fences);
> -
> -err_free_sync_file:
> -	fput(sync_file->file);
> -	return NULL;
>  }
>  
>  static int sync_file_release(struct inode *inode, struct file *file)
> diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
> index e9d114637294..43c8a9bbee88 100644
> --- a/include/linux/dma-fence-unwrap.h
> +++ b/include/linux/dma-fence-unwrap.h
> @@ -48,4 +48,28 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor);
>  	for (fence = dma_fence_unwrap_first(head, cursor); fence;	\
>  	     fence = dma_fence_unwrap_next(cursor))
>  
> +struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
> +					   struct dma_fence **fences,
> +					   struct dma_fence_unwrap *cursors);
> +
> +/**
> + * dma_fence_unwrap_merge - unwrap and merge fences
> + *
> + * All fences given as parameters are unwrapped and merged back together as flat
> + * dma_fence_array. Useful if multiple containers need to be merged together.
> + *
> + * Implemented as a macro to allocate the necessary arrays on the stack and
> + * account the stack frame size to the caller.
> + *
> + * Returns NULL on memory allocation failure, a dma_fence object representing
> + * all the given fences otherwise.
> + */
> +#define dma_fence_unwrap_merge(...)					\
> +	({								\
> +		struct dma_fence *__f[] = { __VA_ARGS__ };		\
> +		struct dma_fence_unwrap __c[ARRAY_SIZE(__f)];		\
> +									\
> +		__dma_fence_unwrap_merge(ARRAY_SIZE(__f), __f, __c);	\
> +	})
> +
>  #endif
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 4/5] dma-buf: generalize dma_fence unwrap & merging v2

[Christian König]

Introduce a dma_fence_unwrap_merge() macro which allows to unwrap fences
which potentially can be containers as well and then merge them back
together into a flat dma_fence_array.

v2: rename the function, add some more comments about how the wrapper is
    used, move filtering of signaled fences into the unwrap iterator,
    add complex selftest which covers more cases.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/dma-fence-unwrap.c    |  99 +++++++++++++++++++++
 drivers/dma-buf/st-dma-fence-unwrap.c | 109 +++++++++++++++++++++++
 drivers/dma-buf/sync_file.c           | 119 ++------------------------
 include/linux/dma-fence-unwrap.h      |  24 ++++++
 4 files changed, 238 insertions(+), 113 deletions(-)

diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c
index 7b0b91086ded..4ed5ea8807d7 100644
--- a/drivers/dma-buf/dma-fence-unwrap.c
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -11,6 +11,7 @@
 #include <linux/dma-fence-array.h>
 #include <linux/dma-fence-chain.h>
 #include <linux/dma-fence-unwrap.h>
+#include <linux/slab.h>
 
 /* Internal helper to start new array iteration, don't use directly */
 static struct dma_fence *
@@ -66,3 +67,101 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor)
 	return tmp;
 }
 EXPORT_SYMBOL_GPL(dma_fence_unwrap_next);
+
+/* Implementation for the dma_fence_merge() marco, don't use directly */
+struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
+					   struct dma_fence **fences,
+					   struct dma_fence_unwrap *iter)
+{
+	struct dma_fence_array *result;
+	struct dma_fence *tmp, **array;
+	unsigned int i;
+	size_t count;
+
+	count = 0;
+	for (i = 0; i < num_fences; ++i) {
+		dma_fence_unwrap_for_each(tmp, &iter[i], fences[i])
+			++count;
+	}
+
+	if (count == 0)
+		return dma_fence_get_stub();
+
+	array = kmalloc_array(count, sizeof(*array), GFP_KERNEL);
+	if (!array)
+		return NULL;
+
+	/*
+	 * This trashes the input fence array and uses it as position for the
+	 * following merge loop. This works because the dma_fence_merge()
+	 * wrapper macro is creating this temporary array on the stack together
+	 * with the iterators.
+	 */
+	for (i = 0; i < num_fences; ++i)
+		fences[i] = dma_fence_unwrap_first(fences[i], &iter[i]);
+
+	count = 0;
+	do {
+		unsigned int sel;
+
+restart:
+		tmp = NULL;
+		for (i = 0; i < num_fences; ++i) {
+			struct dma_fence *next = fences[i];
+
+			if (!next)
+				continue;
+
+			/*
+			 * We can't guarantee that inpute fences are ordered by
+			 * context, but it is still quite likely when this
+			 * function is used multiple times. So attempt to order
+			 * the fences by context as we pass over them and merge
+			 * fences with the same context.
+			 */
+			if (!tmp || tmp->context > next->context) {
+				tmp = next;
+				sel = i;
+
+			} else if (tmp->context < next->context) {
+				continue;
+
+			} else if (dma_fence_is_later(tmp, next)) {
+				fences[i] = dma_fence_unwrap_next(&iter[i]);
+				goto restart;
+			} else {
+				fences[sel] = dma_fence_unwrap_next(&iter[sel]);
+				goto restart;
+			}
+		}
+
+		if (tmp) {
+			array[count++] = dma_fence_get(tmp);
+			fences[sel] = dma_fence_unwrap_next(&iter[sel]);
+		}
+	} while (tmp);
+
+	if (count == 0) {
+		tmp = dma_fence_get_stub();
+		goto return_tmp;
+	}
+
+	if (count == 1) {
+		tmp = array[0];
+		goto return_tmp;
+	}
+
+	result = dma_fence_array_create(count, array,
+					dma_fence_context_alloc(1),
+					1, false);
+	if (!result) {
+		tmp = NULL;
+		goto return_tmp;
+	}
+	return &result->base;
+
+return_tmp:
+	kfree(array);
+	return tmp;
+}
+EXPORT_SYMBOL_GPL(__dma_fence_unwrap_merge);
diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c
index 59628add93f5..3a8aca651938 100644
--- a/drivers/dma-buf/st-dma-fence-unwrap.c
+++ b/drivers/dma-buf/st-dma-fence-unwrap.c
@@ -240,6 +240,113 @@ static int unwrap_chain_array(void *arg)
 	return err;
 }
 
+static int unwrap_merge(void *arg)
+{
+	struct dma_fence *fence, *f1, *f2, *f3;
+	struct dma_fence_unwrap iter;
+	int err = 0;
+
+	f1 = mock_fence();
+	if (!f1)
+		return -ENOMEM;
+
+	f2 = mock_fence();
+	if (!f2) {
+		err = -ENOMEM;
+		goto error_put_f1;
+	}
+
+	f3 = dma_fence_unwrap_merge(f1, f2);
+	if (!f3) {
+		err = -ENOMEM;
+		goto error_put_f2;
+	}
+
+	dma_fence_unwrap_for_each(fence, &iter, f3) {
+		if (fence == f1) {
+			dma_fence_put(f1);
+			f1 = NULL;
+		} else if (fence == f2) {
+			dma_fence_put(f2);
+			f2 = NULL;
+		} else {
+			pr_err("Unexpected fence!\n");
+			err = -EINVAL;
+		}
+	}
+
+	if (f1 || f2) {
+		pr_err("Not all fences seen!\n");
+		err = -EINVAL;
+	}
+
+	dma_fence_put(f3);
+error_put_f2:
+	dma_fence_put(f2);
+error_put_f1:
+	dma_fence_put(f1);
+	return err;
+}
+
+static int unwrap_merge_complex(void *arg)
+{
+	struct dma_fence *fence, *f1, *f2, *f3, *f4, *f5;
+	struct dma_fence_unwrap iter;
+	int err = -ENOMEM;
+
+	f1 = mock_fence();
+	if (!f1)
+		return -ENOMEM;
+
+	f2 = mock_fence();
+	if (!f2)
+		goto error_put_f1;
+
+	f3 = dma_fence_unwrap_merge(f1, f2);
+	if (!f3)
+		goto error_put_f2;
+
+	/* The resulting array has the fences in reverse */
+	f4 = dma_fence_unwrap_merge(f2, f1);
+	if (!f4)
+		goto error_put_f3;
+
+	/* Signaled fences should be filtered, the two arrays merged. */
+	f5 = dma_fence_unwrap_merge(f3, f4, dma_fence_get_stub());
+	if (!f5)
+		goto error_put_f4;
+
+	err = 0;
+	dma_fence_unwrap_for_each(fence, &iter, f5) {
+		if (fence == f1) {
+			dma_fence_put(f1);
+			f1 = NULL;
+		} else if (fence == f2) {
+			dma_fence_put(f2);
+			f2 = NULL;
+		} else {
+			pr_err("Unexpected fence!\n");
+			err = -EINVAL;
+		}
+	}
+
+	if (f1 || f2) {
+		pr_err("Not all fences seen!\n");
+		err = -EINVAL;
+	}
+
+	dma_fence_put(f5);
+error_put_f4:
+	dma_fence_put(f4);
+error_put_f3:
+	dma_fence_put(f3);
+error_put_f2:
+	dma_fence_put(f2);
+error_put_f1:
+	dma_fence_put(f1);
+	return err;
+}
+
 int dma_fence_unwrap(void)
 {
 	static const struct subtest tests[] = {
@@ -247,6 +354,8 @@ int dma_fence_unwrap(void)
 		SUBTEST(unwrap_array),
 		SUBTEST(unwrap_chain),
 		SUBTEST(unwrap_chain_array),
+		SUBTEST(unwrap_merge),
+		SUBTEST(unwrap_merge_complex),
 	};
 
 	return subtests(tests, NULL);
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 0fe564539166..3ebec19a8e02 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -146,50 +146,6 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len)
 	return buf;
 }
 
-static int sync_file_set_fence(struct sync_file *sync_file,
-			       struct dma_fence **fences, int num_fences)
-{
-	struct dma_fence_array *array;
-
-	/*
-	 * The reference for the fences in the new sync_file and held
-	 * in add_fence() during the merge procedure, so for num_fences == 1
-	 * we already own a new reference to the fence. For num_fence > 1
-	 * we own the reference of the dma_fence_array creation.
-	 */
-
-	if (num_fences == 0) {
-		sync_file->fence = dma_fence_get_stub();
-		kfree(fences);
-
-	} else if (num_fences == 1) {
-		sync_file->fence = fences[0];
-		kfree(fences);
-
-	} else {
-		array = dma_fence_array_create(num_fences, fences,
-					       dma_fence_context_alloc(1),
-					       1, false);
-		if (!array)
-			return -ENOMEM;
-
-		sync_file->fence = &array->base;
-	}
-
-	return 0;
-}
-
-static void add_fence(struct dma_fence **fences,
-		      int *i, struct dma_fence *fence)
-{
-	fences[*i] = fence;
-
-	if (!dma_fence_is_signaled(fence)) {
-		dma_fence_get(fence);
-		(*i)++;
-	}
-}
-
 /**
  * sync_file_merge() - merge two sync_files
  * @name:	name of new fence
@@ -203,84 +159,21 @@ static void add_fence(struct dma_fence **fences,
 static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
 					 struct sync_file *b)
 {
-	struct dma_fence *a_fence, *b_fence, **fences;
-	struct dma_fence_unwrap a_iter, b_iter;
-	unsigned int index, num_fences;
 	struct sync_file *sync_file;
+	struct dma_fence *fence;
 
 	sync_file = sync_file_alloc();
 	if (!sync_file)
 		return NULL;
 
-	num_fences = 0;
-	dma_fence_unwrap_for_each(a_fence, &a_iter, a->fence)
-		++num_fences;
-	dma_fence_unwrap_for_each(b_fence, &b_iter, b->fence)
-		++num_fences;
-
-	if (num_fences > INT_MAX)
-		goto err_free_sync_file;
-
-	fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
-	if (!fences)
-		goto err_free_sync_file;
-
-	/*
-	 * We can't guarantee that fences in both a and b are ordered, but it is
-	 * still quite likely.
-	 *
-	 * So attempt to order the fences as we pass over them and merge fences
-	 * with the same context.
-	 */
-
-	index = 0;
-	for (a_fence = dma_fence_unwrap_first(a->fence, &a_iter),
-	     b_fence = dma_fence_unwrap_first(b->fence, &b_iter);
-	     a_fence || b_fence; ) {
-
-		if (!b_fence) {
-			add_fence(fences, &index, a_fence);
-			a_fence = dma_fence_unwrap_next(&a_iter);
-
-		} else if (!a_fence) {
-			add_fence(fences, &index, b_fence);
-			b_fence = dma_fence_unwrap_next(&b_iter);
-
-		} else if (a_fence->context < b_fence->context) {
-			add_fence(fences, &index, a_fence);
-			a_fence = dma_fence_unwrap_next(&a_iter);
-
-		} else if (b_fence->context < a_fence->context) {
-			add_fence(fences, &index, b_fence);
-			b_fence = dma_fence_unwrap_next(&b_iter);
-
-		} else if (__dma_fence_is_later(a_fence->seqno, b_fence->seqno,
-						a_fence->ops)) {
-			add_fence(fences, &index, a_fence);
-			a_fence = dma_fence_unwrap_next(&a_iter);
-			b_fence = dma_fence_unwrap_next(&b_iter);
-
-		} else {
-			add_fence(fences, &index, b_fence);
-			a_fence = dma_fence_unwrap_next(&a_iter);
-			b_fence = dma_fence_unwrap_next(&b_iter);
-		}
+	fence = dma_fence_unwrap_merge(a->fence, b->fence);
+	if (!fence) {
+		fput(sync_file->file);
+		return NULL;
 	}
-
-	if (sync_file_set_fence(sync_file, fences, index) < 0)
-		goto err_put_fences;
-
+	sync_file->fence = fence;
 	strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name));
 	return sync_file;
-
-err_put_fences:
-	while (index)
-		dma_fence_put(fences[--index]);
-	kfree(fences);
-
-err_free_sync_file:
-	fput(sync_file->file);
-	return NULL;
 }
 
 static int sync_file_release(struct inode *inode, struct file *file)
diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
index e9d114637294..43c8a9bbee88 100644
--- a/include/linux/dma-fence-unwrap.h
+++ b/include/linux/dma-fence-unwrap.h
@@ -48,4 +48,28 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor);
 	for (fence = dma_fence_unwrap_first(head, cursor); fence;	\
 	     fence = dma_fence_unwrap_next(cursor))
 
+struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
+					   struct dma_fence **fences,
+					   struct dma_fence_unwrap *cursors);
+
+/**
+ * dma_fence_unwrap_merge - unwrap and merge fences
+ *
+ * All fences given as parameters are unwrapped and merged back together as flat
+ * dma_fence_array. Useful if multiple containers need to be merged together.
+ *
+ * Implemented as a macro to allocate the necessary arrays on the stack and
+ * account the stack frame size to the caller.
+ *
+ * Returns NULL on memory allocation failure, a dma_fence object representing
+ * all the given fences otherwise.
+ */
+#define dma_fence_unwrap_merge(...)					\
+	({								\
+		struct dma_fence *__f[] = { __VA_ARGS__ };		\
+		struct dma_fence_unwrap __c[ARRAY_SIZE(__f)];		\
+									\
+		__dma_fence_unwrap_merge(ARRAY_SIZE(__f), __f, __c);	\
+	})
+
 #endif
-- 
2.25.1