Received-SPF: pass (google.com: domain of linux-kernel+bounces-94478-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 6 Mar 2024 14:27:38 -0500
From: Steven Rostedt <rostedt@goodmis.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>, linke li <lilinke99@qq.com>,
 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,
 qiang.zhang1211@gmail.com, quic_neeraju@quicinc.com, rcu@vger.kernel.org
Subject: Re: [PATCH] rcutorture: Fix
 rcu_torture_pipe_update_one()/rcu_torture_writer() data race and
 concurrency bug
Message-ID: <20240306142738.7b66a716@rorschach.local.home>
In-Reply-To: <CAHk-=wjbDgMKLgxbV+yK4LKZ+2Qj6zVL_sHeb+L9KDia980Q8Q@mail.gmail.com>
References: <f3624f39-bbb1-451d-8161-8518e4108d8e@joelfernandes.org>
	<tencent_9882B228F292088CDD68F10CF1C228742009@qq.com>
	<20240306103719.1d241b93@gandalf.local.home>
	<27665890-8314-4252-8622-1e019fee27e4@paulmck-laptop>
	<20240306130103.6da71ddf@gandalf.local.home>
	<CAHk-=wgG6Dmt1JTXDbrbXh_6s2yLjL=9pHo7uv0==LHFD+aBtg@mail.gmail.com>
	<20240306135504.2b3872ef@gandalf.local.home>
	<CAHk-=wjbDgMKLgxbV+yK4LKZ+2Qj6zVL_sHeb+L9KDia980Q8Q@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Wed, 6 Mar 2024 11:01:55 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, 6 Mar 2024 at 10:53, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > Now, are you OK with an addition of ADD_ONCE() and/or INC_ONCE()? So that we
> > don't have to look at:
> >
> >         WRITE_ONCE(a, READ_ONCE(a) + 1);
> >
> > ?  
> 
> In a generic header file under include/linux/?
> 
> Absolutely not. The above is a completely broken operation. There is
> no way in hell we should expose it as a general helper.
> 
> So there is no way we'd add that kind of sh*t-for-brains operation in
> (for example) our <asm/rwonce.h> header file next to the normal
> READ/WRITE_ONCE defines.
> 
> In some individual tracing C file where it has a comment above it how
> it's braindamaged and unsafe and talking about why it's ok in that
> particular context? Go wild.

Note this has nothing to do with tracing. This thread is in RCU. I just
happen to receive the same patch "fix" for my code.

> 
> But honestly, I do not see when a ADD_ONCE() would ever be a valid
> thing to do, and *if* it's a valid thing to do, why you'd do it with
> READ_ONCE and WRITE_ONCE.
> 
> If you don't care about races, just do a simple "++" and be done with
> it. The end result is random.

That was my feeling. But when I saw this going into RCU, I was thinking
there was a more fundamental problem here.

> 
> Adding a "ADD_ONCE()" macro doesn't make it one whit less random. It
> just makes a broken concept even uglier.
> 
> So honestly, I think the ADD_ONCE macro not only needs to be in some
> tracing-specific C file, the comment needs to be pretty damn big too.
> Because as a random number generator, it's not even a very good one.
> So you need to explain *why* you want a particularly bad random number
> generator in the first place.

Again, this has nothing to do with tracing. The code here is solely in
RCU. I did receive a patch in the tracing code, but that had to deal
with wakeups of readers with respect to writers which is a common thing
across the kernel and is not anything tracing specific.

I wasn't about to take the patch to my code, but when I saw the same
changes in RCU, then I thought this might be something I need to worry
about.

-- Steve