linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Will Deacon <will@kernel.org>, paulmck <paulmck@kernel.org>,
	Andrea Parri <parri.andrea@gmail.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	Nicholas Piggin <npiggin@gmail.com>,
	David Howells <dhowells@redhat.com>,
	j alglave <j.alglave@ucl.ac.uk>,
	luc maranget <luc.maranget@inria.fr>,
	akiyks@gmail.com, linux-kernel <linux-kernel@vger.kernel.org>,
	linux-toolchains <linux-toolchains@vger.kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>
Subject: Re: [RFC] LKMM: Add volatile_if()
Date: Fri, 24 Sep 2021 16:22:36 -0400 (EDT)	[thread overview]
Message-ID: <1103437077.37428.1632514956401.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <20210924195223.GA287835@rowland.harvard.edu>

----- On Sep 24, 2021, at 3:52 PM, Alan Stern stern@rowland.harvard.edu wrote:

> On Fri, Sep 24, 2021 at 02:38:58PM -0400, Mathieu Desnoyers wrote:
>> Hi,
>> 
>> Following the LPC2021 BoF about control dependency, I re-read the kernel
>> documentation about control dependency, and ended up thinking that what
>> we have now is utterly fragile.
>> 
>> Considering that the goal here is to prevent the compiler from being able to
>> optimize a conditional branch into something which lacks the control
>> dependency, while letting the compiler choose the best conditional
>> branch in each case, how about the following approach ?
>> 
>> #define ctrl_dep_eval(x)        ({ BUILD_BUG_ON(__builtin_constant_p((_Bool)
>> x)); x; })
>> #define ctrl_dep_emit_loop(x)   ({ __label__ l_dummy; l_dummy: asm volatile goto
>> ("" : : : "cc", "memory" : l_dummy); (x); })
>> #define ctrl_dep_if(x)          if ((ctrl_dep_eval(x) && ctrl_dep_emit_loop(1))
>> || ctrl_dep_emit_loop(0))
>> 
>> The idea is to forbid the compiler from considering the two branches as
>> identical by adding a dummy loop in each branch with an empty asm goto.
>> Considering that the compiler should not assume anything about the
>> contents of the asm goto (it's been designed so the generated assembly
>> can be modified at runtime), then the compiler can hardly know whether
>> each branch will trigger an infinite loop or not, which should prevent
>> unwanted optimisations.
>> 
>> With this approach, the following code now keeps the control dependency:
>> 
>> 	z = READ_ONCE(var1);
>>         ctrl_dep_if (z)
>>                 WRITE_ONCE(var2, 5);
>>         else
>>                 WRITE_ONCE(var2, 5);
>> 
>> And the ctrl_dep_eval() checking the constant triggers a build error
>> for:
>> 
>>         y = READ_ONCE(var1);
>>         ctrl_dep_if (y % 1)
>>                 WRITE_ONCE(var2, 5);
>>         else
>>                 WRITE_ONCE(var2, 6);
>> 
>> Which is good to have to ensure the compiler don't end up removing the
>> conditional branch because the resulting evaluation ends up evaluating a
>> constant.
>> 
>> Thoughts ?
> 
> As I remember the earlier discussion, Linus felt that the kernel doesn't
> really need any sort of explicit control dependency (although we called
> it "volatile if").  In many cases there is an actual semantic
> dependency, so it doesn't matter what the compiler does -- the hardware
> will enforce the actual dependency.  In other cases, we can work around
> the issue by using acquire loads or release stores.

IMHO, having to chase down what the instruction selection does on every
architecture for every supported compiler for each control dependency in
order to confirm that the control dependency is still present in the resulting
assembly is fragile. If the kernel really doesn't need explicit control
dependency, then maybe the whole notion of "control dependency"-based
ordering should be removed in favor of explicit acquire loads/release stores
and barriers.

> In fact, Linus's biggest wish was to have a weak form of compiler
> barrier, one which would block the compiler from reordering accesses
> across the barrier but wouldn't invalidate the compiler's knowledge
> about the values of earlier reads (which barrier() would do).

Then maybe we could simply tweak the ctrl_dep_emit_loop() implementation and
remove the "memory" clobber. Then its only effect is to prevent the compiler
from knowing whether there is an infinite loop, thus preventing reordering
accesses across the conditional branch without requiring the compiler to
discard earlier reads:

