mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Michael Matz <matz@suse.de>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>,
	Frederic Weisbecker <frederic@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Joel Fernandes <joel@joelfernandes.org>,
	Josh Triplett <josh@joshtriplett.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Neeraj Upadhyay <neeraj.upadhyay@amd.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Uladzislau Rezki <urezki@gmail.com>, rcu <rcu@vger.kernel.org>,
	Zqiang <qiang.zhang1211@gmail.com>,
	"Liam R . Howlett" <Liam.Howlett@oracle.com>,
	ubizjak@gmail.com
Subject: Re: [PATCH 2/4] rcu/tasks: Handle new PF_IDLE semantics
Date: Tue, 31 Oct 2023 14:16:34 +0000 (UTC)	[thread overview]
Message-ID: <alpine.LSU.2.20.2310311357450.15233@wotan.suse.de> (raw)
In-Reply-To: <20231031095202.GC35651@noisy.programming.kicks-ass.net>

Hello,

On Tue, 31 Oct 2023, Peter Zijlstra wrote:

(I can't say anything about the WRITE_ONCE/rcu code, just about the below 
codegen part)

> Welcome is not the right word. What bugs me most is that this was never
> raised when this code was written :/
> 
> Mostly my problem is that GCC generates such utter shite when you
> mention volatile. See, the below patch changes the perfectly fine and
> non-broken:
> 
> 0148  1d8:      49 83 06 01             addq   $0x1,(%r14)

What is non-broken here that is ...

> into:
> 
> 0148  1d8:	49 8b 06             	mov    (%r14),%rax
> 014b  1db:	48 83 c0 01          	add    $0x1,%rax
> 014f  1df:	49 89 06             	mov    %rax,(%r14)

... broken here?  (Sure code size and additional register use, but I don't 
think you mean this with broken).

> For absolutely no reason :-(

The reason is simple (and should be obvious): to adhere to the abstract 
machine regarding volatile.  When x is volatile then x++ consists of a 
read and a write, in this order.  The easiest way to ensure this is to 
actually generate a read and a write instruction.  Anything else is an 
optimization, and for each such optimization you need to actively find an 
argument why this optimization is correct to start with (and then if it's 
an optimization at all).  In this case the argument needs to somehow 
involve arguing that an rmw instruction on x86 is in fact completely 
equivalent to the separate instructions, from read cycle to write cycle 
over all pipeline stages, on all implementations of x86.  I.e. that a rmw 
instruction is spec'ed to be equivalent.

You most probably can make that argument in this specific case, I'll give 
you that.  But why bother to start with, in a piece of software that is 
already fairly complex (the compiler)?  It's much easier to just not do 
much anything with volatile accesses at all and be guaranteed correct.
Even more so as the software author, when using volatile, most likely is 
much more interested in correct code (even from a abstract machine 
perspective) than micro optimizations.

> At least clang doesn't do this, it stays:
> 
> 0403  413:      49 ff 45 00             incq   0x0(%r13)
> 
> irrespective of the volatile.

And, are you 100% sure that this is correct?  Even for x86 CPU 
pipeline implementations that you aren't intimately knowing about? ;-)

But all that seems to be a side-track anyway, what's your real worry with  
the code sequence generated by GCC?


Ciao,
Michael.

  reply	other threads:[~2023-10-31 14:16 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20231027144050.110601-1-frederic@kernel.org>
     [not found] ` <20231027144050.110601-3-frederic@kernel.org>
2023-10-27 19:20   ` Peter Zijlstra
2023-10-27 21:23     ` Paul E. McKenney
2023-10-27 22:46       ` Peter Zijlstra
2023-10-27 23:41         ` Paul E. McKenney
2023-10-30  8:21           ` Peter Zijlstra
2023-10-30 20:11             ` Paul E. McKenney
2023-10-31  9:52               ` Peter Zijlstra
2023-10-31 14:16                 ` Michael Matz [this message]
2023-10-31 14:43                   ` Paul E. McKenney
2023-10-31 15:16                   ` Peter Zijlstra
2023-10-31 15:55                     ` Michael Matz
2023-10-31 16:23                       ` Peter Zijlstra
2023-10-31 16:49                         ` Michael Matz
2023-10-31 17:10                           ` Paul E. McKenney
2023-10-31 14:24                 ` Paul E. McKenney
2023-10-31 15:20                   ` Peter Zijlstra
2023-10-31 18:12                     ` Paul E. McKenney
2023-11-15  9:04                       ` [tip: sched/core] sched: Use WRITE_ONCE() for p->on_rq tip-bot2 for Paul E. McKenney
2023-10-27 19:23 ` [PATCH 0/4] rcu: Fix PF_IDLE related issues v3 Peter Zijlstra
2023-10-24 21:46 [PATCH 0/4] rcu: Fix PF_IDLE related issues v2 Frederic Weisbecker
2023-10-24 21:46 ` [PATCH 2/4] rcu/tasks: Handle new PF_IDLE semantics Frederic Weisbecker
2023-10-25  8:40   ` Peter Zijlstra
2023-10-25 10:31     ` Frederic Weisbecker
2023-10-26 12:15       ` Z qiang
2023-10-26 14:34         ` Frederic Weisbecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LSU.2.20.2310311357450.15233@wotan.suse.de \
    --to=matz@suse.de \
    --cc=Liam.Howlett@oracle.com \
    --cc=boqun.feng@gmail.com \
    --cc=frederic@kernel.org \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=neeraj.upadhyay@amd.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=qiang.zhang1211@gmail.com \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=ubizjak@gmail.com \
    --cc=urezki@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®