[1/9] dma-buf: add new dma_fence_chain container v6

Submitted by Chunming Zhou on March 20, 2019, 5:47 a.m.

Details

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

Not browsing as part of any series.

Commit Message

Chunming Zhou March 20, 2019, 5:47 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
v5: fix iteration when walking each chain node
v6: add __rcu for member 'prev' of struct chain node

Signed-off-by: Christian König <christian.koenig@amd.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.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..934a442db8ac
--- /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 __rcu *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(iter))
+
+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 Chunming,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.1-rc1 next-20190320]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chunming-Zhou/dma-buf-add-new-dma_fence_chain-container-v6/20190320-223607
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'


sparse warnings: (new ones prefixed by >>)

>> drivers/dma-buf/dma-fence-chain.c:73:23: sparse: incorrect type in initializer (different address spaces) @@    expected struct dma_fence [noderef] <asn:4>*__old @@    got  dma_fence [noderef] <asn:4>*__old @@
   drivers/dma-buf/dma-fence-chain.c:73:23:    expected struct dma_fence [noderef] <asn:4>*__old
   drivers/dma-buf/dma-fence-chain.c:73:23:    got struct dma_fence *[assigned] prev
>> drivers/dma-buf/dma-fence-chain.c:73:23: sparse: incorrect type in initializer (different address spaces) @@    expected struct dma_fence [noderef] <asn:4>*__new @@    got  dma_fence [noderef] <asn:4>*__new @@
   drivers/dma-buf/dma-fence-chain.c:73:23:    expected struct dma_fence [noderef] <asn:4>*__new
   drivers/dma-buf/dma-fence-chain.c:73:23:    got struct dma_fence *[assigned] replacement
>> drivers/dma-buf/dma-fence-chain.c:73:21: sparse: incorrect type in assignment (different address spaces) @@    expected struct dma_fence *tmp @@    got struct dma_fence [noderef] <struct dma_fence *tmp @@
   drivers/dma-buf/dma-fence-chain.c:73:21:    expected struct dma_fence *tmp
   drivers/dma-buf/dma-fence-chain.c:73:21:    got struct dma_fence [noderef] <asn:4>*[assigned] __ret
>> drivers/dma-buf/dma-fence-chain.c:190:28: sparse: incorrect type in argument 1 (different address spaces) @@    expected struct dma_fence *fence @@    got struct dma_fence struct dma_fence *fence @@
   drivers/dma-buf/dma-fence-chain.c:190:28:    expected struct dma_fence *fence
   drivers/dma-buf/dma-fence-chain.c:190:28:    got struct dma_fence [noderef] <asn:4>*prev
>> drivers/dma-buf/dma-fence-chain.c:222:21: sparse: incorrect type in assignment (different address spaces) @@    expected struct dma_fence [noderef] <asn:4>*prev @@    got [noderef] <asn:4>*prev @@
   drivers/dma-buf/dma-fence-chain.c:222:21:    expected struct dma_fence [noderef] <asn:4>*prev
   drivers/dma-buf/dma-fence-chain.c:222:21:    got struct dma_fence *prev
   drivers/dma-buf/dma-fence-chain.c:235:33: sparse: expression using sizeof(void)
   drivers/dma-buf/dma-fence-chain.c:235:33: sparse: expression using sizeof(void)

