On Thu, Dec 12, 2019 at 09:41:32AM -0800, Linus Torvalds wrote: > On Thu, Dec 12, 2019 at 2:46 AM Peter Zijlstra <pet...@infradead.org> wrote: > > > > +#ifdef GCC_VERSION < 40800 > > Where does that 4.8 version check come from, and why? > > Yeah, I know, but this really wants a comment. Sadly it looks like gcc > bugzilla is down, so > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 > > currently gives an "Internal Server Error" for me. > > [ Delete the horrid code we have because of gcc bugs ] > > > +#else /* GCC_VERSION < 40800 */ > > + > > +#define READ_ONCE_NOCHECK(x) \ > > +({ \ > > + typeof(x) __x = *(volatile typeof(x))&(x); \ > > I think we can/should just do this unconditionally if it helps th eissue.
I'm currently trying to solve the issue by removing volatile from the bitop function signatures, but it's grotty because there are quite a few callers to fix up. I'm still trying to do it, because removing volatile fields from structurs is generally a "good thing", but I'd be keen to simplify READ_ONCE() as you suggest regardless. > Maybe add a warning about how gcc < 4.8 might mis-compile the kernel - > those versions are getting close to being unacceptable for kernel > builds anyway. > > We could also look at being stricter for the normal READ/WRITE_ONCE(), > and require that they are > > (a) regular integer types > > (b) fit in an atomic word > > We actually did (b) for a while, until we noticed that we do it on > loff_t's etc and relaxed the rules. But maybe we could have a > "non-atomic" version of READ/WRITE_ONCE() that is used for the > questionable cases? That makes a lot of sense to me, and it would allow us to use compiletime_assert_atomic_type() as we do for the acquire/release accessors. Will
