[01/10] dma-buf: add new dma_fence_chain container v4

Submitted by Zhou, David(ChunMing) on Dec. 11, 2018, 10:34 a.m.

Details

Message ID 20181211103449.25899-1-david1.zhou@amd.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in AMD X.Org drivers

Browsing this patch as part of:
"Series without cover letter" rev 1 in AMD X.Org drivers
<< prev patch [1/10] next patch >>

Commit Message

Zhou, David(ChunMing) Dec. 11, 2018, 10:34 a.m.
From: Christian König <ckoenig.leichtzumerken@gmail.com>

Lockless container implementation similar to a dma_fence_array, but with
only two elements per node and automatic garbage collection.

v2: properly document dma_fence_chain_for_each, add dma_fence_chain_find_seqno,
    drop prev reference during garbage collection if it's not a chain fence.
v3: use head and iterator for dma_fence_chain_for_each
v4: fix reference count in dma_fence_chain_enable_signaling

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/Makefile          |   3 +-
 drivers/dma-buf/dma-fence-chain.c | 241 ++++++++++++++++++++++++++++++
 include/linux/dma-fence-chain.h   |  81 ++++++++++
 3 files changed, 324 insertions(+), 1 deletion(-)
 create mode 100644 drivers/dma-buf/dma-fence-chain.c
 create mode 100644 include/linux/dma-fence-chain.h

Patch hide | download patch | download mbox

diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index 0913a6ccab5a..1f006e083eb9 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -1,4 +1,5 @@ 
-obj-y := dma-buf.o dma-fence.o dma-fence-array.o reservation.o seqno-fence.o
+obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
+	 reservation.o seqno-fence.o
 obj-$(CONFIG_SYNC_FILE)		+= sync_file.o
 obj-$(CONFIG_SW_SYNC)		+= sw_sync.o sync_debug.o
 obj-$(CONFIG_UDMABUF)		+= udmabuf.o
diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
new file mode 100644
index 000000000000..0c5e3c902fa0
--- /dev/null
+++ b/drivers/dma-buf/dma-fence-chain.c
@@ -0,0 +1,241 @@ 
+/*
+ * fence-chain: chain fences together in a timeline
+ *
+ * Copyright (C) 2018 Advanced Micro Devices, Inc.
+ * Authors:
+ *	Christian König <christian.koenig@amd.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/dma-fence-chain.h>
+
+static bool dma_fence_chain_enable_signaling(struct dma_fence *fence);
+
+/**
+ * dma_fence_chain_get_prev - use RCU to get a reference to the previous fence
+ * @chain: chain node to get the previous node from
+ *
+ * Use dma_fence_get_rcu_safe to get a reference to the previous fence of the
+ * chain node.
+ */
+static struct dma_fence *dma_fence_chain_get_prev(struct dma_fence_chain *chain)
+{
+	struct dma_fence *prev;
+
+	rcu_read_lock();
+	prev = dma_fence_get_rcu_safe(&chain->prev);
+	rcu_read_unlock();
+	return prev;
+}
+
+/**
+ * dma_fence_chain_walk - chain walking function
+ * @fence: current chain node
+ *
+ * Walk the chain to the next node. Returns the next fence or NULL if we are at
+ * the end of the chain. Garbage collects chain nodes which are already
+ * signaled.
+ */
+struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence)
+{
+	struct dma_fence_chain *chain, *prev_chain;
+	struct dma_fence *prev, *replacement, *tmp;
+
+	chain = to_dma_fence_chain(fence);
+	if (!chain) {
+		dma_fence_put(fence);
+		return NULL;
+	}
+
+	while ((prev = dma_fence_chain_get_prev(chain))) {
+
+		prev_chain = to_dma_fence_chain(prev);
+		if (prev_chain) {
+			if (!dma_fence_is_signaled(prev_chain->fence))
+				break;
+
+			replacement = dma_fence_chain_get_prev(prev_chain);
+		} else {
+			if (!dma_fence_is_signaled(prev))
+				break;
+
+			replacement = NULL;
+		}
+
+		tmp = cmpxchg(&chain->prev, prev, replacement);
+		if (tmp == prev)
+			dma_fence_put(tmp);
+		else
+			dma_fence_put(replacement);
+		dma_fence_put(prev);
+	}
+
+	dma_fence_put(fence);
+	return prev;
+}
+EXPORT_SYMBOL(dma_fence_chain_walk);
+
+/**
+ * dma_fence_chain_find_seqno - find fence chain node by seqno
+ * @pfence: pointer to the chain node where to start
+ * @seqno: the sequence number to search for
+ *
+ * Advance the fence pointer to the chain node which will signal this sequence
+ * number. If no sequence number is provided then this is a no-op.
+ *
+ * Returns EINVAL if the fence is not a chain node or the sequence number has
+ * not yet advanced far enough.
+ */
+int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno)
+{
+	struct dma_fence_chain *chain;
+
+	if (!seqno)
+		return 0;
+
+	chain = to_dma_fence_chain(*pfence);
+	if (!chain || chain->base.seqno < seqno)
+		return -EINVAL;
+
+	dma_fence_chain_for_each(*pfence, &chain->base) {
+		if ((*pfence)->context != chain->base.context ||
+		    to_dma_fence_chain(*pfence)->prev_seqno < seqno)
+			break;
+	}
+	dma_fence_put(&chain->base);
+
+	return 0;
+}
+EXPORT_SYMBOL(dma_fence_chain_find_seqno);
+
+static const char *dma_fence_chain_get_driver_name(struct dma_fence *fence)
+{
+        return "dma_fence_chain";
+}
+
+static const char *dma_fence_chain_get_timeline_name(struct dma_fence *fence)
+{
+        return "unbound";
+}
+
+static void dma_fence_chain_irq_work(struct irq_work *work)
+{
+	struct dma_fence_chain *chain;
+
+	chain = container_of(work, typeof(*chain), work);
+
+	/* Try to rearm the callback */
+	if (!dma_fence_chain_enable_signaling(&chain->base))
+		/* Ok, we are done. No more unsignaled fences left */
+		dma_fence_signal(&chain->base);
+	dma_fence_put(&chain->base);
+}
+
+static void dma_fence_chain_cb(struct dma_fence *f, struct dma_fence_cb *cb)
+{
+	struct dma_fence_chain *chain;
+
+	chain = container_of(cb, typeof(*chain), cb);
+	irq_work_queue(&chain->work);
+	dma_fence_put(f);
+}
+
+static bool dma_fence_chain_enable_signaling(struct dma_fence *fence)
+{
+	struct dma_fence_chain *head = to_dma_fence_chain(fence);
+
+	dma_fence_get(&head->base);
+	dma_fence_chain_for_each(fence, &head->base) {
+		struct dma_fence_chain *chain = to_dma_fence_chain(fence);
+		struct dma_fence *f = chain ? chain->fence : fence;
+
+		dma_fence_get(f);
+		if (!dma_fence_add_callback(f, &head->cb, dma_fence_chain_cb)) {
+			dma_fence_put(fence);
+			return true;
+		}
+		dma_fence_put(f);
+	}
+	dma_fence_put(&head->base);
+	return false;
+}
+
+static bool dma_fence_chain_signaled(struct dma_fence *fence)
+{
+	dma_fence_chain_for_each(fence, fence) {
+		struct dma_fence_chain *chain = to_dma_fence_chain(fence);
+		struct dma_fence *f = chain ? chain->fence : fence;
+
+		if (!dma_fence_is_signaled(f)) {
+			dma_fence_put(fence);
+			return false;
+		}
+	}
+
+	return true;
+}
+
+static void dma_fence_chain_release(struct dma_fence *fence)
+{
+	struct dma_fence_chain *chain = to_dma_fence_chain(fence);
+
+	dma_fence_put(chain->prev);
+	dma_fence_put(chain->fence);
+	dma_fence_free(fence);
+}
+
+const struct dma_fence_ops dma_fence_chain_ops = {
+	.get_driver_name = dma_fence_chain_get_driver_name,
+	.get_timeline_name = dma_fence_chain_get_timeline_name,
+	.enable_signaling = dma_fence_chain_enable_signaling,
+	.signaled = dma_fence_chain_signaled,
+	.release = dma_fence_chain_release,
+};
+EXPORT_SYMBOL(dma_fence_chain_ops);
+
+/**
+ * dma_fence_chain_init - initialize a fence chain
+ * @chain: the chain node to initialize
+ * @prev: the previous fence
+ * @fence: the current fence
+ *
+ * Initialize a new chain node and either start a new chain or add the node to
+ * the existing chain of the previous fence.
+ */
+void dma_fence_chain_init(struct dma_fence_chain *chain,
+			  struct dma_fence *prev,
+			  struct dma_fence *fence,
+			  uint64_t seqno)
+{
+	struct dma_fence_chain *prev_chain = to_dma_fence_chain(prev);
+	uint64_t context;
+
+	spin_lock_init(&chain->lock);
+	chain->prev = prev;
+	chain->fence = fence;
+	chain->prev_seqno = 0;
+	init_irq_work(&chain->work, dma_fence_chain_irq_work);
+
+	/* Try to reuse the context of the previous chain node. */
+	if (prev_chain && __dma_fence_is_later(seqno, prev->seqno)) {
+		context = prev->context;
+		chain->prev_seqno = prev->seqno;
+	} else {
+		context = dma_fence_context_alloc(1);
+		/* Make sure that we always have a valid sequence number. */
+		if (prev_chain)
+			seqno = max(prev->seqno, seqno);
+	}
+
+	dma_fence_init(&chain->base, &dma_fence_chain_ops,
+		       &chain->lock, context, seqno);
+}
+EXPORT_SYMBOL(dma_fence_chain_init);
diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h
new file mode 100644
index 000000000000..a5c2e8c6915c
--- /dev/null
+++ b/include/linux/dma-fence-chain.h
@@ -0,0 +1,81 @@ 
+/*
+ * fence-chain: chain fences together in a timeline
+ *
+ * Copyright (C) 2018 Advanced Micro Devices, Inc.
+ * Authors:
+ *	Christian König <christian.koenig@amd.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#ifndef __LINUX_DMA_FENCE_CHAIN_H
+#define __LINUX_DMA_FENCE_CHAIN_H
+
+#include <linux/dma-fence.h>
+#include <linux/irq_work.h>
+
+/**
+ * struct dma_fence_chain - fence to represent an node of a fence chain
+ * @base: fence base class
+ * @lock: spinlock for fence handling
+ * @prev: previous fence of the chain
+ * @prev_seqno: original previous seqno before garbage collection
+ * @fence: encapsulated fence
+ * @cb: callback structure for signaling
+ * @work: irq work item for signaling
+ */
+struct dma_fence_chain {
+	struct dma_fence base;
+	spinlock_t lock;
+	struct dma_fence *prev;
+	u64 prev_seqno;
+	struct dma_fence *fence;
+	struct dma_fence_cb cb;
+	struct irq_work work;
+};
+
+extern const struct dma_fence_ops dma_fence_chain_ops;
+
+/**
+ * to_dma_fence_chain - cast a fence to a dma_fence_chain
+ * @fence: fence to cast to a dma_fence_array
+ *
+ * Returns NULL if the fence is not a dma_fence_chain,
+ * or the dma_fence_chain otherwise.
+ */
+static inline struct dma_fence_chain *
+to_dma_fence_chain(struct dma_fence *fence)
+{
+	if (!fence || fence->ops != &dma_fence_chain_ops)
+		return NULL;
+
+	return container_of(fence, struct dma_fence_chain, base);
+}
+
+/**
+ * dma_fence_chain_for_each - iterate over all fences in chain
+ * @iter: current fence
+ * @head: starting point
+ *
+ * Iterate over all fences in the chain. We keep a reference to the current
+ * fence while inside the loop which must be dropped when breaking out.
+ */
+#define dma_fence_chain_for_each(iter, head)	\
+	for (iter = dma_fence_get(head); iter; \
+	     iter = dma_fence_chain_walk(head))
+
+struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence);
+int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno);
+void dma_fence_chain_init(struct dma_fence_chain *chain,
+			  struct dma_fence *prev,
+			  struct dma_fence *fence,
+			  uint64_t seqno);
+
+#endif /* __LINUX_DMA_FENCE_CHAIN_H */

