[PATCH 1/9] dma-buf: add new dma_fence_chain container v6
Christian König
ckoenig.leichtzumerken at gmail.com
Fri Mar 22 15:44:00 UTC 2019
Yeah, that should work.
Christian.
Am 22.03.19 um 08:34 schrieb zhoucm1:
>
> how about the attached?
>
> If ok, I will merge to pathc#1.
>
>
> -David
>
>
> On 2019年03月21日 22:40, Christian König wrote:
>> No, atomic cmpxchg is a hardware operation. If you want to replace
>> that you need a lock again.
>>
>> Maybe just add a comment and use an explicit cast to void* ? Not sure
>> if that silences the warning.
>>
>> Christian.
>>
>> Am 21.03.19 um 15:13 schrieb Zhou, David(ChunMing):
>>> 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
>>> >
>>>
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20190322/baad8ebe/attachment-0001.html>
More information about the amd-gfx
mailing list