Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: Guenter Roeck <groeck7@gmail.com>
Date:   Mon, 29 Mar 2021 14:53:39 -0700
From:   Guenter Roeck <linux@roeck-us.net>
To:     Jonas Malaco <jonas@protocubo.io>
Cc:     Jean Delvare <jdelvare@suse.com>, linux-hwmon@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] hwmon: (nzxt-kraken2) mark and order concurrent accesses
Message-ID: <20210329215339.GH220164@roeck-us.net>
References: <20210329082211.86716-1-jonas@protocubo.io>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210329082211.86716-1-jonas@protocubo.io>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Mon, Mar 29, 2021 at 05:22:01AM -0300, Jonas Malaco wrote:
> To avoid a spinlock, the driver explores concurrent memory accesses
> between _raw_event and _read, having the former updating fields on a
> data structure while the latter could be reading from them.  Because
> these are "plain" accesses, those are data races according to the Linux
> kernel memory model (LKMM).
> 
> Data races are undefined behavior in both C11 and LKMM.  In practice,
> the compiler is free to make optimizations assuming there is no data
> race, including load tearing, load fusing and many others,[1] most of
> which could result in corruption of the values reported to user-space.
> 
> Prevent undesirable optimizations to those concurrent accesses by
> marking them with READ_ONCE() and WRITE_ONCE().  This also removes the
> data races, according to the LKMM, because both loads and stores to each
> location are now "marked" accesses.
> 
> As a special case, use smp_load_acquire() and smp_load_release() when
> loading and storing ->updated, as it is used to track the validity of
> the other values, and thus has to be stored after and loaded before
> them.  These imply READ_ONCE()/WRITE_ONCE() but also ensure the desired
> order of memory accesses.
> 
> [1] https://lwn.net/Articles/793253/
> 

I think you lost me a bit there. What out-of-order accesses that would be
triggered by a compiler optimization are you concerned about here ?
The only "problem" I can think of is that priv->updated may have been
written before the actual values. The impact would be ... zero. An
attribute read would return "stale" data for a few microseconds.
Why is that a concern, and what difference does it make ?

Thanks,
Guenter