On Tue, 5 Mar 2024 14:24:20 +0800 linke li <lilinke99@xxxxxx> wrote: > > Anyway, a slightly related/different question: > > > > Should that: > > WRITE_ONCE(rp->rtort_pipe_count, rp->rtort_pipe_count + 1); > > > > Be: > > WRITE_ONCE(rp->rtort_pipe_count, READ_ONCE(rp->rtort_pipe_count) + 1); > > > > ? > > Hi, Joel. Sorry, I am not very sure about this. I do a little research on > it. > > I looked through all code in kernel that looks like this, both kinds are > used. I also try to compile this file with and without the READ_ONCE in > WRITE_ONCE using gcc-9. Their binary is just the same. > > Thus I think in the current compiler version, they do not have a big > difference, but if the compiler wants to optimize more, maybe the latter > one is better. I'm sorry but all these READ_ONCE/WRITE_ONCE() additions that are being added because there's a fear of the compiler tearing long words, is going to make the code really ugly and hard to read. If we take the policy of handling a compiler that can tear reads and writes of any size word, then we should have proper macros to handle it. Perhaps READ_SHARED(), WRITE_SHARED(), ADD_SHARED(), SUB_SHARED(). The ONCE has nothing to do with the reasons for these changes. But at least "SHARED" can be considered "this variable is shared between different contexts". Note, this is different than "atomic". It's just to document that this variable must be loaded or stored in one transaction. I don't know if Linus even cares about fixing "read/write tearing" which is why I Cc'd him. But I'm not going to take any patches that add these macros to fix compilers that tear words on load and store until we have a set policy on what to do with them. -- Steve
