[PATCH 1/9] dma-buf: add new dma_fence_chain container v6
Christian König
ckoenig.leichtzumerken at gmail.com
Thu Mar 21 14:40:50 UTC 2019
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20190321/7524afb3/attachment-0001.html>
More information about the amd-gfx
mailing list