From: Waiman Long <waiman.long@hpe.com>
To: Jason Low <jason.low2@hpe.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>, <linux-kernel@vger.kernel.org>,
Dave Chinner <david@fromorbit.com>,
Peter Hurley <peter@hurleysoftware.com>,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Scott J Norton <scott.norton@hpe.com>,
Douglas Hatch <doug.hatch@hpe.com>, <jason.low2@hp.com>
Subject: Re: [PATCH v4 2/5] locking/rwsem: Protect all writes to owner by WRITE_ONCE
Date: Wed, 18 May 2016 14:29:42 -0400 [thread overview]
Message-ID: <573CB496.4010707@hpe.com> (raw)
In-Reply-To: <1463592095.3369.10.camel@j-VirtualBox>
On 05/18/2016 01:21 PM, Jason Low wrote:
> On Wed, 2016-05-18 at 07:04 -0700, Davidlohr Bueso wrote:
>> On Tue, 17 May 2016, Waiman Long wrote:
>>
>>> Without using WRITE_ONCE(), the compiler can potentially break a
>>> write into multiple smaller ones (store tearing). So a read from the
>>> same data by another task concurrently may return a partial result.
>>> This can result in a kernel crash if the data is a memory address
>>> that is being dereferenced.
>>>
>>> This patch changes all write to rwsem->owner to use WRITE_ONCE()
>>> to make sure that store tearing will not happen. READ_ONCE() may
>>> not be needed for rwsem->owner as long as the value is only used for
>>> comparison and not dereferencing.
> It might be okay to leave out READ_ONCE() for reading rwsem->owner, but
> couldn't we include it to at least document that we're performing a
> "special" lockless read?
>
Using READ_ONCE() does have a bit of cost as it limits compiler
optimization. If we changes all access to rwsem->owner to READ_ONCE()
and WRITE_ONCE(), we may as well change its type to volatile and be done
with. I am not against doing that, but it feels a bit over-reach for me.
On the other hand, we may define a do-nothing macro that designates the
owner as a special variable for documentation purpose, but don't need
protection at that particular call site.
Cheers,
Longman
next prev parent reply other threads:[~2016-05-18 18:29 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-18 1:26 [PATCH v4 0/5] [PATCH v3 0/4] locking/rwsem: Add reader-owned state to the owner field Waiman Long
2016-05-18 1:26 ` [PATCH v4 1/5] " Waiman Long
2016-06-06 17:18 ` Davidlohr Bueso
2016-06-06 20:03 ` Waiman Long
2016-06-06 21:02 ` Peter Zijlstra
2016-06-06 21:49 ` Waiman Long
2016-06-08 14:25 ` [tip:locking/core] " tip-bot for Waiman Long
2016-05-18 1:26 ` [PATCH v4 2/5] locking/rwsem: Protect all writes to owner by WRITE_ONCE() Waiman Long
2016-05-18 14:04 ` Davidlohr Bueso
2016-05-18 17:21 ` [PATCH v4 2/5] locking/rwsem: Protect all writes to owner by WRITE_ONCE Jason Low
2016-05-18 18:29 ` Waiman Long [this message]
2016-05-18 19:58 ` Jason Low
2016-05-19 22:21 ` Jason Low
2016-05-20 20:26 ` Waiman Long
2016-05-21 16:04 ` Peter Hurley
2016-05-22 10:42 ` Peter Zijlstra
2016-05-23 18:46 ` Jason Low
2016-05-23 19:44 ` Davidlohr Bueso
2016-05-23 20:15 ` Paul E. McKenney
2016-05-23 21:04 ` Davidlohr Bueso
2016-05-25 1:25 ` Waiman Long
2016-05-18 17:23 ` [PATCH v4 2/5] locking/rwsem: Protect all writes to owner by WRITE_ONCE() Jason Low
2016-06-08 14:25 ` [tip:locking/core] " tip-bot for Waiman Long
2016-05-18 1:26 ` [PATCH v4 3/5] locking/rwsem: Don't wake up one's own task Waiman Long
2016-05-18 10:30 ` Peter Zijlstra
2016-05-18 16:04 ` Waiman Long
2016-05-18 1:26 ` [PATCH v4 4/5] locking/rwsem: Improve reader wakeup code Waiman Long
2016-06-08 14:25 ` [tip:locking/core] " tip-bot for Waiman Long
2016-05-18 1:26 ` [PATCH v4 5/5] locking/rwsem: Streamline the rwsem_optimistic_spin() code Waiman Long
2016-06-08 14:26 ` [tip:locking/core] " tip-bot for Waiman Long
2016-05-18 10:52 ` [PATCH v4 0/5] [PATCH v3 0/4] locking/rwsem: Add reader-owned state to the owner field Peter Zijlstra
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=573CB496.4010707@hpe.com \
--to=waiman.long@hpe.com \
--cc=dave@stgolabs.net \
--cc=david@fromorbit.com \
--cc=doug.hatch@hpe.com \
--cc=jason.low2@hp.com \
--cc=jason.low2@hpe.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peter@hurleysoftware.com \
--cc=peterz@infradead.org \
--cc=scott.norton@hpe.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.