On Wed, Mar 8, 2017 at 4:20 PM, Mark Rutland <mark.rutland@xxxxxxx> wrote: > Hi, > > On Wed, Mar 08, 2017 at 02:42:10PM +0100, Dmitry Vyukov wrote: >> I think if we scope compiler atomic builtins to KASAN/KTSAN/KMSAN (and >> consequently x86/arm64) initially, it becomes more realistic. For the >> tools we don't care about absolute efficiency and this gets rid of >> Will's points (2), (4) and (6) here https://lwn.net/Articles/691295/. >> Re (3) I think rmb/wmb can be reasonably replaced with >> atomic_thread_fence(acquire/release). Re (5) situation with >> correctness becomes better very quickly as more people use them in >> user-space. Since KASAN is not intended to be used in production (or >> at least such build is expected to crash), we can afford to shake out >> any remaining correctness issues in such build. (1) I don't fully >> understand, what exactly is the problem with seq_cst? > > I'll have to leave it to Will to have the final word on these; I'm > certainly not familiar enough with the C11 memory model to comment on > (1). > > However, w.r.t. (3), I don't think we can substitute rmb() and wmb() > with atomic_thread_fence_acquire() and atomic_thread_fence_release() > respectively on arm64. > > The former use barriers with full system scope, whereas the latter may > be limited to the inner shareable domain. While I'm not sure of the > precise intended semantics of wmb() and rmb(), I believe this > substitution would break some cases (e.g. communicating with a > non-coherent master). > > Note that regardless, we'd have to special-case __iowmb() to use a full > system barrier. > > Also, w.r.t. (5), modulo the lack of atomic instrumentation, people use > KASAN today, with compilers that are known to have bugs in their atomics > (e.g. GCC bug 69875). Thus, we cannot rely on the compiler's > implementation of atomics without introducing a functional regression. > >> i'Ve sketched a patch that does it, and did some testing with/without >> KASAN on x86_64. >> >> In short, it adds include/linux/atomic_compiler.h which is included >> from include/linux/atomic.h when CONFIG_COMPILER_ATOMIC is defined; >> and <asm/atomic.h> is not included when CONFIG_COMPILER_ATOMIC is >> defined. >> For bitops it is similar except that only parts of asm/bitops.h are >> selectively disabled when CONFIG_COMPILER_ATOMIC, because it also >> defines other stuff. >> asm/barriers.h is left intact for now. We don't need it for KASAN. But >> for KTSAN we can do similar thing -- selectively disable some of the >> barriers in asm/barriers.h (e.g. leaving dma_rmb/wmb per arch). >> >> Such change would allow us to support atomic ops for multiple arches >> for all of KASAN/KTSAN/KMSAN. >> >> Thoughts? > > As in my other reply, I'd prefer that we wrapped the (arch-specific) > atomic implementations such that we can instrument them explicitly in a > core header. That means that the implementation and semantics of the > atomics don't change at all. > > Note that we could initially do this just for x86 and arm64), e.g. by > having those explicitly include an <asm-generic/atomic-instrumented.h> > at the end of their <asm/atomic.h>. How exactly do you want to do this incrementally? I don't feel ready to shuffle all archs, but doing x86 in one patch and then arm64 in another looks tractable. > For architectures which can use the compiler's atomics, we can allow > them to do so, skipping the redundant explicit instrumentation. > > Other than being potentially slower (which we've established we don't > care too much about above), is there a problem with that approach? -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
