Date: Mon, 8 Jan 2018 19:30:21 +0100
From: Andrea Arcangeli <aarcange@redhat.com>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: tim.c.chen@linux.intel.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable
 SPEC_CTRL feature
Message-ID: <20180108183021.GE4703@redhat.com>
References: <20180108174340.GA7310@avx2>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180108174340.GA7310@avx2>
User-Agent: Mutt/1.9.2 (2017-12-15)
Sender: linux-kernel-owner@vger.kernel.org

On Mon, Jan 08, 2018 at 08:43:40PM +0300, Alexey Dobriyan wrote:
> > +	len = sprintf(buf, "%d\n", READ_ONCE(*field));
> 
> READ_ONCE isn't necessary as there is only one read being made.

Others might disagree but you shouldn't ever let gcc touch any memory
that can change under gcc without READ_ONCE or volatile.

Ages ago I was told in a switch() statement gcc could decide to use an
hash and re-read the value and crash.

Not for the simple case above, and we often don't use READ_ONCE and we
might user barrier() instead, but it would be better to use READ_ONCE.

READ_ONCE is more correct and there's no point to try to optimize it
especially if there's only one read being made in that function.

Either version in practice will work, but READ_ONCE is preferable IMHO.

Thanks,
Andrea