linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Alan Huang <mmpgouride@gmail.com>
To: Jonas Oberhauser <jonas.oberhauser@huaweicloud.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Will Deacon <will@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	John Stultz <jstultz@google.com>,
	Neeraj upadhyay <Neeraj.Upadhyay@amd.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	Joel Fernandes <joel@joelfernandes.org>,
	Josh Triplett <josh@joshtriplett.org>,
	"Uladzislau Rezki (Sony)" <urezki@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Zqiang <qiang.zhang1211@gmail.com>,
	Ingo Molnar <mingo@redhat.com>, Waiman Long <longman@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vlastimil Babka <vbabka@suse.cz>,
	maged.michael@gmail.com, Mateusz Guzik <mjguzik@gmail.com>,
	Gary Guo <gary@garyguo.net>, RCU <rcu@vger.kernel.org>,
	linux-mm@kvack.org, lkmm@lists.linux.dev
Subject: Re: [PATCH 1/2] compiler.h: Introduce ptr_eq() to preserve address dependency
Date: Mon, 30 Sep 2024 17:27:45 +0800	[thread overview]
Message-ID: <E2551572-494D-46A0-A1D5-DF91DDB8B9F9@gmail.com> (raw)
In-Reply-To: <EEAE4623-DCD7-4BBE-8E06-7F6FF6E3070B@gmail.com>

2024年9月30日 17:15,Alan Huang <mmpgouride@gmail.com> 写道:
> 
> 2024年9月30日 16:57,Jonas Oberhauser <jonas.oberhauser@huaweicloud.com> 写道:
>> 
>> 
>> 
>> Am 9/29/2024 um 12:26 AM schrieb Alan Huang:
>>> 2024年9月28日 23:55,Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>>>> 
>>>> On 2024-09-28 17:49, Alan Stern wrote:
>>>>> On Sat, Sep 28, 2024 at 11:32:18AM -0400, Mathieu Desnoyers wrote:
>>>>>> On 2024-09-28 16:49, Alan Stern wrote:
>>>>>>> On Sat, Sep 28, 2024 at 09:51:27AM -0400, Mathieu Desnoyers wrote:
>>>>>>>> equality, which does not preserve address dependencies and allows the
>>>>>>>> following misordering speculations:
>>>>>>>> 
>>>>>>>> - If @b is a constant, the compiler can issue the loads which depend
>>>>>>>>   on @a before loading @a.
>>>>>>>> - If @b is a register populated by a prior load, weakly-ordered
>>>>>>>>   CPUs can speculate loads which depend on @a before loading @a.
>>>>>>> 
>>>>>>> It shouldn't matter whether @a and @b are constants, registers, or
>>>>>>> anything else.  All that matters is that the compiler uses the wrong
>>>>>>> one, which allows weakly ordered CPUs to speculate loads you wouldn't
>>>>>>> expect it to, based on the source code alone.
>>>>>> 
>>>>>> I only partially agree here.
>>>>>> 
>>>>>> On weakly-ordered architectures, indeed we don't care whether the
>>>>>> issue is caused by the compiler reordering the code (constant)
>>>>>> or the CPU speculating the load (registers).
>>>>>> 
>>>>>> However, on strongly-ordered architectures, AFAIU, only the constant
>>>>>> case is problematic (compiler reordering the dependent load), because
>>>>> I thought you were trying to prevent the compiler from using one pointer
>>>>> instead of the other, not trying to prevent it from reordering anything.
>>>>> Isn't this the point the documentation wants to get across when it says
>>>>> that comparing pointers can be dangerous?
>>>> 
>>>> The motivation for introducing ptr_eq() is indeed because the
>>>> compiler barrier is not sufficient to prevent the compiler from
>>>> using one pointer instead of the other.
>>> barrier_data(&b) prevents that.
>> 
>> I don't think one barrier_data can garantuee preventing this, because right after doing the comparison, the compiler still could do b=a.
>> 
>> In that case you would be guaranteed to use the value in b, but that value is not the value loaded into b originally but rather the value loaded into a, and hence your address dependency goes to the wrong load still.
> 
> After barrier_data(&b), *b will be loaded from memory, you mean even if *b is loaded from memory, the address dependency goes to the wrong load still?