Comments

Hi Daniel and Chris,

Could you take a look on all the patches? Can we get your RB or AB on all patches including igt patch before we submit to drm-misc? 

We already fix all existing issues, and also add  test case in IGT as your required.

Btw, the patch set is tested by below tests:
a. vulkan cts  " ./deqp-vk -n dEQP-VK. *semaphore*" 
b. internal vulkan timeline test
c. libdrm test "sudo ./amdgpu_test -s 9"
d. IGT test, "sudo ./syncobj_basic"
e. IGT test, "sudo ./syncobj_wait"
f. IGT test, "sudo ./syncobj_timeline"

Any other suggestion or requirement is welcome.

-David

> -----Original Message-----

> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of

> Chunming Zhou

> Sent: Tuesday, December 11, 2018 6:35 PM

> To: Koenig, Christian <Christian.Koenig@amd.com>; dri-

> devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org; intel-

> gfx@lists.freedesktop.org

> Cc: Christian König <ckoenig.leichtzumerken@gmail.com>; Koenig, Christian

> <Christian.Koenig@amd.com>

> Subject: [PATCH 01/10] dma-buf: add new dma_fence_chain container v4

> 

> From: Christian König <ckoenig.leichtzumerken@gmail.com>

> 

> Lockless container implementation similar to a dma_fence_array, but with