#define ctrl_dep_emit_loop(x)   ({ __label__ l_dummy; l_dummy: asm_volatile_goto ("" : : : : l_dummy); (x); })

and for the records, the ctrl_dep_eval(x) implementation needs extra parentheses:

#define ctrl_dep_eval(x)        ({ BUILD_BUG_ON(__builtin_constant_p((_Bool) (x))); (x); })

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

  reply	other threads:[~2021-09-24 20:22 UTC|newest]

Thread overview: 127+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-04 10:12 Peter Zijlstra
2021-06-04 10:44 ` Will Deacon
2021-06-04 11:13   ` Will Deacon
2021-06-04 11:31   ` Peter Zijlstra
2021-06-04 13:44     ` Will Deacon
2021-06-04 13:56       ` Peter Zijlstra
2021-06-04 15:13         ` Will Deacon
2021-06-04 15:22           ` Peter Zijlstra
2021-06-04 15:36             ` Alan Stern
2021-06-04 15:42             ` Peter Zijlstra
2021-06-04 15:51               ` Alan Stern
2021-06-04 16:17                 ` Peter Zijlstra
2021-06-04 18:27                   ` Alan Stern
2021-06-04 19:09                     ` Linus Torvalds
2021-06-04 19:18                       ` Linus Torvalds
2021-06-04 20:56                         ` Paul E. McKenney
2021-06-04 21:27                           ` Linus Torvalds
2021-06-04 21:40                             ` Paul E. McKenney
2021-06-04 22:19                               ` Linus Torvalds
2021-06-05 14:57                                 ` Alan Stern
2021-06-06  0:14                                   ` Paul E. McKenney
2021-06-06  1:29                                     ` Alan Stern
2021-06-06  3:41                                       ` Linus Torvalds
2021-06-06  4:43                                         ` Paul E. McKenney
2021-06-06 13:17                                           ` Segher Boessenkool
2021-06-06 19:07                                             ` Paul E. McKenney
2021-06-06 12:59                                         ` Segher Boessenkool
2021-06-06 13:47                                           ` Alan Stern
2021-06-06 17:13                                             ` Segher Boessenkool
2021-06-06 18:25                                           ` Linus Torvalds
2021-06-06 19:19                                             ` Segher Boessenkool
2021-06-06 18:41                                         ` Alan Stern
2021-06-06 18:59                                         ` Jakub Jelinek
2021-06-06 19:15                                           ` Paul E. McKenney
2021-06-06 19:22                                           ` Linus Torvalds
2021-06-06 20:11                                             ` Segher Boessenkool
2021-06-06 21:19                                             ` Alexander Monakov
2021-06-06 22:38                                               ` Linus Torvalds
2021-06-06 23:39                                                 ` Rasmus Villemoes
2021-06-06 23:44                                                   ` Rasmus Villemoes
2021-06-07  8:01                                                 ` Alexander Monakov
2021-06-07  8:27                                                   ` Marco Elver
2021-06-07 15:28                                                     ` Paul E. McKenney
2021-06-07 17:04                                                       ` Marco Elver
2021-06-08  9:30                                                         ` Marco Elver
2021-06-08 11:22                                                           ` Peter Zijlstra
2021-06-08 15:28                                                             ` Segher Boessenkool
2021-06-09 12:44                                                               ` Marco Elver
2021-06-09 15:31                                                                 ` Segher Boessenkool
2021-06-09 16:13                                                                   ` Marco Elver
2021-06-09 17:14                                                                     ` Segher Boessenkool
2021-06-09 17:31                                                                       ` Nick Desaulniers
2021-06-09 20:24                                                                         ` Segher Boessenkool
2021-06-09 18:25                                                                     ` Linus Torvalds
2021-06-07 17:52                                                   ` Segher Boessenkool
2021-06-07 18:07                                                     ` Alexander Monakov
2021-06-07 18:18                                                       ` Segher Boessenkool
2021-06-07 17:42                                                 ` Segher Boessenkool
2021-06-07 20:31                                                   ` Linus Torvalds
2021-06-07 22:54                                                     ` Segher Boessenkool
2021-06-06 11:53                                       ` Segher Boessenkool
2021-06-06 13:45                                         ` Alan Stern
2021-06-06 18:04                                         ` Linus Torvalds
2021-06-06 18:22                                           ` Alan Stern
2021-06-06 18:43                                             ` Linus Torvalds
2021-06-07 10:43                                               ` Peter Zijlstra
2021-06-07 11:52                                                 ` Will Deacon
2021-06-07 15:25                                                   ` Paul E. McKenney
2021-06-07 16:02                                                     ` Will Deacon
2021-06-07 18:08                                                       ` Paul E. McKenney
     [not found]                                                   ` <20210730172020.GA32396@knuckles.cs.ucl.ac.uk>