Sorry, *b should b.

> 
>> 
>> However, doing
>> 
>> barrier_data(&b);
>> if (a == b) {
>>  barrier();
>>  foo(*b);
>> }
>> 
>> might maybe prevent it, because after the address of b is escaped, the compiler might no longer be allowed to just do b=a;, but I'm not sure if that is completely correct, since the compiler knows b==a and no other thread can be concurrently modifying a or b. Therefore, given that the compiler knows the hardware, it might know that assigning b=a would not cause any  race-related issues even if another thread was reading b concurrently.
>> 
>> Finally, it may be only a combination of barrier_data and making b volatile could be guaranteed to solve the issue, but the code will be very obscure compared to using ptr_eq.
>> 
>> jonas




  reply	other threads:[~2024-09-30  9:28 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-28 13:51 [PATCH 0/2] " Mathieu Desnoyers
2024-09-28 13:51 ` [PATCH 1/2] compiler.h: " Mathieu Desnoyers
2024-09-28 14:49   ` Alan Stern
2024-09-28 15:30     ` Mathieu Desnoyers
2024-09-28 15:32     ` Mathieu Desnoyers
2024-09-28 15:49       ` Alan Stern
2024-09-28 15:55         ` Mathieu Desnoyers
2024-09-28 21:15           ` Alan Stern
2024-09-30  9:42             ` Jonas Oberhauser
2024-09-30 11:04               ` Paul E. McKenney
2024-09-30 12:06                 ` Jonas Oberhauser
2024-09-30 13:54                   ` Paul E. McKenney
2024-09-28 22:26           ` Alan Huang
2024-09-28 23:55             ` Boqun Feng
2024-09-29  0:20               ` Alan Huang
2024-09-30  8:57             ` Jonas Oberhauser
2024-09-30  9:15               ` Alan Huang
2024-09-30  9:27                 ` Alan Huang [this message]
2024-09-30  9:33                   ` Jonas Oberhauser
2024-09-30 10:12                     ` Alan Huang
2024-09-30 11:26     ` Jonas Oberhauser
2024-09-30 16:43       ` Alan Stern
2024-09-30 17:05         ` Jonas Oberhauser
2024-09-30 18:53           ` Alan Stern
2024-10-01 17:11             ` David Laight
2024-10-01 22:57               ` 'Alan Stern'
2024-10-02  8:13                 ` David Laight
2024-10-02 14:14                   ` 'Alan Stern'
2024-10-02 15:24                     ` David Laight
2024-10-03  1:50                       ` 'Alan Stern'
2024-10-03 13:23                         ` Mathieu Desnoyers
2024-10-03 17:07                           ` David Laight
2024-10-03 18:00                             ` Mathieu Desnoyers
2024-10-07 11:54                           ` Jonas Oberhauser
2024-10-07 13:18                             ` David Laight
2024-10-07 13:21                               ` Mathieu Desnoyers
2024-10-07 14:59                               ` Jonas Oberhauser
2024-09-28 23:24   ` Gary Guo
2024-09-29 10:36     ` Mathieu Desnoyers
2024-09-28 13:51 ` [PATCH 2/2] Documentation: RCU: Refer to ptr_eq() Mathieu Desnoyers
2024-09-28 14:58   ` Alan Stern
2024-09-28 15:09     ` 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=E2551572-494D-46A0-A1D5-DF91DDB8B9F9@gmail.com \
    --to=mmpgouride@gmail.com \
    --cc=Neeraj.Upadhyay@amd.com \
    --cc=bigeasy@linutronix.de \
    --cc=boqun.feng@gmail.com \
    --cc=frederic@kernel.org \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=jonas.oberhauser@huaweicloud.com \
    --cc=josh@joshtriplett.org \
    --cc=jstultz@google.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=mjguzik@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=qiang.zhang1211@gmail.com \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=stern@rowland.harvard.edu \
    --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