Received-SPF: pass (google.com: domain of linux-kernel+bounces-94163-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Wed, 6 Mar 2024 10:37:19 -0500
From: Steven Rostedt <rostedt@goodmis.org>
To: linke li <lilinke99@qq.com>
Cc: joel@joelfernandes.org, boqun.feng@gmail.com, dave@stgolabs.net,
 frederic@kernel.org, jiangshanlai@gmail.com, josh@joshtriplett.org,
 linux-kernel@vger.kernel.org, mathieu.desnoyers@efficios.com,
 paulmck@kernel.org, qiang.zhang1211@gmail.com, quic_neeraju@quicinc.com,
 rcu@vger.kernel.org, Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH] rcutorture: Fix
 rcu_torture_pipe_update_one()/rcu_torture_writer() data race and
 concurrency bug
Message-ID: <20240306103719.1d241b93@gandalf.local.home>
In-Reply-To: <tencent_9882B228F292088CDD68F10CF1C228742009@qq.com>
References: <f3624f39-bbb1-451d-8161-8518e4108d8e@joelfernandes.org>
	<tencent_9882B228F292088CDD68F10CF1C228742009@qq.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Tue,  5 Mar 2024 14:24:20 +0800
linke li <lilinke99@qq.com> wrote:

> > Anyway, a slightly related/different question:
> > 
> > Should that:
> > WRITE_ONCE(rp->rtort_pipe_count, rp->rtort_pipe_count + 1);
> > 
> > Be:
> > WRITE_ONCE(rp->rtort_pipe_count, READ_ONCE(rp->rtort_pipe_count) + 1);
> > 
> > ?  
> 
> Hi, Joel. Sorry, I am not very sure about this. I do a little research on
> it.
> 
> I looked through all code in kernel that looks like this, both kinds are
> used. I also try to compile this file with and without the READ_ONCE in
> WRITE_ONCE using gcc-9. Their binary is just the same. 
> 
> Thus I think in the current compiler version, they do not have a big
> difference, but if the compiler wants to optimize more, maybe the latter
> one is better.

I'm sorry but all these READ_ONCE/WRITE_ONCE() additions that are being
added because there's a fear of the compiler tearing long words, is going to
make the code really ugly and hard to read.

If we take the policy of handling a compiler that can tear reads and writes
of any size word, then we should have proper macros to handle it.

Perhaps READ_SHARED(), WRITE_SHARED(), ADD_SHARED(), SUB_SHARED(). The ONCE
has nothing to do with the reasons for these changes. But at least "SHARED"
can be considered "this variable is shared between different contexts".
Note, this is different than "atomic". It's just to document that this
variable must be loaded or stored in one transaction.

I don't know if Linus even cares about fixing "read/write tearing" which is
why I Cc'd him.

But I'm not going to take any patches that add these macros to fix
compilers that tear words on load and store until we have a set policy on
what to do with them.

-- Steve