lkml.org 
[lkml]   [2019]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2] buffer: Fix I/O error due to ARM read-after-read hazard
On Wed, Nov 13, 2019 at 2:24 AM Will Deacon <will@kernel.org> wrote:
>
> Ok, I'll stick my neck out here, but if test_bit() is being used to read
> a bitmap that is being concurrently modified (e.g. by set_bit() which boils
> down to atomic_long_or()), then why isn't READ_ONCE() required? Right now,
> test_bit takes a 'const volatile unsigned long *addr' argument, so I don't
> see that you'll get a change in codegen except on alpha and, with this
> erratum, arm32.

You're right. We've moved back to actually having it volatile (or
possibly never got away from it). My bad.

At one point, we tried very hard to make test_bit() perform much
better when you tested multiple bits, and I thought we ended up having
that merged and didn't want to regress. But apparently we never did
that, or it got undone.

test_bit() is a very unfortunate interface, in that we actually use it
in some situations where we _really_ would want to merge reads (not
split them, but merge them). There are several cases where we do
constant test-bits on the same word, and don't care about ordering.
Things like thread flags etc.

It's explicitly not ordered, so *merging* reads would be right and
lovely, it's the "don't read twice" that is bad. But we have no way to
tell the compiler that ;(

Anyway, READ_ONCE() is ok by me when I look at the code, because as
you say, it's already volatile (like my memory ;).

Linus

\
 
 \ /
  Last update: 2019-11-13 17:37    [W:0.213 / U:7.380 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site