vim +73 drivers/dma-buf/dma-fence-chain.c

    38	
    39	/**
    40	 * dma_fence_chain_walk - chain walking function
    41	 * @fence: current chain node
    42	 *
    43	 * Walk the chain to the next node. Returns the next fence or NULL if we are at
    44	 * the end of the chain. Garbage collects chain nodes which are already
    45	 * signaled.
    46	 */
    47	struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence)
    48	{
    49		struct dma_fence_chain *chain, *prev_chain;
    50		struct dma_fence *prev, *replacement, *tmp;
    51	
    52		chain = to_dma_fence_chain(fence);
    53		if (!chain) {
    54			dma_fence_put(fence);
    55			return NULL;
    56		}
    57	
    58		while ((prev = dma_fence_chain_get_prev(chain))) {
    59	
    60			prev_chain = to_dma_fence_chain(prev);
    61			if (prev_chain) {
    62				if (!dma_fence_is_signaled(prev_chain->fence))
    63					break;
    64	
    65				replacement = dma_fence_chain_get_prev(prev_chain);
    66			} else {
    67				if (!dma_fence_is_signaled(prev))
    68					break;
    69	
    70				replacement = NULL;
    71			}
    72	
  > 73			tmp = cmpxchg(&chain->prev, prev, replacement);
    74			if (tmp == prev)
    75				dma_fence_put(tmp);
    76			else
    77				dma_fence_put(replacement);
    78			dma_fence_put(prev);
    79		}
    80	
    81		dma_fence_put(fence);
    82		return prev;
    83	}
    84	EXPORT_SYMBOL(dma_fence_chain_walk);
    85	
    86	/**
    87	 * dma_fence_chain_find_seqno - find fence chain node by seqno
    88	 * @pfence: pointer to the chain node where to start
    89	 * @seqno: the sequence number to search for
    90	 *
    91	 * Advance the fence pointer to the chain node which will signal this sequence
    92	 * number. If no sequence number is provided then this is a no-op.
    93	 *
    94	 * Returns EINVAL if the fence is not a chain node or the sequence number has
    95	 * not yet advanced far enough.
    96	 */
    97	int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno)
    98	{
    99		struct dma_fence_chain *chain;
   100	
   101		if (!seqno)
   102			return 0;
   103	
   104		chain = to_dma_fence_chain(*pfence);
   105		if (!chain || chain->base.seqno < seqno)
   106			return -EINVAL;
   107	
   108		dma_fence_chain_for_each(*pfence, &chain->base) {
   109			if ((*pfence)->context != chain->base.context ||
   110			    to_dma_fence_chain(*pfence)->prev_seqno < seqno)
   111				break;
   112		}
   113		dma_fence_put(&chain->base);
   114	
   115		return 0;
   116	}
   117	EXPORT_SYMBOL(dma_fence_chain_find_seqno);
   118	
   119	static const char *dma_fence_chain_get_driver_name(struct dma_fence *fence)
   120	{
   121	        return "dma_fence_chain";
   122	}
   123	
   124	static const char *dma_fence_chain_get_timeline_name(struct dma_fence *fence)
   125	{
   126	        return "unbound";
   127	}
   128	
   129	static void dma_fence_chain_irq_work(struct irq_work *work)
   130	{
   131		struct dma_fence_chain *chain;
   132	
   133		chain = container_of(work, typeof(*chain), work);
   134	
   135		/* Try to rearm the callback */
   136		if (!dma_fence_chain_enable_signaling(&chain->base))
   137			/* Ok, we are done. No more unsignaled fences left */
   138			dma_fence_signal(&chain->base);
   139		dma_fence_put(&chain->base);
   140	}
   141	
   142	static void dma_fence_chain_cb(struct dma_fence *f, struct dma_fence_cb *cb)
   143	{
   144		struct dma_fence_chain *chain;
   145	
   146		chain = container_of(cb, typeof(*chain), cb);
   147		irq_work_queue(&chain->work);
   148		dma_fence_put(f);
   149	}
   150	
   151	static bool dma_fence_chain_enable_signaling(struct dma_fence *fence)
   152	{
   153		struct dma_fence_chain *head = to_dma_fence_chain(fence);
   154	
   155		dma_fence_get(&head->base);
   156		dma_fence_chain_for_each(fence, &head->base) {
   157			struct dma_fence_chain *chain = to_dma_fence_chain(fence);
   158			struct dma_fence *f = chain ? chain->fence : fence;
   159	
   160			dma_fence_get(f);
   161			if (!dma_fence_add_callback(f, &head->cb, dma_fence_chain_cb)) {
   162				dma_fence_put(fence);
   163				return true;
   164			}
   165			dma_fence_put(f);
   166		}
   167		dma_fence_put(&head->base);
   168		return false;
   169	}
   170	
   171	static bool dma_fence_chain_signaled(struct dma_fence *fence)
   172	{
   173		dma_fence_chain_for_each(fence, fence) {
   174			struct dma_fence_chain *chain = to_dma_fence_chain(fence);
   175			struct dma_fence *f = chain ? chain->fence : fence;
   176	
   177			if (!dma_fence_is_signaled(f)) {
   178				dma_fence_put(fence);
   179				return false;
   180			}
   181		}
   182	
   183		return true;
   184	}
   185	
   186	static void dma_fence_chain_release(struct dma_fence *fence)
   187	{
   188		struct dma_fence_chain *chain = to_dma_fence_chain(fence);
   189	
 > 190		dma_fence_put(chain->prev);
   191		dma_fence_put(chain->fence);
   192		dma_fence_free(fence);
   193	}
   194	
   195	const struct dma_fence_ops dma_fence_chain_ops = {
   196		.get_driver_name = dma_fence_chain_get_driver_name,
   197		.get_timeline_name = dma_fence_chain_get_timeline_name,
   198		.enable_signaling = dma_fence_chain_enable_signaling,
   199		.signaled = dma_fence_chain_signaled,
   200		.release = dma_fence_chain_release,
   201	};
   202	EXPORT_SYMBOL(dma_fence_chain_ops);
   203	
   204	/**
   205	 * dma_fence_chain_init - initialize a fence chain
   206	 * @chain: the chain node to initialize
   207	 * @prev: the previous fence
   208	 * @fence: the current fence
   209	 *
   210	 * Initialize a new chain node and either start a new chain or add the node to
   211	 * the existing chain of the previous fence.
   212	 */
   213	void dma_fence_chain_init(struct dma_fence_chain *chain,
   214				  struct dma_fence *prev,
   215				  struct dma_fence *fence,
   216				  uint64_t seqno)
   217	{
   218		struct dma_fence_chain *prev_chain = to_dma_fence_chain(prev);
   219		uint64_t context;
   220	
   221		spin_lock_init(&chain->lock);
 > 222		chain->prev = prev;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Hi Lionel and Christian,

Below is robot report for chain->prev, which was added __rcu as you 
suggested.

How to fix this line "tmp = cmpxchg(&chain->prev, prev, replacement); "?
I checked kernel header file, seems it has no cmpxchg for rcu.

Any suggestion to fix this robot report?

Thanks,
-David

On 2019年03月21日 08:24, kbuild test robot wrote:
> Hi Chunming,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v5.1-rc1 next-20190320]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Chunming-Zhou/dma-buf-add-new-dma_fence_chain-container-v6/20190320-223607
> reproduce:
>          # apt-get install sparse
>          make ARCH=x86_64 allmodconfig
>          make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
>
>
> sparse warnings: (new ones prefixed by >>)
>
>>> drivers/dma-buf/dma-fence-chain.c:73:23: sparse: incorrect type in initializer (different address spaces) @@    expected struct dma_fence [noderef] <asn:4>*__old @@    got  dma_fence [noderef] <asn:4>*__old @@
>     drivers/dma-buf/dma-fence-chain.c:73:23:    expected struct dma_fence [noderef] <asn:4>*__old
>     drivers/dma-buf/dma-fence-chain.c:73:23:    got struct dma_fence *[assigned] prev
>>> drivers/dma-buf/dma-fence-chain.c:73:23: sparse: incorrect type in initializer (different address spaces) @@    expected struct dma_fence [noderef] <asn:4>*__new @@    got  dma_fence [noderef] <asn:4>*__new @@
>     drivers/dma-buf/dma-fence-chain.c:73:23:    expected struct dma_fence [noderef] <asn:4>*__new
>     drivers/dma-buf/dma-fence-chain.c:73:23:    got struct dma_fence *[assigned] replacement
>>> drivers/dma-buf/dma-fence-chain.c:73:21: sparse: incorrect type in assignment (different address spaces) @@    expected struct dma_fence *tmp @@    got struct dma_fence [noderef] <struct dma_fence *tmp @@
>     drivers/dma-buf/dma-fence-chain.c:73:21:    expected struct dma_fence *tmp
>     drivers/dma-buf/dma-fence-chain.c:73:21:    got struct dma_fence [noderef] <asn:4>*[assigned] __ret
>>> drivers/dma-buf/dma-fence-chain.c:190:28: sparse: incorrect type in argument 1 (different address spaces) @@    expected struct dma_fence *fence @@    got struct dma_fence struct dma_fence *fence @@
>     drivers/dma-buf/dma-fence-chain.c:190:28:    expected struct dma_fence *fence
>     drivers/dma-buf/dma-fence-chain.c:190:28:    got struct dma_fence [noderef] <asn:4>*prev
>>> drivers/dma-buf/dma-fence-chain.c:222:21: sparse: incorrect type in assignment (different address spaces) @@    expected struct dma_fence [noderef] <asn:4>*prev @@    got [noderef] <asn:4>*prev @@
>     drivers/dma-buf/dma-fence-chain.c:222:21:    expected struct dma_fence [noderef] <asn:4>*prev
>     drivers/dma-buf/dma-fence-chain.c:222:21:    got struct dma_fence *prev
>     drivers/dma-buf/dma-fence-chain.c:235:33: sparse: expression using sizeof(void)
>     drivers/dma-buf/dma-fence-chain.c:235:33: sparse: expression using sizeof(void)
>
> vim +73 drivers/dma-buf/dma-fence-chain.c
>
>      38	
>      39	/**
>      40	 * dma_fence_chain_walk - chain walking function
>      41	 * @fence: current chain node
>      42	 *
>      43	 * Walk the chain to the next node. Returns the next fence or NULL if we are at
>      44	 * the end of the chain. Garbage collects chain nodes which are already
>      45	 * signaled.
>      46	 */
>      47	struct dma_fence *dma_fence_chain_walk(struct dma_fence *fence)
>      48	{
>      49		struct dma_fence_chain *chain, *prev_chain;
>      50		struct dma_fence *prev, *replacement, *tmp;
>      51	
>      52		chain = to_dma_fence_chain(fence);
>      53		if (!chain) {
>      54			dma_fence_put(fence);
>      55			return NULL;
>      56		}
>      57	
>      58		while ((prev = dma_fence_chain_get_prev(chain))) {
>      59	
>      60			prev_chain = to_dma_fence_chain(prev);
>      61			if (prev_chain) {
>      62				if (!dma_fence_is_signaled(prev_chain->fence))
>      63					break;
>      64	
>      65				replacement = dma_fence_chain_get_prev(prev_chain);
>      66			} else {
>      67				if (!dma_fence_is_signaled(prev))
>      68					break;
>      69	
>      70				replacement = NULL;
>      71			}
>      72	
>    > 73			tmp = cmpxchg(&chain->prev, prev, replacement);
>      74			if (tmp == prev)
>      75				dma_fence_put(tmp);
>      76			else
>      77				dma_fence_put(replacement);
>      78			dma_fence_put(prev);
>      79		}
>      80	
>      81		dma_fence_put(fence);
>      82		return prev;
>      83	}
>      84	EXPORT_SYMBOL(dma_fence_chain_walk);
>      85	
>      86	/**
>      87	 * dma_fence_chain_find_seqno - find fence chain node by seqno
>      88	 * @pfence: pointer to the chain node where to start
>      89	 * @seqno: the sequence number to search for
>      90	 *
>      91	 * Advance the fence pointer to the chain node which will signal this sequence
>      92	 * number. If no sequence number is provided then this is a no-op.
>      93	 *
>      94	 * Returns EINVAL if the fence is not a chain node or the sequence number has
>      95	 * not yet advanced far enough.
>      96	 */
>      97	int dma_fence_chain_find_seqno(struct dma_fence **pfence, uint64_t seqno)
>      98	{
>      99		struct dma_fence_chain *chain;
>     100	
>     101		if (!seqno)
>     102			return 0;
>     103	
>     104		chain = to_dma_fence_chain(*pfence);
>     105		if (!chain || chain->base.seqno < seqno)
>     106			return -EINVAL;
>     107	
>     108		dma_fence_chain_for_each(*pfence, &chain->base) {
>     109			if ((*pfence)->context != chain->base.context ||
>     110			    to_dma_fence_chain(*pfence)->prev_seqno < seqno)
>     111				break;
>     112		}
>     113		dma_fence_put(&chain->base);
>     114	
>     115		return 0;
>     116	}
>     117	EXPORT_SYMBOL(dma_fence_chain_find_seqno);
>     118	
>     119	static const char *dma_fence_chain_get_driver_name(struct dma_fence *fence)
>     120	{
>     121	        return "dma_fence_chain";
>     122	}
>     123	
>     124	static const char *dma_fence_chain_get_timeline_name(struct dma_fence *fence)
>     125	{
>     126	        return "unbound";
>     127	}
>     128	
>     129	static void dma_fence_chain_irq_work(struct irq_work *work)
>     130	{
>     131		struct dma_fence_chain *chain;
>     132	
>     133		chain = container_of(work, typeof(*chain), work);
>     134	
>     135		/* Try to rearm the callback */
>     136		if (!dma_fence_chain_enable_signaling(&chain->base))
>     137			/* Ok, we are done. No more unsignaled fences left */
>     138			dma_fence_signal(&chain->base);
>     139		dma_fence_put(&chain->base);
>     140	}
>     141	
>     142	static void dma_fence_chain_cb(struct dma_fence *f, struct dma_fence_cb *cb)
>     143	{
>     144		struct dma_fence_chain *chain;
>     145	
>     146		chain = container_of(cb, typeof(*chain), cb);
>     147		irq_work_queue(&chain->work);
>     148		dma_fence_put(f);
>     149	}
>     150	
>     151	static bool dma_fence_chain_enable_signaling(struct dma_fence *fence)
>     152	{
>     153		struct dma_fence_chain *head = to_dma_fence_chain(fence);
>     154	
>     155		dma_fence_get(&head->base);
>     156		dma_fence_chain_for_each(fence, &head->base) {
>     157			struct dma_fence_chain *chain = to_dma_fence_chain(fence);
>     158			struct dma_fence *f = chain ? chain->fence : fence;
>     159	
>     160			dma_fence_get(f);
>     161			if (!dma_fence_add_callback(f, &head->cb, dma_fence_chain_cb)) {
>     162				dma_fence_put(fence);
>     163				return true;
>     164			}
>     165			dma_fence_put(f);
>     166		}
>     167		dma_fence_put(&head->base);
>     168		return false;
>     169	}
>     170	
>     171	static bool dma_fence_chain_signaled(struct dma_fence *fence)
>     172	{
>     173		dma_fence_chain_for_each(fence, fence) {
>     174			struct dma_fence_chain *chain = to_dma_fence_chain(fence);
>     175			struct dma_fence *f = chain ? chain->fence : fence;
>     176	
>     177			if (!dma_fence_is_signaled(f)) {
>     178				dma_fence_put(fence);
>     179				return false;
>     180			}
>     181		}
>     182	
>     183		return true;
>     184	}
>     185	
>     186	static void dma_fence_chain_release(struct dma_fence *fence)
>     187	{
>     188		struct dma_fence_chain *chain = to_dma_fence_chain(fence);
>     189	
>   > 190		dma_fence_put(chain->prev);
>     191		dma_fence_put(chain->fence);
>     192		dma_fence_free(fence);
>     193	}
>     194	
>     195	const struct dma_fence_ops dma_fence_chain_ops = {
>     196		.get_driver_name = dma_fence_chain_get_driver_name,
>     197		.get_timeline_name = dma_fence_chain_get_timeline_name,
>     198		.enable_signaling = dma_fence_chain_enable_signaling,
>     199		.signaled = dma_fence_chain_signaled,
>     200		.release = dma_fence_chain_release,
>     201	};
>     202	EXPORT_SYMBOL(dma_fence_chain_ops);
>     203	
>     204	/**
>     205	 * dma_fence_chain_init - initialize a fence chain
>     206	 * @chain: the chain node to initialize
>     207	 * @prev: the previous fence
>     208	 * @fence: the current fence
>     209	 *
>     210	 * Initialize a new chain node and either start a new chain or add the node to
>     211	 * the existing chain of the previous fence.
>     212	 */
>     213	void dma_fence_chain_init(struct dma_fence_chain *chain,
>     214				  struct dma_fence *prev,
>     215				  struct dma_fence *fence,
>     216				  uint64_t seqno)
>     217	{
>     218		struct dma_fence_chain *prev_chain = to_dma_fence_chain(prev);
>     219		uint64_t context;
>     220	
>     221		spin_lock_init(&chain->lock);
>   > 222		chain->prev = prev;
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Hi David,

For the cmpxchg() case I of hand don't know either. Looks like so far 
nobody has used cmpxchg() with rcu protected structures.

The other cases should be replaced by RCU_INIT_POINTER() or 
rcu_dereference_protected(.., true);

Regards,
Christian.

Am 21.03.19 um 07:34 schrieb zhoucm1:
> Hi Lionel and Christian,
>
> Below is robot report for chain->prev, which was added __rcu as you 
> suggested.
>
> How to fix this line "tmp = cmpxchg(&chain->prev, prev, replacement); "?
> I checked kernel header file, seems it has no cmpxchg for rcu.
>
> Any suggestion to fix this robot report?
>
> Thanks,
> -David
>
> On 2019年03月21日 08:24, kbuild test robot wrote:
>> Hi Chunming,
>>
>> I love your patch! Perhaps something to improve:
>>
>> [auto build test WARNING on linus/master]
>> [also build test WARNING on v5.1-rc1 next-20190320]
>> [if your patch is applied to the wrong git tree, please drop us a 
>> note to help improve the system]
>>
>> url: 
>> https://github.com/0day-ci/linux/commits/Chunming-Zhou/dma-buf-add-new-dma_fence_chain-container-v6/20190320-223607
>> reproduce:
>>          # apt-get install sparse
>>          make ARCH=x86_64 allmodconfig
>>          make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
>>
>>
>> sparse warnings: (new ones prefixed by >>)
>>
>>>> drivers/dma-buf/dma-fence-chain.c:73:23: sparse: incorrect type in 
>>>> initializer (different address spaces) @@    expected struct 
>>>> dma_fence [noderef] <asn:4>*__old @@    got  dma_fence [noderef] 
>>>> <asn:4>*__old @@
>>     drivers/dma-buf/dma-fence-chain.c:73:23:    expected struct 
>> dma_fence [noderef] <asn:4>*__old
>>     drivers/dma-buf/dma-fence-chain.c:73:23:    got struct dma_fence 
>> *[assigned] prev
>>>> drivers/dma-buf/dma-fence-chain.c:73:23: sparse: incorrect type in 
>>>> initializer (different address spaces) @@    expected struct 
>>>> dma_fence [noderef] <asn:4>*__new @@    got  dma_fence [noderef] 
>>>> <asn:4>*__new @@
>>     drivers/dma-buf/dma-fence-chain.c:73:23:    expected struct 
>> dma_fence [noderef] <asn:4>*__new
>>     drivers/dma-buf/dma-fence-chain.c:73:23:    got struct dma_fence 
>> *[assigned] replacement
>>>> drivers/dma-buf/dma-fence-chain.c:73:21: sparse: incorrect type in 
>>>> assignment (different address spaces) @@    expected struct 
>>>> dma_fence *tmp @@    got struct dma_fence [noderef] <struct 
>>>> dma_fence *tmp @@
>>     drivers/dma-buf/dma-fence-chain.c:73:21:    expected struct 
>> dma_fence *tmp
>>     drivers/dma-buf/dma-fence-chain.c:73:21:    got struct dma_fence 
>> [noderef] <asn:4>*[assigned] __ret
>>>> drivers/dma-buf/dma-fence-chain.c:190:28: sparse: incorrect type in 
>>>> argument 1 (different address spaces) @@    expected struct 
>>>> dma_fence *fence @@    got struct dma_fence struct dma_fence *fence @@
>>     drivers/dma-buf/dma-fence-chain.c:190:28:    expected struct 
>> dma_fence *fence
>>     drivers/dma-buf/dma-fence-chain.c:190:28:    got struct dma_fence 
>> [noderef] <asn:4>*prev
>>>> drivers/dma-buf/dma-fence-chain.c:222:21: sparse: incorrect type in 
>>>> assignment (different address spaces) @@    expected struct 
>>>> dma_fence [noderef] <asn:4>*prev @@    got [noderef] <asn:4>*prev @@
>>     drivers/dma-buf/dma-fence-chain.c:222:21:    expected struct 
>> dma_fence [noderef] <asn:4>*prev
>>     drivers/dma-buf/dma-fence-chain.c:222:21:    got struct dma_fence 
>> *prev
>>     drivers/dma-buf/dma-fence-chain.c:235:33: sparse: expression 
>> using sizeof(void)
>>     drivers/dma-buf/dma-fence-chain.c:235:33: sparse: expression 
>> using sizeof(void)
>>
>> vim +73 drivers/dma-buf/dma-fence-chain.c
>>
>>      38
>>      39    /**
>>      40     * dma_fence_chain_walk - chain walking function
>>      41     * @fence: current chain node
>>      42     *
>>      43     * Walk the chain to the next node. Returns the next fence 
>> or NULL if we are at
>>      44     * the end of the chain. Garbage collects chain nodes 
>> which are already
>>      45     * signaled.
>>      46     */
>>      47    struct dma_fence *dma_fence_chain_walk(struct dma_fence 
>> *fence)
>>      48    {
>>      49        struct dma_fence_chain *chain, *prev_chain;
>>      50        struct dma_fence *prev, *replacement, *tmp;
>>      51
>>      52        chain = to_dma_fence_chain(fence);
>>      53        if (!chain) {
>>      54            dma_fence_put(fence);
>>      55            return NULL;
>>      56        }
>>      57
>>      58        while ((prev = dma_fence_chain_get_prev(chain))) {
>>      59
>>      60            prev_chain = to_dma_fence_chain(prev);
>>      61            if (prev_chain) {
>>      62                if (!dma_fence_is_signaled(prev_chain->fence))
>>      63                    break;
>>      64
>>      65                replacement = 
>> dma_fence_chain_get_prev(prev_chain);
>>      66            } else {
>>      67                if (!dma_fence_is_signaled(prev))
>>      68                    break;
>>      69
>>      70                replacement = NULL;
>>      71            }
>>      72
>>    > 73            tmp = cmpxchg(&chain->prev, prev, replacement);
>>      74            if (tmp == prev)
>>      75                dma_fence_put(tmp);
>>      76            else
>>      77                dma_fence_put(replacement);
>>      78            dma_fence_put(prev);
>>      79        }
>>      80
>>      81        dma_fence_put(fence);
>>      82        return prev;
>>      83    }
>>      84    EXPORT_SYMBOL(dma_fence_chain_walk);
>>      85
>>      86    /**
>>      87     * dma_fence_chain_find_seqno - find fence chain node by 
>> seqno
>>      88     * @pfence: pointer to the chain node where to start
>>      89     * @seqno: the sequence number to search for
>>      90     *
>>      91     * Advance the fence pointer to the chain node which will 
>> signal this sequence
>>      92     * number. If no sequence number is provided then this is 
>> a no-op.
>>      93     *
>>      94     * Returns EINVAL if the fence is not a chain node or the 
>> sequence number has
>>      95     * not yet advanced far enough.
>>      96     */
>>      97    int dma_fence_chain_find_seqno(struct dma_fence **pfence, 
>> uint64_t seqno)
>>      98    {
>>      99        struct dma_fence_chain *chain;
>>     100
>>     101        if (!seqno)
>>     102            return 0;
>>     103
>>     104        chain = to_dma_fence_chain(*pfence);
>>     105        if (!chain || chain->base.seqno < seqno)
>>     106            return -EINVAL;
>>     107
>>     108        dma_fence_chain_for_each(*pfence, &chain->base) {
>>     109            if ((*pfence)->context != chain->base.context ||
>>     110 to_dma_fence_chain(*pfence)->prev_seqno < seqno)
>>     111                break;
>>     112        }
>>     113        dma_fence_put(&chain->base);
>>     114
>>     115        return 0;
>>     116    }
>>     117    EXPORT_SYMBOL(dma_fence_chain_find_seqno);
>>     118
>>     119    static const char *dma_fence_chain_get_driver_name(struct 
>> dma_fence *fence)
>>     120    {
>>     121            return "dma_fence_chain";
>>     122    }
>>     123
>>     124    static const char 
>> *dma_fence_chain_get_timeline_name(struct dma_fence *fence)
>>     125    {
>>     126            return "unbound";
>>     127    }
>>     128
>>     129    static void dma_fence_chain_irq_work(struct irq_work *work)
>>     130    {
>>     131        struct dma_fence_chain *chain;
>>     132
>>     133        chain = container_of(work, typeof(*chain), work);
>>     134
>>     135        /* Try to rearm the callback */
>>     136        if (!dma_fence_chain_enable_signaling(&chain->base))
>>     137            /* Ok, we are done. No more unsignaled fences left */
>>     138            dma_fence_signal(&chain->base);
>>     139        dma_fence_put(&chain->base);
>>     140    }
>>     141
>>     142    static void dma_fence_chain_cb(struct dma_fence *f, struct 
>> dma_fence_cb *cb)
>>     143    {
>>     144        struct dma_fence_chain *chain;
>>     145
>>     146        chain = container_of(cb, typeof(*chain), cb);
>>     147        irq_work_queue(&chain->work);
>>     148        dma_fence_put(f);
>>     149    }
>>     150
>>     151    static bool dma_fence_chain_enable_signaling(struct 
>> dma_fence *fence)
>>     152    {
>>     153        struct dma_fence_chain *head = to_dma_fence_chain(fence);
>>     154
>>     155        dma_fence_get(&head->base);
>>     156        dma_fence_chain_for_each(fence, &head->base) {
>>     157            struct dma_fence_chain *chain = 
>> to_dma_fence_chain(fence);
>>     158            struct dma_fence *f = chain ? chain->fence : fence;
>>     159
>>     160            dma_fence_get(f);
>>     161            if (!dma_fence_add_callback(f, &head->cb, 
>> dma_fence_chain_cb)) {
>>     162                dma_fence_put(fence);
>>     163                return true;
>>     164            }
>>     165            dma_fence_put(f);
>>     166        }
>>     167        dma_fence_put(&head->base);
>>     168        return false;
>>     169    }
>>     170
>>     171    static bool dma_fence_chain_signaled(struct dma_fence *fence)
>>     172    {
>>     173        dma_fence_chain_for_each(fence, fence) {
>>     174            struct dma_fence_chain *chain = 
>> to_dma_fence_chain(fence);
>>     175            struct dma_fence *f = chain ? chain->fence : fence;
>>     176
>>     177            if (!dma_fence_is_signaled(f)) {
>>     178                dma_fence_put(fence);
>>     179                return false;
>>     180            }
>>     181        }
>>     182
>>     183        return true;
>>     184    }
>>     185
>>     186    static void dma_fence_chain_release(struct dma_fence *fence)
>>     187    {
>>     188        struct dma_fence_chain *chain = 
>> to_dma_fence_chain(fence);
>>     189
>>   > 190        dma_fence_put(chain->prev);
>>     191        dma_fence_put(chain->fence);
>>     192        dma_fence_free(fence);
>>     193    }
>>     194
>>     195    const struct dma_fence_ops dma_fence_chain_ops = {
>>     196        .get_driver_name = dma_fence_chain_get_driver_name,
>>     197        .get_timeline_name = dma_fence_chain_get_timeline_name,
>>     198        .enable_signaling = dma_fence_chain_enable_signaling,
>>     199        .signaled = dma_fence_chain_signaled,
>>     200        .release = dma_fence_chain_release,
>>     201    };
>>     202    EXPORT_SYMBOL(dma_fence_chain_ops);
>>     203
>>     204    /**
>>     205     * dma_fence_chain_init - initialize a fence chain
>>     206     * @chain: the chain node to initialize
>>     207     * @prev: the previous fence
>>     208     * @fence: the current fence
>>     209     *
>>     210     * Initialize a new chain node and either start a new 
>> chain or add the node to
>>     211     * the existing chain of the previous fence.
>>     212     */
>>     213    void dma_fence_chain_init(struct dma_fence_chain *chain,
>>     214                  struct dma_fence *prev,
>>     215                  struct dma_fence *fence,
>>     216                  uint64_t seqno)
>>     217    {
>>     218        struct dma_fence_chain *prev_chain = 
>> to_dma_fence_chain(prev);
>>     219        uint64_t context;
>>     220
>>     221        spin_lock_init(&chain->lock);
>>   > 222        chain->prev = prev;
>>
>> ---
>> 0-DAY kernel test infrastructure                Open Source 
>> Technology Center
>> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>