On 2018年11月28日 22:50, Christian König wrote:

For a lot of use cases we need 64bit sequence numbers. Currently drivers overload the dma_fence structure to store the additional bits.

Stop doing that and make the sequence number in the dma_fence always 64bit.

For compatibility with hardware which can do only 32bit sequences the comparisons in __dma_fence_is_later still only takes the lower 32bits as significant.

Can't we compare 64bit variable directly?  Can we do it as below?

-static inline bool __dma_fence_is_later(u32 f1, u32 f2) +static inline bool __dma_fence_is_later(u64 f1, u64 f2) { - return (int)(f1 - f2) > 0; + return (f1 > f2) ? true : false;

}

-David

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

drivers/dma-buf/dma-fence.c | 2 +- drivers/dma-buf/sw_sync.c | 2 +- drivers/dma-buf/sync_file.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +- drivers/gpu/drm/i915/i915_sw_fence.c | 2 +- drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- drivers/gpu/drm/vgem/vgem_fence.c | 4 ++-- include/linux/dma-fence.h | 14 +++++++------- 8 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 1551ca7df394..37e24b69e94b 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -615,7 +615,7 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout); */ void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,

•       spinlock_t *lock, u64 context, unsigned seqno)
  

•       spinlock_t *lock, u64 context, u64 seqno)
  

  { BUG_ON(!lock); BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name);

diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 53c1d6d36a64..32dcf7b4c935 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -172,7 +172,7 @@ static bool timeline_fence_enable_signaling(struct dma_fence *fence) static void timeline_fence_value_str(struct dma_fence *fence, char *str, int size) {

• snprintf(str, size, "%d", fence->seqno);

• snprintf(str, size, "%lld", fence->seqno); }

  static void timeline_fence_timeline_value_str(struct dma_fence *fence,

diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 35dd06479867..4f6305ca52c8 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -144,7 +144,7 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len) } else { struct dma_fence *fence = sync_file->fence;

• snprintf(buf, len, "%s-%s%llu-%d",
  

• snprintf(buf, len, "%s-%s%llu-%lld",
   fence->ops->get_driver_name(fence),
   fence->ops->get_timeline_name(fence),
   fence->context,
  

@@ -258,7 +258,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,

	i_b++;
} else {

• 	if (pt_a->seqno - pt_b->seqno <= INT_MAX)
  

• 	if (__dma_fence_is_later(pt_a->seqno, pt_b->seqno))
  	add_fence(fences, &i, pt_a);
  else
  	add_fence(fences, &i, pt_b);
  

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c index 12f2bf97611f..bfaf5c6323be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c @@ -388,7 +388,7 @@ void amdgpu_sa_bo_dump_debug_info(struct amdgpu_sa_manager *sa_manager, soffset, eoffset, eoffset - soffset);

if (i->fence)

• 	seq_printf(m, " protected by 0x%08x on context %llu",
  

• 	seq_printf(m, " protected by 0x%016llx on context %llu",
  	   i->fence->seqno, i->fence->context);
  
  

  seq_printf(m, "\n");

diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c index 6dbeed079ae5..11bcdabd5177 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence.c +++ b/drivers/gpu/drm/i915/i915_sw_fence.c @@ -393,7 +393,7 @@ static void timer_i915_sw_fence_wake(struct timer_list *t) if (!fence) return;

• pr_notice("Asynchronous wait on fence %s:%s:%x timed out (hint:%pS)\n",

• pr_notice("Asynchronous wait on fence %s:%s:%llx timed out (hint:%pS)\n", cb->dma->ops->get_driver_name(cb->dma), cb->dma->ops->get_timeline_name(cb->dma), cb->dma->seqno,

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 217ed3ee1cab..f28a66c67d34 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1236,7 +1236,7 @@ static void print_request(struct drm_printer *m,

x = print_sched_attr(rq->i915, &rq->sched.attr, buf, x, sizeof(buf));

• drm_printf(m, "%s%x%s [%llx:%x]%s @ %dms: %s\n",

• drm_printf(m, "%s%x%s [%llx:%llx]%s @ %dms: %s\n", prefix, rq->global_seqno, i915_request_completed(rq) ? "!" : "",

diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c index c1c420afe2dd..eb17c0cd3727 100644 --- a/drivers/gpu/drm/vgem/vgem_fence.c +++ b/drivers/gpu/drm/vgem/vgem_fence.c @@ -53,13 +53,13 @@ static void vgem_fence_release(struct dma_fence *base)

static void vgem_fence_value_str(struct dma_fence *fence, char *str, int size) {

• snprintf(str, size, "%u", fence->seqno);

• snprintf(str, size, "%llu", fence->seqno); }

  static void vgem_fence_timeline_value_str(struct dma_fence *fence, char *str, int size) {

• snprintf(str, size, "%u",

• snprintf(str, size, "%llu", dma_fence_is_signaled(fence) ? fence->seqno : 0); }

diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 02dba8cd033d..1393529c0c1f 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -77,7 +77,7 @@ struct dma_fence { struct list_head cb_list; spinlock_t *lock; u64 context;

• unsigned seqno;

• u64 seqno; unsigned long flags; ktime_t timestamp; int error;

@@ -244,7 +244,7 @@ struct dma_fence_ops { };

void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,

•     spinlock_t *lock, u64 context, unsigned seqno);
  

•     spinlock_t *lock, u64 context, u64 seqno);
  
  

  void dma_fence_release(struct kref *kref); void dma_fence_free(struct dma_fence *fence);

@@ -414,9 +414,9 @@ dma_fence_is_signaled(struct dma_fence *fence)

• Returns true if f1 is chronologically later than f2. Both fences must be
• from the same context, since a seqno is not common across contexts.

*/ -static inline bool __dma_fence_is_later(u32 f1, u32 f2) +static inline bool __dma_fence_is_later(u64 f1, u64 f2) {

• return (int)(f1 - f2) > 0;

• return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0; }

  /**

@@ -547,21 +547,21 @@ u64 dma_fence_context_alloc(unsigned num); do { \ struct dma_fence *__ff = (f); \ if (IS_ENABLED(CONFIG_DMA_FENCE_TRACE)) \

• 	pr_info("f %llu#%u: " fmt,			\
  

• 	pr_info("f %llu#%llu: " fmt,			\
  	__ff->context, __ff->seqno, ##args);	\
  

  } while (0)

  #define DMA_FENCE_WARN(f, fmt, args...) \ do { \ struct dma_fence *__ff = (f); \

• pr_warn("f %llu#%u: " fmt, __ff->context, __ff->seqno,	\
  

• pr_warn("f %llu#%llu: " fmt, __ff->context, __ff->seqno,\
   ##args);					\
  

  } while (0)

  #define DMA_FENCE_ERR(f, fmt, args...) \ do { \ struct dma_fence *__ff = (f); \

• pr_err("f %llu#%u: " fmt, __ff->context, __ff->seqno,	\
  

• pr_err("f %llu#%llu: " fmt, __ff->context, __ff->seqno,	\
  ##args);					\
  

  } while (0)

Am 03.12.18 um 06:25 schrieb zhoucm1:

On 2018年11月28日 22:50, Christian König wrote:

...

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.

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

drivers/dma-buf/Makefile          |   3 +-   drivers/dma-buf/dma-fence-chain.c | 235 ++++++++++++++++++++++++++++++++++++++   include/linux/dma-fence-chain.h   |  79 +++++++++++++   3 files changed, 316 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..de05101fc48d --- /dev/null +++ b/drivers/dma-buf/dma-fence-chain.c @@ -0,0 +1,235 @@ +/*

• • 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) { +        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); +}

+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_chain_for_each(fence) { +        struct dma_fence_chain *chain = to_dma_fence_chain(fence); +        struct dma_fence *f = chain ? chain->fence : fence;

+        if (!dma_fence_add_callback(f, &head->cb, dma_fence_chain_cb)) +            return true; +    } +    return false; +}

+static bool dma_fence_chain_signaled(struct dma_fence *fence) +{ +    dma_fence_chain_for_each(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 && seqno > prev->seqno && +        __dma_fence_is_later(seqno, prev->seqno)) {

As your patch#1 makes __dma_fence_is_later only be valid for 32bit, we cannot use it for 64bit here, we should remove it from here, just compare seqno directly.

That is intentional. We must make sure that the number both increments as 64bit number as well as not wraps around as 32bit number.

In other words the largest difference between two sequence numbers userspace is allowed to submit is 1<<31.

...

+        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);

I still cannot judge if this case is proper, but I prefer we just drop it when seqno is <= last seqno. we can image that when we enable export/import, we could mess them. So we shouldn't change timeline existing signal point any time.

Yeah, but we can't lose fences either. This is the most defensive approach I can think of.

E.g. we still ad the fence, but start a new timeline so all queries for all previous sequence number will always wait for everything.

Regards, Christian.

-David

...

+    }

+    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..3bb0efd6bc65 --- /dev/null +++ b/include/linux/dma-fence-chain.h @@ -0,0 +1,79 @@ +/*

• • 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
• • @fence: starting point as well as current fence
• • 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(fence)    \ +    for (dma_fence_get(fence);fence;fence=dma_fence_chain_walk(fence))

+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 */

Am 03.12.18 um 14:18 schrieb Chunming Zhou:

在 2018/12/3 19:00, Christian König 写道:

...

Am 03.12.18 um 06:25 schrieb zhoucm1:

...

On 2018年11月28日 22:50, Christian König wrote:

...

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.

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

drivers/dma-buf/Makefile          |   3 +-   drivers/dma-buf/dma-fence-chain.c | 235 ++++++++++++++++++++++++++++++++++++++   include/linux/dma-fence-chain.h   |  79 +++++++++++++   3 files changed, 316 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..de05101fc48d --- /dev/null +++ b/drivers/dma-buf/dma-fence-chain.c @@ -0,0 +1,235 @@ +/*

• • 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) { +        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); +}

+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_chain_for_each(fence) { +        struct dma_fence_chain *chain = to_dma_fence_chain(fence); +        struct dma_fence *f = chain ? chain->fence : fence;

+        if (!dma_fence_add_callback(f, &head->cb, dma_fence_chain_cb)) +            return true; +    } +    return false; +}

+static bool dma_fence_chain_signaled(struct dma_fence *fence) +{ +    dma_fence_chain_for_each(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 && seqno > prev->seqno && +        __dma_fence_is_later(seqno, prev->seqno)) {

As your patch#1 makes __dma_fence_is_later only be valid for 32bit, we cannot use it for 64bit here, we should remove it from here, just compare seqno directly.

That is intentional. We must make sure that the number both increments as 64bit number as well as not wraps around as 32bit number.

In other words the largest difference between two sequence numbers userspace is allowed to submit is 1<<31.

Why? no one can make sure that, application users would only think it is an uint64 sequence nubmer, and they can signal any advanced point. I already see umd guys writing timeline test use max_uint64-1 as a final signal. We shouldn't add this limitation here.

We need to be backward compatible to hardware which can only do 32bit signaling with the dma-fence implementation.

Otherwise dma_fence_later() could return an inconsistent result and break at other places.

So if userspace wants to use more than 1<<31 difference between sequence numbers we need to push back on this.

Christian.

-David

...

...

...

+        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);

I still cannot judge if this case is proper, but I prefer we just drop it when seqno is <= last seqno. we can image that when we enable export/import, we could mess them. So we shouldn't change timeline existing signal point any time.

Yeah, but we can't lose fences either. This is the most defensive approach I can think of.

E.g. we still ad the fence, but start a new timeline so all queries for all previous sequence number will always wait for everything.

Regards, Christian.

...

-David

...

+    }

+    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..3bb0efd6bc65 --- /dev/null +++ b/include/linux/dma-fence-chain.h @@ -0,0 +1,79 @@ +/*

• • 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
• • @fence: starting point as well as current fence
• • 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(fence)    \ +    for (dma_fence_get(fence);fence;fence=dma_fence_chain_walk(fence))

+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 */

---

amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Am 03.12.18 um 14:44 schrieb Chunming Zhou:

在 2018/12/3 21:28, Christian König 写道:

...

Am 03.12.18 um 14:18 schrieb Chunming Zhou:

...

在 2018/12/3 19:00, Christian König 写道:

...

Am 03.12.18 um 06:25 schrieb zhoucm1:

...

On 2018年11月28日 22:50, Christian König wrote:

...

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.

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

drivers/dma-buf/Makefile          |   3 +-    drivers/dma-buf/dma-fence-chain.c | 235 ++++++++++++++++++++++++++++++++++++++    include/linux/dma-fence-chain.h   |  79 +++++++++++++    3 files changed, 316 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..de05101fc48d --- /dev/null +++ b/drivers/dma-buf/dma-fence-chain.c @@ -0,0 +1,235 @@ +/*

• • 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) { +        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); +}

+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_chain_for_each(fence) { +        struct dma_fence_chain *chain = to_dma_fence_chain(fence); +        struct dma_fence *f = chain ? chain->fence : fence;

+        if (!dma_fence_add_callback(f, &head->cb, dma_fence_chain_cb)) +            return true; +    } +    return false; +}

+static bool dma_fence_chain_signaled(struct dma_fence *fence) +{ +    dma_fence_chain_for_each(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 && seqno > prev->seqno && +        __dma_fence_is_later(seqno, prev->seqno)) {

As your patch#1 makes __dma_fence_is_later only be valid for 32bit, we cannot use it for 64bit here, we should remove it from here, just compare seqno directly.

That is intentional. We must make sure that the number both increments as 64bit number as well as not wraps around as 32bit number.

In other words the largest difference between two sequence numbers userspace is allowed to submit is 1<<31.

Why? no one can make sure that, application users would only think it is an uint64 sequence nubmer, and they can signal any advanced point. I already see umd guys writing timeline test use max_uint64-1 as a final signal. We shouldn't add this limitation here.

We need to be backward compatible to hardware which can only do 32bit signaling with the dma-fence implementation.

I see that, you already explained that before. but can't we just grep low 32bit of seqno only when 32bit hardware try to use?

then we can make dma_fence_later use 64bit comparation.

The problem is that we don't know at all times when to use a 32bit compare and when to use a 64bit compare.

What we could do is test if any of the upper 32bits of a sequence number is not 0 and if that is the case do a 64bit compare. This way max_uint64_t would still be handled correctly.

Christian.

...

Otherwise dma_fence_later() could return an inconsistent result and break at other places.

So if userspace wants to use more than 1<<31 difference between sequence numbers we need to push back on this.

It's rare case, but I don't think we can convince them add this limitation. So we cannot add this limitation here.

-David

Could you move this one to dma-fence as you said? Which will be used in other place as well.

-David

On 2018年11月28日 22:50, Christian König wrote:

Extract of useful code from the timeline work. Let's use just a single stub fence instance instead of allocating a new one all the time.

Signed-off-by: Chunming Zhou david1.zhou@amd.com Signed-off-by: Christian König christian.koenig@amd.com

---

drivers/gpu/drm/drm_syncobj.c | 67 ++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index b92e3c726229..f78321338c1f 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -56,10 +56,8 @@ #include "drm_internal.h" #include <drm/drm_syncobj.h>

-struct drm_syncobj_stub_fence {

• struct dma_fence base;
• spinlock_t lock;

-}; +static DEFINE_SPINLOCK(stub_fence_lock); +static struct dma_fence stub_fence;

static const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence) { @@ -71,6 +69,25 @@ static const struct dma_fence_ops drm_syncobj_stub_fence_ops = { .get_timeline_name = drm_syncobj_stub_fence_get_name, };

+/**

• • drm_syncobj_get_stub_fence - return a signaled fence
• • Return a stub fence which is already signaled.
• */

+static struct dma_fence *drm_syncobj_get_stub_fence(void) +{

• spin_lock(&stub_fence_lock);
• if (!stub_fence.ops) {

• dma_fence_init(&stub_fence,
  

• 	       &drm_syncobj_stub_fence_ops,
  

• 	       &stub_fence_lock,
  

• 	       0, 0);
  

• dma_fence_signal_locked(&stub_fence);
  

• }
• spin_unlock(&stub_fence_lock);
• return dma_fence_get(&stub_fence);

+}

/**

• drm_syncobj_find - lookup and reference a sync object.

@@ -188,23 +205,18 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence);

-static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) +/**

• • drm_syncobj_assign_null_handle - assign a stub fence to the sync object
• • @syncobj: sync object to assign the fence on
• • Assign a already signaled stub fence to the sync object.
• */

+static void drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) {

• struct drm_syncobj_stub_fence *fence;
• fence = kzalloc(sizeof(*fence), GFP_KERNEL);
• if (fence == NULL)

• return -ENOMEM;
  

• struct dma_fence *fence = drm_syncobj_get_stub_fence();

• spin_lock_init(&fence->lock);
• dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops,

•        &fence->lock, 0, 0);
  

• dma_fence_signal(&fence->base);
• drm_syncobj_replace_fence(syncobj, &fence->base);
• dma_fence_put(&fence->base);
• return 0;

• drm_syncobj_replace_fence(syncobj, fence);

• dma_fence_put(fence); }

  /**

@@ -272,7 +284,6 @@ EXPORT_SYMBOL(drm_syncobj_free); int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags, struct dma_fence *fence) {

• int ret; struct drm_syncobj *syncobj;

  syncobj = kzalloc(sizeof(struct drm_syncobj), GFP_KERNEL);

@@ -283,13 +294,8 @@ int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags, INIT_LIST_HEAD(&syncobj->cb_list); spin_lock_init(&syncobj->lock);

• if (flags & DRM_SYNCOBJ_CREATE_SIGNALED) {

• ret = drm_syncobj_assign_null_handle(syncobj);
  

• if (ret < 0) {
  

• 	drm_syncobj_put(syncobj);
  

• 	return ret;
  

• }
  

• }

• if (flags & DRM_SYNCOBJ_CREATE_SIGNALED)

• drm_syncobj_assign_null_handle(syncobj);
  
  

  if (fence) drm_syncobj_replace_fence(syncobj, fence);

@@ -980,11 +986,8 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, if (ret < 0) return ret;

• for (i = 0; i < args->count_handles; i++) {

• ret = drm_syncobj_assign_null_handle(syncobjs[i]);
  

• if (ret < 0)
  

• 	break;
  

• }

• for (i = 0; i < args->count_handles; i++)

• drm_syncobj_assign_null_handle(syncobjs[i]);
  
  

  drm_syncobj_array_free(syncobjs, args->count_handles);