On Fri, Aug 4, 2023 at 11:47 AM Alan Huang <mmpgouride@xxxxxxxxx> wrote: > > > > >>> > >>>>>> But the example here is different, > >>>>> > >>>>> That is intentional. Wills discussion partially triggered this. Though I am wondering > >>>>> if we should document that as well. > >>>>> > >>>>>> the compiler can not use the value loaded from line 5 > >>>>>> unless the compiler can deduce that the tmp is equals to p in which case the address dependency > >>>>>> doesn’t exist anymore. > >>>>>> > >>>>>> What am I missing here? > >>>>> > >>>>> Maybe you are trying to rationalize too much that the sequence mentioned cannot result > >>>>> in a counter intuitive outcome like I did? > >>>>> > >>>>> The point AFAIU is not just about line 10 but that the compiler can replace any of the > >>>>> lines after the plain access with the cached value. > >>>> > >>>> Well, IIUC, according to the C standard, the compiler can do anything if there is a data race (undefined behavior). > >>>> > >>>> However, what if a write is not protected with WRITE_ONCE and the read is marked with READ_ONCE? > >>>> That’s also a data race, right? But the kernel considers it is Okay if the write is machine word aligned. > >>> > >>> Yes, but there is a compiler between the HLL code and what the > >>> processor sees which can tear the write. How can not using > >>> WRITE_ONCE() prevent store-tearing? See [1]. My understanding is that > >>> it is OK only if the reader did a NULL check. In that case the torn > >> > >> Yes, a write-write data race where the value is the same is also fine. > >> > >> But they are still data race, if the compiler is within its right to do anything it likes (due to data race), > >> we still need WRITE_ONCE() in these cases, though it’s semantically safe. > >> > >> IIUC, even with _ONCE(), the compiler is within its right do anything according to the standard (at least before the upcoming C23), because the standard doesn’t consider a volatile access to be atomic. > >> > >> However, the kernel consider the volatile access to be atomic, right? > >> > >> BTW, line 5 in the example is likely to be optimized away. And yes, the compiler can cache the value loaded from line 5 from the perspective of undefined behavior, even if I believe it would be a compiler bug from the perspective of kernel. > > > > I am actually a bit lost with what you are trying to say. Are you saying that mixing > > plain accesses with marked accesses is an acceptable practice? > > > I’m trying to say that sometimes data race is fine, that’s why we have the data_race(). > > Even if the standard says data race results in UB. > > And IMHO, the possible data race at line 5 in this example is also fine, unless the compiler > deduces that the value of gp is always the same. IMHO, no one is saying it is not "fine". As in, such behavior is neither a compiler nor strictly a kernel bug. More a wtf that the programmer should know off (does not hurt to know). I will rest my case with AlanH pending any input from people who know more than me. If there is a better way to represent such matters in the docs, I am happy to make changes to this patch. Cheers, - Joel
