Received-SPF: pass (google.com: domain of linux-kernel+bounces-179852-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 15 May 2024 05:48:41 -0700
From: Breno Leitao <leitao@debian.org>
To: Marco Elver <elver@google.com>, paulmck@kernel.org
Cc: paulmck@kernel.org, Bart Van Assche <bvanassche@acm.org>,
	Jens Axboe <axboe@kernel.dk>,
	"open list:BLOCK LAYER" <linux-block@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] block: Annotate a racy read in blk_do_io_stat()
Message-ID: <ZkSvKbWj1lhGZvLE@gmail.com>
References: <de92101c-f9c4-4af4-95f4-19a6f59b636f@paulmck-laptop>
 <d037f37a-4722-4a1d-a282-63355a97a1a1@acm.org>
 <c83d9c25-b839-4e31-8dd4-85f3cb938653@paulmck-laptop>
 <4d230bac-bdb0-4a01-8006-e95156965aa8@acm.org>
 <447ad732-3ff8-40bf-bd82-f7be66899cee@paulmck-laptop>
 <ca7c2ef0-7e21-4fb3-ac6b-3dae652a7a0e@acm.org>
 <59ec96c2-52ce-4da1-92c3-9fe38053cd3d@paulmck-laptop>
 <CANpmjNMj9r1V6Z63fcJxrFC1v4i2vUCEhm1HT77ikxhx0Rghdw@mail.gmail.com>
 <dd251dba-0a63-4b57-a05b-bfa02615fae5@paulmck-laptop>
 <CANpmjNMqRUNUs1mZEhrOSyK0Hk+PdGOi+VAs22qYD+1zTkwfhg@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CANpmjNMqRUNUs1mZEhrOSyK0Hk+PdGOi+VAs22qYD+1zTkwfhg@mail.gmail.com>

On Wed, May 15, 2024 at 09:58:35AM +0200, Marco Elver wrote:
> On Wed, 15 May 2024 at 01:47, Paul E. McKenney <paulmck@kernel.org> wrote:
> > On Mon, May 13, 2024 at 10:13:49AM +0200, Marco Elver wrote:
> > +Use of volatile and __data_racy
> > +-------------------------------
> > +
> > +Adding the volatile keyword to the declaration of a variable causes both
> > +the compiler and KCSAN to treat all reads from that variable as if they
> > +were protected by READ_ONCE() and all writes to that variable as if they
> > +were protected by WRITE_ONCE().
 
> "volatile" isn't something we encourage, right? In which case, I think
> to avoid confusion we should not mention volatile. After all we have
> this: Documentation/process/volatile-considered-harmful.rst

Since you mentioned this document, the other day I was reading
volatile-considered-harmful.rst document, and I was surprised that there
is no reference for READ_ONCE() primitive at all (same for WRITE_ONCE).

	# grep -c READ_ONCE Documentation/process/volatile-considered-harmful.rst
	0

From my perspective, READ_ONCE() is another way of doing real memory
read (volatile) when you really need, at the same time keeping the
compiler free to optimize and reuse the value that was read.

Should volatile-considered-harmful.rst be also expanded to cover
READ_ONCE()?

Thanks!

--breno