> only two elements per node and automatic garbage collection.

> 

> v2: properly document dma_fence_chain_for_each, add

> dma_fence_chain_find_seqno,

>     drop prev reference during garbage collection if it's not a chain fence.

> v3: use head and iterator for dma_fence_chain_for_each

> v4: fix reference count in dma_fence_chain_enable_signaling

> 

> Signed-off-by: Christian König <christian.koenig@amd.com>

> ---

>  drivers/dma-buf/Makefile          |   3 +-

>  drivers/dma-buf/dma-fence-chain.c | 241

> ++++++++++++++++++++++++++++++

>  include/linux/dma-fence-chain.h   |  81 ++++++++++

>  3 files changed, 324 insertions(+), 1 deletion(-)  create mode 100644

> drivers/dma-buf/dma-fence-chain.c  create mode 100644 include/linux/dma-

> fence-chain.h

> 

> diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index

> 0913a6ccab5a..1f006e083eb9 100644

> --- a/drivers/dma-buf/Makefile

> +++ b/drivers/dma-buf/Makefile

> @@ -1,4 +1,5 @@

> -obj-y := dma-buf.o dma-fence.o dma-fence-array.o reservation.o seqno-

> fence.o

> +obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \

> +	 reservation.o seqno-fence.o

>  obj-$(CONFIG_SYNC_FILE)		+= sync_file.o

>  obj-$(CONFIG_SW_SYNC)		+= sw_sync.o sync_debug.o

>  obj-$(CONFIG_UDMABUF)		+= udmabuf.o

> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-

> fence-chain.c

> new file mode 100644

> index 000000000000..0c5e3c902fa0

> --- /dev/null

> +++ b/drivers/dma-buf/dma-fence-chain.c

> @@ -0,0 +1,241 @@

> +/*

> + * fence-chain: chain fences together in a timeline

> + *

> + * Copyright (C) 2018 Advanced Micro Devices, Inc.

> + * Authors:

> + *	Christian König <christian.koenig@amd.com>

> + *

> + * This program is free software; you can redistribute it and/or modify

> +it

> + * under the terms of the GNU General Public License version 2 as

> +published by

> + * the Free Software Foundation.

> + *

> + * This program is distributed in the hope that it will be useful, but

> +WITHOUT

> + * ANY WARRANTY; without even the implied warranty of

> MERCHANTABILITY

> +or

> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public

> +License for

> + * more details.

> + */

> +

> +#include <linux/dma-fence-chain.h>

> +

> +static bool dma_fence_chain_enable_signaling(struct dma_fence *fence);

> +

> +/**

> + * dma_fence_chain_get_prev - use RCU to get a reference to the

> +previous fence

> + * @chain: chain node to get the previous node from

> + *

> + * Use dma_fence_get_rcu_safe to get a reference to the previous fence

> +of the

> + * chain node.

> + */

> +static struct dma_fence *dma_fence_chain_get_prev(struct

> +dma_fence_chain *chain) {

> +	struct dma_fence *prev;

> +

> +	rcu_read_lock();

> +	prev = dma_fence_get_rcu_safe(&chain->prev);

> +	rcu_read_unlock();

> +	return prev;

> +}

> +

> +/**

> + * dma_fence_chain_walk - chain walking function

> + * @fence: current chain node

> + *

> + * Walk the chain to the next node. Returns the next fence or NULL if

> +we are at

> + * the end of the chain. Garbage collects chain nodes which are already

> + * signaled.

> + */

> +struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence) {

> +	struct dma_fence_chain *chain, *prev_chain;

> +	struct dma_fence *prev, *replacement, *tmp;

> +

> +	chain = to_dma_fence_chain(fence);

> +	if (!chain) {

> +		dma_fence_put(fence);

> +		return NULL;

> +	}

> +

> +	while ((prev = dma_fence_chain_get_prev(chain))) {

> +

> +		prev_chain = to_dma_fence_chain(prev);

> +		if (prev_chain) {

> +			if (!dma_fence_is_signaled(prev_chain->fence))

> +				break;

> +

> +			replacement =

> dma_fence_chain_get_prev(prev_chain);

> +		} else {

> +			if (!dma_fence_is_signaled(prev))

> +				break;

> +

> +			replacement = NULL;

> +		}

> +

> +		tmp = cmpxchg(&chain->prev, prev, replacement);

> +		if (tmp == prev)

> +			dma_fence_put(tmp);

> +		else

> +			dma_fence_put(replacement);

> +		dma_fence_put(prev);

> +	}

> +

> +	dma_fence_put(fence);

> +	return prev;

> +}

> +EXPORT_SYMBOL(dma_fence_chain_walk);

> +

> +/**

> + * dma_fence_chain_find_seqno - find fence chain node by seqno

> + * @pfence: pointer to the chain node where to start

> + * @seqno: the sequence number to search for

> + *

> + * Advance the fence pointer to the chain node which will signal this

> +sequence

> + * number. If no sequence number is provided then this is a no-op.

> + *

> + * Returns EINVAL if the fence is not a chain node or the sequence

> +number has

> + * not yet advanced far enough.

> + */

> +int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t

> +seqno) {

> +	struct dma_fence_chain *chain;

> +

> +	if (!seqno)

> +		return 0;

> +

> +	chain = to_dma_fence_chain(*pfence);

> +	if (!chain || chain->base.seqno < seqno)

> +		return -EINVAL;

> +

> +	dma_fence_chain_for_each(*pfence, &chain->base) {

> +		if ((*pfence)->context != chain->base.context ||

> +		    to_dma_fence_chain(*pfence)->prev_seqno < seqno)

> +			break;

> +	}

> +	dma_fence_put(&chain->base);

> +

> +	return 0;

> +}

> +EXPORT_SYMBOL(dma_fence_chain_find_seqno);

> +

> +static const char *dma_fence_chain_get_driver_name(struct dma_fence

> +*fence) {

> +        return "dma_fence_chain";

> +}

> +

> +static const char *dma_fence_chain_get_timeline_name(struct dma_fence

> +*fence) {

> +        return "unbound";

> +}

> +

> +static void dma_fence_chain_irq_work(struct irq_work *work) {

> +	struct dma_fence_chain *chain;

> +

> +	chain = container_of(work, typeof(*chain), work);

> +

> +	/* Try to rearm the callback */

> +	if (!dma_fence_chain_enable_signaling(&chain->base))

> +		/* Ok, we are done. No more unsignaled fences left */

> +		dma_fence_signal(&chain->base);

> +	dma_fence_put(&chain->base);

> +}

> +

> +static void dma_fence_chain_cb(struct dma_fence *f, struct dma_fence_cb

> +*cb) {

> +	struct dma_fence_chain *chain;

> +

> +	chain = container_of(cb, typeof(*chain), cb);

> +	irq_work_queue(&chain->work);

> +	dma_fence_put(f);

> +}

> +

