Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752132AbdFSUzm (ORCPT ); Mon, 19 Jun 2017 16:55:42 -0400 Received: from frisell.zx2c4.com ([192.95.5.64]:41327 "EHLO frisell.zx2c4.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751736AbdFSUzl (ORCPT ); Mon, 19 Jun 2017 16:55:41 -0400 MIME-Version: 1.0 In-Reply-To: <20170619074553.wo3ec6i2yaojn7qs@linutronix.de> References: <20170614192838.3jz4sxpcuhxygx4z@breakpoint.cc> <20170614224526.29076-1-Jason@zx2c4.com> <20170616143515.yn6oo6tvmcsrxidw@linutronix.de> <20170619074553.wo3ec6i2yaojn7qs@linutronix.de> From: "Jason A. Donenfeld" Date: Mon, 19 Jun 2017 22:55:37 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] random: silence compiler warnings and fix race To: Sebastian Andrzej Siewior Cc: "Theodore Ts'o" , Linux Crypto Mailing List , LKML , kernel-hardening@lists.openwall.com, Greg Kroah-Hartman , Eric Biggers , Linus Torvalds , David Miller , Thomas Gleixner Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v5JKtlZr008589 Content-Length: 1200 Lines: 25 On Mon, Jun 19, 2017 at 9:45 AM, Sebastian Andrzej Siewior wrote: > 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… Yes, I'm sure. You moved the call to invalidate_batched_entropy() to be after the assignment of crng_init. However, the call to invalidate_batched_entropy() must be made _before_ the assignment of crng_init. >> > 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(). Except READ_ONCE ensures that the compiler will never inline it, so it actually needs to stay.