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-de...@lists.freedesktop.org,amd-gfx@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-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