> +static bool dma_fence_chain_enable_signaling(struct dma_fence *fence) {

> +	struct dma_fence_chain *head = to_dma_fence_chain(fence);

> +

> +	dma_fence_get(&head->base);

> +	dma_fence_chain_for_each(fence, &head->base) {

> +		struct dma_fence_chain *chain =

> to_dma_fence_chain(fence);

> +		struct dma_fence *f = chain ? chain->fence : fence;

> +

> +		dma_fence_get(f);

> +		if (!dma_fence_add_callback(f, &head->cb,

> dma_fence_chain_cb)) {

> +			dma_fence_put(fence);

> +			return true;

> +		}

> +		dma_fence_put(f);

> +	}

> +	dma_fence_put(&head->base);

> +	return false;

> +}

> +

> +static bool dma_fence_chain_signaled(struct dma_fence *fence) {

> +	dma_fence_chain_for_each(fence, fence) {

> +		struct dma_fence_chain *chain =

> to_dma_fence_chain(fence);

> +		struct dma_fence *f = chain ? chain->fence : fence;

> +

> +		if (!dma_fence_is_signaled(f)) {

> +			dma_fence_put(fence);

> +			return false;

> +		}

> +	}

> +

> +	return true;

> +}

> +

> +static void dma_fence_chain_release(struct dma_fence *fence) {

> +	struct dma_fence_chain *chain = to_dma_fence_chain(fence);

> +

> +	dma_fence_put(chain->prev);

> +	dma_fence_put(chain->fence);

> +	dma_fence_free(fence);

> +}

> +

> +const struct dma_fence_ops dma_fence_chain_ops = {

> +	.get_driver_name = dma_fence_chain_get_driver_name,

> +	.get_timeline_name = dma_fence_chain_get_timeline_name,

> +	.enable_signaling = dma_fence_chain_enable_signaling,

> +	.signaled = dma_fence_chain_signaled,

> +	.release = dma_fence_chain_release,

> +};

> +EXPORT_SYMBOL(dma_fence_chain_ops);

> +

> +/**

> + * dma_fence_chain_init - initialize a fence chain

> + * @chain: the chain node to initialize

> + * @prev: the previous fence

> + * @fence: the current fence

> + *

> + * Initialize a new chain node and either start a new chain or add the

> +node to

> + * the existing chain of the previous fence.

> + */

> +void dma_fence_chain_init(struct dma_fence_chain *chain,

> +			  struct dma_fence *prev,

> +			  struct dma_fence *fence,

> +			  uint64_t seqno)

> +{

> +	struct dma_fence_chain *prev_chain = to_dma_fence_chain(prev);

> +	uint64_t context;

> +

> +	spin_lock_init(&chain->lock);

> +	chain->prev = prev;

> +	chain->fence = fence;

> +	chain->prev_seqno = 0;

> +	init_irq_work(&chain->work, dma_fence_chain_irq_work);

> +

> +	/* Try to reuse the context of the previous chain node. */

> +	if (prev_chain && __dma_fence_is_later(seqno, prev->seqno)) {

> +		context = prev->context;

> +		chain->prev_seqno = prev->seqno;

> +	} else {

> +		context = dma_fence_context_alloc(1);

> +		/* Make sure that we always have a valid sequence number.

> */

> +		if (prev_chain)

> +			seqno = max(prev->seqno, seqno);

> +	}

> +

> +	dma_fence_init(&chain->base, &dma_fence_chain_ops,

> +		       &chain->lock, context, seqno); }

> +EXPORT_SYMBOL(dma_fence_chain_init);

> diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-

> chain.h new file mode 100644 index 000000000000..a5c2e8c6915c

> --- /dev/null

> +++ b/include/linux/dma-fence-chain.h

> @@ -0,0 +1,81 @@

> +/*

> + * fence-chain: chain fences together in a timeline

> + *

> + * Copyright (C) 2018 Advanced Micro Devices, Inc.

> + * Authors:

> + *	Christian König <christian.koenig@amd.com>

> + *

> + * This program is free software; you can redistribute it and/or modify

> +it

> + * under the terms of the GNU General Public License version 2 as

> +published by

> + * the Free Software Foundation.

> + *

> + * This program is distributed in the hope that it will be useful, but

> +WITHOUT

> + * ANY WARRANTY; without even the implied warranty of

> MERCHANTABILITY

> +or

> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public

> +License for

> + * more details.

> + */

