On Wed, Feb 27, 2019 at 8:48 AM Peter Zijlstra <pet...@infradead.org> wrote: > > On Wed, Feb 27, 2019 at 08:14:09AM -0800, Linus Torvalds wrote: > > In particular, making it "asm volatile" really is a big hammer > > approach. It's worth noting that the *other* this_cpu_xyz ops don't > > even do that. > > Right, this patch 'fixes' that :-)
Yeah, but I do hate seeing these patches that randomly add double underscore versions just because the regular one is so bad. So I'd much rather improve the regular this_cpu_read() instead, and hope that we don't need to have this kind of big difference between that and the double-underscore version. I'm not convinced that the "asm volatile" is right on the _other_ ops either. You added them to cmpxchg and xadd too, and it's not obvious that they should have them. They have the "memory" clobber to order them wrt actual memory ops, why are they now "asm volatile"? So I don't really like this patch that randomly adds volatile to things, and then removes it from one special case that I don't think should have had it in the first place. It all seems pretty ad-hoc, and we already _know_ that "asm volatile" is bad. Linus