linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Jonas Oberhauser <jonas.oberhauser@huaweicloud.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	linux-kernel@vger.kernel.org, rcu@vger.kernel.org,
	linux-mm@kvack.org, lkmm@lists.linux.dev,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Frederic Weisbecker <frederic@kernel.org>,
	Neeraj Upadhyay <neeraj.upadhyay@kernel.org>,
	Joel Fernandes <joel@joelfernandes.org>,
	Josh Triplett <josh@joshtriplett.org>,
	Uladzislau Rezki <urezki@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Zqiang <qiang.zhang1211@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
	Waiman Long <longman@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Kent Overstreet <kent.overstreet@gmail.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	maged.michael@gmail.com,
	Neeraj Upadhyay <neeraj.upadhyay@amd.com>
Subject: Re: [RFC PATCH 1/4] hazptr: Add initial implementation of hazard pointers
Date: Thu, 26 Sep 2024 18:40:30 +0200	[thread overview]
Message-ID: <54487a36-f74c-46c3-aed7-fc86eaaa9ca2@huaweicloud.com> (raw)
In-Reply-To: <CAHk-=wjL803+FxtAPSGrWqThGQP5cCHzzwZJFq+-fkgt5DQ3VQ@mail.gmail.com>



Am 9/26/2024 um 6:12 PM schrieb Linus Torvalds:
> On Thu, 26 Sept 2024 at 08:54, Jonas Oberhauser
> <jonas.oberhauser@huaweicloud.com> wrote:
>>
>> No, the issue introduced by the compiler optimization (or by your
>> original patch) is that the CPU can speculatively load from the first
>> pointer as soon as it has completed the load of that pointer:
> 
> You mean the compiler can do it.

What I mean is that if we only use rcu_dereference for the second load 
(and not either some form of compiler barrier or an acquire load), then 
the compiler can transform the second program from my previous e-mail 
(which if mapped 1:1 to hardware would be correct because hardware 
ensures the ordering based on the address dependency) into the first one 
(which is incorrect).

In particular, the compiler can change

  if (node == node2) t = *node2;

into

  if (node == node2) t = *node;

and then the CPU can speculatively read *node before knowing the value 
of node2.

The compiler can also speculatively read *node in this case, but that is 
not what I meant.

The code in Mathieu's original patch is already like the latter one and 
is broken even if the compiler does not do any optimizations.


> The inline asm has no impact on what
> the CPU does. The conditional isn't a barrier for the actual hardware.
> But once the compiler doesn't try to do it, the data dependency on the
> address does end up being an ordering constraint on the hardware too

Exactly. The inline asm would prevent the compiler from doing the 
transformation though, which would mean that the address dependency 
appears in the final compiler output.

> Just use a barrier.  Or make sure to use the proper ordered memory
> accesses when possible. 
 >
> Don't use an inline asm for the compare - we
> don't even have anything insane like that as a portable helper, and we
> shouldn't have it.

I'm glad you say that :))

I would also just use a barrier before returing the pointer.

Boqun seems to be unhappy with a barrier though, because it would 
theoretically also forbid unrelated optimizations.
But I have not seen any evidence that there are any unrelated 
optimizations going on in the first place that would be forbidden by this.