> +

> +#ifndef __LINUX_DMA_FENCE_CHAIN_H

> +#define __LINUX_DMA_FENCE_CHAIN_H

> +

> +#include <linux/dma-fence.h>

> +#include <linux/irq_work.h>

> +

> +/**

> + * struct dma_fence_chain - fence to represent an node of a fence chain

> + * @base: fence base class

> + * @lock: spinlock for fence handling

> + * @prev: previous fence of the chain

> + * @prev_seqno: original previous seqno before garbage collection

> + * @fence: encapsulated fence

> + * @cb: callback structure for signaling

> + * @work: irq work item for signaling

> + */

> +struct dma_fence_chain {

> +	struct dma_fence base;

> +	spinlock_t lock;

> +	struct dma_fence *prev;

> +	u64 prev_seqno;

> +	struct dma_fence *fence;

> +	struct dma_fence_cb cb;

> +	struct irq_work work;

> +};

> +

> +extern const struct dma_fence_ops dma_fence_chain_ops;

> +

> +/**

> + * to_dma_fence_chain - cast a fence to a dma_fence_chain

> + * @fence: fence to cast to a dma_fence_array

> + *

> + * Returns NULL if the fence is not a dma_fence_chain,

> + * or the dma_fence_chain otherwise.

> + */

> +static inline struct dma_fence_chain *

> +to_dma_fence_chain(struct dma_fence *fence) {

> +	if (!fence || fence->ops != &dma_fence_chain_ops)

> +		return NULL;

> +

> +	return container_of(fence, struct dma_fence_chain, base); }

> +

> +/**

> + * dma_fence_chain_for_each - iterate over all fences in chain

> + * @iter: current fence

> + * @head: starting point

> + *

> + * Iterate over all fences in the chain. We keep a reference to the

> +current

> + * fence while inside the loop which must be dropped when breaking out.

> + */

> +#define dma_fence_chain_for_each(iter, head)	\

> +	for (iter = dma_fence_get(head); iter; \

> +	     iter = dma_fence_chain_walk(head))

> +

> +struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence); int

> +dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t

> seqno);

> +void dma_fence_chain_init(struct dma_fence_chain *chain,

> +			  struct dma_fence *prev,

> +			  struct dma_fence *fence,

> +			  uint64_t seqno);

> +

> +#endif /* __LINUX_DMA_FENCE_CHAIN_H */

> --

> 2.17.1

> 

> _______________________________________________

> dri-devel mailing list