2021-07-30 20:35                                                     ` Alan Stern
2021-08-02 21:18                                                     ` Alan Stern
2021-08-02 23:31                                                     ` Paul E. McKenney
2021-08-04 20:09                                                       ` Alan Stern
2021-08-05 19:47                                                     ` Alan Stern
2021-08-07  0:51                                                     ` Alan Stern
2021-06-06 18:40                                           ` Segher Boessenkool
2021-06-06 18:48                                             ` Linus Torvalds
2021-06-06 18:53                                               ` Linus Torvalds
2021-06-06 19:52                                               ` Segher Boessenkool
2021-06-06 20:11                                                 ` Linus Torvalds
2021-06-06 20:26                                                   ` Segher Boessenkool
2021-06-06 23:37                                                     ` Paul E. McKenney
2021-06-07 14:12                                                       ` Segher Boessenkool
2021-06-07 15:27                                                         ` Paul E. McKenney
2021-06-07 18:23                                                           ` Segher Boessenkool
2021-06-07 19:51                                                             ` Alan Stern
2021-06-07 20:16                                                               ` Paul E. McKenney
2021-06-07 22:40                                                                 ` Segher Boessenkool
2021-06-07 23:26                                                                   ` Paul E. McKenney
2021-06-07 10:52                                                     ` Peter Zijlstra
2021-06-07 14:16                                                       ` Segher Boessenkool
2021-06-04 22:05                             ` Peter Zijlstra
2021-06-05  3:14                       ` Alan Stern
2021-06-05 16:24                         ` Linus Torvalds
2021-06-04 15:50         ` Segher Boessenkool
2021-06-04 15:47     ` Segher Boessenkool
2021-06-04 11:44 ` Peter Zijlstra
2021-06-04 14:13   ` Paul E. McKenney
2021-06-04 15:35   ` Segher Boessenkool
2021-06-04 16:10     ` Peter Zijlstra
2021-06-04 16:40       ` Segher Boessenkool
2021-06-04 18:55         ` Paul E. McKenney
2021-06-04 19:53           ` Segher Boessenkool
2021-06-04 20:40             ` Paul E. McKenney
2021-06-06 11:36               ` Segher Boessenkool
2021-06-06 19:01                 ` Paul E. McKenney
2021-06-04 14:25 ` Alan Stern
2021-06-04 16:09 ` Segher Boessenkool
2021-06-04 16:33   ` Peter Zijlstra
2021-06-04 16:30 ` Linus Torvalds
2021-06-04 16:37   ` Peter Zijlstra
2021-06-04 16:52     ` Segher Boessenkool
2021-06-04 17:10     ` Linus Torvalds
2021-06-04 17:24       ` Segher Boessenkool
2021-06-04 17:38         ` Linus Torvalds
2021-06-04 18:25           ` Segher Boessenkool
2021-06-04 19:17         ` Peter Zijlstra
2021-06-04 20:43           ` Paul E. McKenney
2021-06-04 18:23       ` Alan Stern
2021-06-08 12:48 ` David Laight
2021-09-24 18:38 ` Mathieu Desnoyers
2021-09-24 19:52   ` Alan Stern
2021-09-24 20:22     ` Mathieu Desnoyers [this message]
2021-09-24 19:55   ` Segher Boessenkool
2021-09-24 20:39     ` Mathieu Desnoyers
2021-09-24 22:07       ` Mathieu Desnoyers

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=1103437077.37428.1632514956401.JavaMail.zimbra@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=akiyks@gmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=j.alglave@ucl.ac.uk \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=luc.maranget@inria.fr \
    --cc=npiggin@gmail.com \
    --cc=parri.andrea@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=stern@rowland.harvard.edu \
    --cc=torvalds@linux-foundation.org \
    --cc=will@kernel.org \
    --subject='Re: [RFC] LKMM: Add volatile_if()' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).