lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 16 May 2016 12:55:57 +0200 From: Peter Zijlstra <peterz@...radead.org> To: Thomas Gleixner <tglx@...utronix.de> Cc: Vikram Mulukutla <markivx@...eaurora.org>, linux-kernel@...r.kernel.org Subject: Re: Additional compiler barrier required in sched_preempt_enable_no_resched? On Sat, May 14, 2016 at 05:39:37PM +0200, Thomas Gleixner wrote: > I have a hard time to understand why the compiler optimizes out stuff w/o that > patch. > > We already have: > > #define preempt_disable() \ > do { \ > preempt_count_inc(); \ > barrier(); \ > } while (0) > > #define sched_preempt_enable_no_resched() \ > do { \ > barrier(); \ > preempt_count_dec(); \ > } while (0) > > #define preempt_enable() \ > do { \ > barrier(); \ > if (unlikely(preempt_count_dec_and_test())) \ > __preempt_schedule(); \ > } while (0) > > So the barriers already forbid that the compiler reorders code. How on earth > is the compiler supposed to optimize the dec/inc out? Order things like: > #define sched_preempt_enable_no_resched() \ > do { \ > barrier(); \ > preempt_count_dec(); \ > } while (0) > #define preempt_disable() \ > do { \ > preempt_count_inc(); \ > barrier(); \ > } while (0) And there is no barrier between the dec and inc, and a smarty pants compiler could just decide to forgo the update, since in program order there is no observable difference either way. Making the thing volatile tells the compiler there can be external observations of the memory and it cannot assume things like that and must emit the operations. You're right in that the 'proper' sequence: > #define preempt_enable() \ > do { \ > barrier(); \ > if (unlikely(preempt_count_dec_and_test())) \ > __preempt_schedule(); \ > } while (0) > #define preempt_disable() \ > do { \ > preempt_count_inc(); \ > barrier(); \ > } while (0) Has a higher chance of succeeding to emit the operations to memory; but an even smarter pants compiler might figure doing something like: if (preempt_count() == 1) __preempt_schedule(); is equivalent and emits that instead, not bothering to modify the actual variable at all -- the program as specified cannot tell the difference etc.. Also; in the case of !PREEMPT && PREEMPT_COUNT, the normal: preempt_disable(); preempt_enable(); sequence turns into the first case again. So I'll go write a proper changelog for the volatile thing and get it merged with a Cc to stable.
Powered by blists - more mailing lists