> dri-devel@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoting Chunming Zhou (2018-12-11 10:34:40)
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> 
> Lockless container implementation similar to a dma_fence_array, but with
> only two elements per node and automatic garbage collection.
> 
> v2: properly document dma_fence_chain_for_each, add dma_fence_chain_find_seqno,
>     drop prev reference during garbage collection if it's not a chain fence.
> v3: use head and iterator for dma_fence_chain_for_each
> v4: fix reference count in dma_fence_chain_enable_signaling
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/dma-buf/Makefile          |   3 +-
>  drivers/dma-buf/dma-fence-chain.c | 241 ++++++++++++++++++++++++++++++
>  include/linux/dma-fence-chain.h   |  81 ++++++++++
>  3 files changed, 324 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/dma-buf/dma-fence-chain.c
>  create mode 100644 include/linux/dma-fence-chain.h
> 
> diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
> index 0913a6ccab5a..1f006e083eb9 100644
> --- a/drivers/dma-buf/Makefile
> +++ b/drivers/dma-buf/Makefile
> @@ -1,4 +1,5 @@
> -obj-y := dma-buf.o dma-fence.o dma-fence-array.o reservation.o seqno-fence.o
> +obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
> +        reservation.o seqno-fence.o
>  obj-$(CONFIG_SYNC_FILE)                += sync_file.o
>  obj-$(CONFIG_SW_SYNC)          += sw_sync.o sync_debug.o
>  obj-$(CONFIG_UDMABUF)          += udmabuf.o
> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
> new file mode 100644
> index 000000000000..0c5e3c902fa0
> --- /dev/null
> +++ b/drivers/dma-buf/dma-fence-chain.c
> @@ -0,0 +1,241 @@
> +/*

SPDX-License-Identifier: GPL-2.0

> + * fence-chain: chain fences together in a timeline
> + *
> + * Copyright (C) 2018 Advanced Micro Devices, Inc.
> + * Authors:
> + *     Christian König <christian.koenig@amd.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/dma-fence-chain.h>
> +
> +static bool dma_fence_chain_enable_signaling(struct dma_fence *fence);
> +
> +/**
> + * dma_fence_chain_get_prev - use RCU to get a reference to the previous fence
> + * @chain: chain node to get the previous node from
> + *
> + * Use dma_fence_get_rcu_safe to get a reference to the previous fence of the
> + * chain node.
> + */
> +static struct dma_fence *dma_fence_chain_get_prev(struct dma_fence_chain *chain)
> +{
> +       struct dma_fence *prev;
> +
> +       rcu_read_lock();
> +       prev = dma_fence_get_rcu_safe(&chain->prev);
> +       rcu_read_unlock();
> +       return prev;
> +}
> +
> +/**
> + * dma_fence_chain_walk - chain walking function
> + * @fence: current chain node
> + *
> + * Walk the chain to the next node. Returns the next fence or NULL if we are at
> + * the end of the chain. Garbage collects chain nodes which are already
> + * signaled.
> + */
> +struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence)
> +{
> +       struct dma_fence_chain *chain, *prev_chain;
> +       struct dma_fence *prev, *replacement, *tmp;
> +
> +       chain = to_dma_fence_chain(fence);
> +       if (!chain) {
> +               dma_fence_put(fence);
> +               return NULL;
> +       }
> +
> +       while ((prev = dma_fence_chain_get_prev(chain))) {
> +
> +               prev_chain = to_dma_fence_chain(prev);
> +               if (prev_chain) {
> +                       if (!dma_fence_is_signaled(prev_chain->fence))
> +                               break;
> +
> +                       replacement = dma_fence_chain_get_prev(prev_chain);
> +               } else {
> +                       if (!dma_fence_is_signaled(prev))
> +                               break;
> +
> +                       replacement = NULL;
> +               }
> +
> +               tmp = cmpxchg(&chain->prev, prev, replacement);
> +               if (tmp == prev)
> +                       dma_fence_put(tmp);
> +               else
> +                       dma_fence_put(replacement);

if (cmpxchg(&chain->prev, prev, replacement) == prev)
	dma_fence_put(prev)
else
	dma_fence_put(replacement);

would be a little easier for me to read.

> +               dma_fence_put(prev);
> +       }
> +
> +       dma_fence_put(fence);

Putting the caller's fence caught me by surprise.

I'd be tempted to do

> +
> +/**
> + * dma_fence_chain_for_each - iterate over all fences in chain
> + * @iter: current fence
> + * @head: starting point
> + *
> + * Iterate over all fences in the chain. We keep a reference to the current
> + * fence while inside the loop which must be dropped when breaking out.
> + */
> +#define dma_fence_chain_for_each(iter, head)   \
> +       for (iter = dma_fence_get(head); iter; \
> +            iter = dma_fence_chain_walk(head))

               dma_fence_chain_walk(head) <-- should be iter?

#define dma_fence_chain_for_each(iter, head)   \
       for (iter = dma_fence_get(head); iter; \
            ({ dma_fence_put(iter); iter = dma_fence_chain_walk(iter)))

just so that dma_fence_chain_walk() didn't look unbalanced and the
put/get more clearly inside the for_each.

> +       return prev;
> +}
> +EXPORT_SYMBOL(dma_fence_chain_walk);

> +
> +/**
> + * dma_fence_chain_find_seqno - find fence chain node by seqno
> + * @pfence: pointer to the chain node where to start
> + * @seqno: the sequence number to search for
> + *
> + * Advance the fence pointer to the chain node which will signal this sequence
> + * number. If no sequence number is provided then this is a no-op.
> + *
> + * Returns EINVAL if the fence is not a chain node or the sequence number has
> + * not yet advanced far enough.
> + */
> +int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno)

That inout parameter took me by surprise. "find" I expected to return
the fence.

Not used in this patch, so I need to read on to see if its use
simplifies the code or is equally surprising ;)
-Chris