On Thu, Jan 17, 2019 at 3:11 PM Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote: > > On 2019/01/16 19:43, Jan Kara wrote: > > On Wed 16-01-19 10:47:56, Dmitry Vyukov wrote: > >>> I still cannot catch. Read/write of sizeof(long) bytes at naturally > >>> aligned address is atomic, isn't it? > >> > >> Nobody guarantees this. According to C non-atomic conflicting > >> reads/writes of sizeof(long) cause undefined behavior of the whole > >> program. > > > > Yes, but to be fair the kernel has always relied on long accesses to be > > atomic pretty heavily so that it is now de-facto standard for the kernel > > AFAICT. I understand this makes life for static checkers hard but such is > > reality. > > Regarding "load a long word from naturally aligned address" and > "store a long word to naturally aligned address", they are required to be atomic > in Linux kernel, (for it is described as "On all systems running Linux, loads from > and stores to pointers are atomic, that is, if a store to a pointer occurs at the > same time as a load from that same pointer, the load will return either the initial > value or the value stored, never some bitwise mashup of the two." at > https://lwn.net/Articles/262464/ ) aren't they? Well, the description of the patch explains how/why a compiler can make a C pointer store non-atomic. Also you can see that list_for_each_entry_rcu uses READ_ONCE: https://elixir.bootlin.com/linux/v5.0-rc2/source/include/linux/rculist.h#L278 > And I know we need to use READ_ONCE()/WRITE_ONCE() etc. when we need to guarantee > that reordering/reloading is not acceptable. READ/WRITE_ONCE don't have anything to do with ordering. They only provide atomicity and visibility. That's various acquire/release/barriers that provide ordering guarantees. > But what damage can cause for this > specific case? Any. > It does only "compare two long variables" in addition to "load a > long word from naturally aligned address" and "store a long word to naturally > aligned address". Reordering/reloading these variables won't cause severe problems. > > > > > But in this particular case I agree with you that special logic is not > > really warranted. > > Anyway, do we apply V2 patch at > https://marc.info/?i=cd9c47c9-1dc6-6949-a138-703f6a10bc9b@xxxxxxxxxxxxxxxxxxx ?
