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-...@01.org,dri-devel@lists.freedesktop.org,amd-...@lists.freedesktop.org,lionel.g.landwer...@intel.com,ja...@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
>
_______________________________________________
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx