On 2017-06-17 02:39:40 [+0200], Jason A. Donenfeld wrote: > On Fri, Jun 16, 2017 at 4:35 PM, Sebastian Andrzej Siewior > <bigeasy@xxxxxxxxxxxxx> wrote: > > I wouldn't just push the lock one up as is but move that write part to > > crng_init to remain within the locked section. Like that: > > We can't quite do that, because invalidate_batched_entropy() needs to > be called _before_ crng_init. Otherwise a concurrent call to > get_random_u32/u64() will have crng_init being the wrong value when > the batched entropy is still old. ehm. You sure? I simply delayed the lock-dropping _after_ the state variable was been modified. So it was basically what your patch did except it was unlocked later… > > > Are use about that? I am not sure that the gcc will inline "crng_init" > > read twice. It is not a local variable. READ_ONCE() is usually used > > where gcc could cache a memory access but you do not want this. But hey! > > If someone knows better I am here to learn. > > The whole purpose is that I _want_ it to cache the memory access so > that it is _not_ inlined. So, based on your understanding, it does > exactly what I intended it to do. The reason is that I'd like to avoid > a lock imbalance, which could happen if the read is inlined. So it was good as it was which means you can drop that READ_ONCE(). > Jason Sebastian
