On Tue, Jun 06, 2017 at 06:15:51PM +0200, Peter Zijlstra wrote: > On Tue, Jun 06, 2017 at 05:27:06PM +0200, Heiko Carstens wrote: > > On Tue, Jun 06, 2017 at 04:45:57PM +0200, Christian Borntraeger wrote: > > > > As a side note, I am asking myself, though, why we do need the > > > preempt_disable/enable for the cases where we use the opcodes > > > like lao (atomic load and or to a memory location) and friends. > > > > Because you want the atomic instruction to be executed on the local cpu for > > which you have to per cpu pointer. If you get preempted to a different cpu > > between the ptr__ assignment and lan instruction it might be executed not > > on the local cpu. It's not really a correctness issue. > > As per the previous email, I think it is a correctness issue wrt CPU > hotplug. Yes, I realized that just a minute after I sent the above. > > However in reality it doesn't matter at all, since all distributions we > > care about have preemption disabled. > > Well, either you support PREEMPT=y or you don't :-) If you do, it needs > to be correct, irrespective of what distro's do with it. That is true. Our s390 specific percpu ops are supposed to be correct for PREEMPT=y, and that's apparently the only reason why I added the preempt disable/enable pairs back then. I just had to remember why I did that ;) > > So this_cpu_inc() should just generate three instructions: two to calculate > > the percpu pointer and an additional asi for the atomic increment, with > > operand specific serialization. This is supposed to be a lot faster than > > disabling/enabling interrupts around a non-atomic operation. > > So typically we joke about s390 that it has an instruction for this > 'very-complicated-thing', but here you guys do not, what gives? ;-) Tough luck. :)
