On Tue, Apr 28, 2020 at 04:25:39PM -0700, Andrii Nakryiko wrote: > > > > compiler doesn't guarantee that plain 32-bit load/store will stay 32-bit > > even on 64-bit archs. > > > > > If that was the case, neither > > > WRITE_ONCE/READ_ONCE nor smp_write_release/smp_load_acquire would > > > help. > > > > what do you mean? They will. That's the point of these macros. > > According to Documentation/memory-barriers.txt, > smp_load_acquire/smp_store_release are about ordering and memory > barriers, not about guaranteeing atomicity of reading value. > Especially READ_ONCE/WRITE_ONCE which are volatile read/write, not > atomic read/write. But nevertheless, I'll do lock and this will become > moot. May be that's something for Paul to clarify in the doc? smp_load_acquire() is READ_ONCE() + smp_mb() unoptimized in general case. And READ_ONCE + barrier on x86. > > > > > But I don't think that's the case, we have code in verifier that > > > does similar racy u32 write/read (it uses READ_ONCE/WRITE_ONCE) and > > > seems to be working fine. > > > > you mean in btf_resolve_helper_id() ? > > What kind of race do you see there? > > Two CPUs reading/writing to same variable without lock? Value starts > at 0 (meaning "not yet ready") and eventually becoming valid and final > non-zero value. Even if they race, and one CPU reads 0 while another > CPU already set it to non-zero, it's fine. In verifier's case it will > be eventually overwritten with the same resolved btf id. In case of > bpf_link, GET_FD_BY_ID would pretend link doesn't exist yet and return > error. Seems similar enough to me. ahh. similar in the sense that only one value is written. it's either zero or whatever_that_id. Right.