Have fun,
   jonas



  reply	other threads:[~2024-09-26 16:41 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-17 14:33 [RFC PATCH 0/4] Add hazard pointers to kernel Boqun Feng
2024-09-17 14:33 ` [RFC PATCH 1/4] hazptr: Add initial implementation of hazard pointers Boqun Feng
2024-09-18  8:27   ` Mathieu Desnoyers
2024-09-18 15:17   ` Alan Huang
2024-09-19  6:56     ` Boqun Feng
2024-09-19 18:07       ` Jonas Oberhauser
2024-09-19  0:12   ` Jann Horn
2024-09-19 20:30     ` Jonas Oberhauser
2024-09-20  7:43       ` Jonas Oberhauser
2024-09-19  6:39   ` Lai Jiangshan
2024-09-19  7:10     ` Boqun Feng
2024-09-19 12:33       ` Alan Huang
2024-09-19 13:57       ` Alan Huang
2024-09-19 18:58         ` Boqun Feng
2024-09-19 19:53           ` Alan Huang
2024-09-19 16:10       ` Alan Huang
2024-09-19 14:00   ` Jonas Oberhauser
2024-09-20  7:41   ` Jonas Oberhauser
2024-09-25 10:02     ` Boqun Feng
2024-09-25 10:11       ` Jonas Oberhauser
2024-09-25 10:45         ` Boqun Feng
2024-09-25 11:59           ` Mathieu Desnoyers
2024-09-25 12:16             ` Boqun Feng
2024-09-25 12:47               ` Mathieu Desnoyers
2024-09-25 13:10                 ` Mathieu Desnoyers
2024-09-25 13:20                   ` Mathieu Desnoyers
2024-09-26  6:16                     ` Mathieu Desnoyers
2024-09-26 15:53                       ` Jonas Oberhauser
2024-09-26 16:12                         ` Linus Torvalds
2024-09-26 16:40                           ` Jonas Oberhauser [this message]
2024-09-26 16:54                             ` Linus Torvalds
2024-09-27  0:01                               ` Boqun Feng
2024-09-27  1:30                                 ` Mathieu Desnoyers
2024-09-27  1:37                                   ` Boqun Feng
2024-09-27  4:28                                     ` Boqun Feng
2024-09-27 10:59                                       ` Mathieu Desnoyers
2024-09-27 14:43                                         ` Mathieu Desnoyers
2024-09-27 15:22                                           ` Mathieu Desnoyers
2024-09-27 16:06                                       ` Alan Huang
2024-09-27 16:44                                     ` Linus Torvalds
2024-09-27 17:15                                       ` Mathieu Desnoyers
2024-09-27 17:23                                         ` Linus Torvalds
2024-09-27 17:51                                           ` Mathieu Desnoyers
2024-09-27 18:13                                             ` Linus Torvalds
2024-09-27 19:12                                               ` Jonas Oberhauser
2024-09-27 19:28                                                 ` Linus Torvalds
2024-09-27 20:24                                                   ` Linus Torvalds
2024-09-27 20:02                                               ` Mathieu Desnoyers
2024-09-27  1:20                           ` Mathieu Desnoyers
2024-09-27  4:38                             ` Boqun Feng
2024-09-27 19:23                               ` Jonas Oberhauser
2024-09-27 20:10                                 ` Mathieu Desnoyers
2024-09-27 22:18                                   ` Jonas Oberhauser
2024-09-28 22:10                                     ` Alan Huang
2024-09-28 23:12                                       ` Alan Huang
2024-09-25 12:19             ` Jonas Oberhauser
2024-09-17 14:34 ` [RFC PATCH 2/4] refscale: Add benchmarks for hazptr Boqun Feng
2024-09-17 14:34 ` [RFC PATCH 3/4] refscale: Add benchmarks for percpu_ref Boqun Feng
2024-09-17 14:34 ` [RFC PATCH 4/4] WIP: hazptr: Add hazptr test sample Boqun Feng
2024-09-18  7:18 ` [RFC PATCH 0/4] Add hazard pointers to kernel Linus Torvalds
2024-09-18 22:44   ` Neeraj Upadhyay
2024-09-19  6:46     ` Linus Torvalds
2024-09-20  5:00       ` Neeraj Upadhyay
2024-09-19 14:30     ` Mateusz Guzik
2024-09-19 14:14   ` Christoph Hellwig
2024-09-19 14:21     ` Linus Torvalds

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=54487a36-f74c-46c3-aed7-fc86eaaa9ca2@huaweicloud.com \
    --to=jonas.oberhauser@huaweicloud.com \
    --cc=boqun.feng@gmail.com \
    --cc=frederic@kernel.org \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=kent.overstreet@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lkmm@lists.linux.dev \
    --cc=longman@redhat.com \
    --cc=maged.michael@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@redhat.com \
    --cc=neeraj.upadhyay@amd.com \
    --cc=neeraj.upadhyay@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=qiang.zhang1211@gmail.com \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=urezki@gmail.com \
    --cc=vbabka@suse.cz \
    --cc=will@kernel.org \
    /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