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

Zhou, David(ChunMing) David1.Zhou at amd.com
Thu Mar 21 14:13:52 UTC 2019


cmpxchg be replaced by some simple c sentance?
otherwise we have to remove __rcu of chian->prev.

-David

-------- Original Message --------
Subject: Re: [PATCH 1/9] dma-buf: add new dma_fence_chain container v6
From: Christian König
To: "Zhou, David(ChunMing)" ,kbuild test robot ,"Zhou, David(ChunMing)"
CC: kbuild-all at 01.org,dri-devel at lists.freedesktop.org,amd-gfx at lists.freedesktop.org,lionel.g.landwerlin at intel.com,jason at jlekstrand.net,"Koenig, Christian" ,"Hector, Tobias"

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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20190321/ec80f61a/attachment-0001.html>


More information about the dri-devel mailing list