[PATCH 02/11] dma-buf: add new dma_fence_chain container v2
Christian König
ckoenig.leichtzumerken at gmail.com
Mon Dec 3 13:55:48 UTC 2018
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 at 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 at 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
More information about the amd-gfx
mailing list