From: Christian Borntraeger <borntraeger@de.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
David Howells <dhowells@redhat.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
linux-mips <linux-mips@linux-mips.org>,
linux-x86_64@vger.kernel.org,
linux-s390 <linux-s390@vger.kernel.org>,
Paolo Bonzini <pbonzini@redhat.com>,
Paul McKenney <paulmck@linux.vnet.ibm.com>,
Ingo Molnar <mingo@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>
Subject: Re: [PATCH/RFC 7/7] kernel: Force ACCESS_ONCE to work only on scalar types
Date: Mon, 24 Nov 2014 21:53:06 +0100 [thread overview]
Message-ID: <54739AB2.8030002@de.ibm.com> (raw)
In-Reply-To: <CA+55aFwHJyyo1y=-u=t798PFTeZN796hnwd9-XzEnL=JaqVmDw@mail.gmail.com>
Am 24.11.2014 um 21:34 schrieb Linus Torvalds:
> On Mon, Nov 24, 2014 at 12:04 PM, David Howells <dhowells@redhat.com> wrote:
>>
>> Reserve ACCESS_ONCE() for reading and add an ASSIGN_ONCE() or something like
>> that for writing?
>
> I wouldn't mind that. We've had situations where reading and writing
> isn't really similar - like alpha where reading a byte is atomic, but
> writing one isn't.
>
> Then we could also make it have the "get_user()/put_user()" kind of
> semantics - .and then use the same "sizeopf()" tricks that we use for
> get_user/put_user.
>
> That would actually work around the gcc bug a completely different way:
>
> #define ACCESS_ONCE(p) \
> ({ typeof(*p) __val; __read_once_size(p, &__val, sizeof(__val)); __val; })
>
> and then we can do things like this:
>
> static __always_inline void __read_once_size(volatile void *p, void
> *res, int size)
> {
> switch (size) {
> case 1: *(u8 *)res = *(volatile u8 *)p; break;
> case 2: *(u16 *)res = *(volatile u16 *)p; break;
> case 4: *(u32 *)res = *(volatile u32 *)p; break;
> #ifdef CONFIG_64BIT
> case 8: *(u64 *)res = *(volatile u64 *)p; break;
> #endif
> }
> }
>
> and same for ASSIGN_ONCE(val, p).
>
> That also hopefully avoids the whole "oops, gcc has a bug", because
> the actual volatile access is always done using a scalar type, even if
> the type of "__val" may in fact be a structure.
>
> Christian, how painful would that be? Sorry to try to make you do a
> totally different approach..
That looks like a lot of changes all over ACCESS_ONCE -> ASSIGN_ONCE:
git grep "ACCESS_ONCE.*=.*"
gives me 200 placea not in Documentation.
Then there is still the 64bit accesses on 32bit via ACCESS_ONCE problem, which we could detect with a default cause in your code. We would need to audit and fix all places :-/
So the last proposal from Alexei, seems easier (for me at least :-) )
next prev parent reply other threads:[~2014-11-24 20:53 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-24 13:03 [PATCH/RFC 0/7] ACCESS_ONCE and non-scalar accesses Christian Borntraeger
2014-11-24 13:03 ` [PATCH 1/7] KVM: s390: Fix ipte locking Christian Borntraeger
2014-11-24 13:03 ` Christian Borntraeger
2014-11-24 13:03 ` [PATCH/RFC 2/7] mm: replace page table access via ACCESS_ONCE with barriers Christian Borntraeger
2014-11-24 13:03 ` [PATCH/RFC 3/7] x86: Rework ACCESS_ONCE for spinlock code Christian Borntraeger
2014-11-24 13:03 ` Christian Borntraeger
2014-11-24 13:03 ` [PATCH/RFC 4/7] x86: Replace ACCESS_ONCE in gup with a barrier Christian Borntraeger
2014-11-24 13:03 ` [PATCH/RFC 5/7] mips: " Christian Borntraeger
2014-11-24 13:03 ` Christian Borntraeger
2014-11-24 13:03 ` [PATCH/RFC 6/7] arm64: Replace ACCESS_ONCE for spinlock code with barriers Christian Borntraeger
2014-11-24 18:50 ` Christian Borntraeger
2014-11-24 18:50 ` Christian Borntraeger
2014-11-24 13:03 ` Christian Borntraeger
2014-11-24 13:03 ` [PATCH/RFC 7/7] kernel: Force ACCESS_ONCE to work only on scalar types Christian Borntraeger
2014-11-24 13:30 ` David Howells
2014-11-24 17:30 ` Linus Torvalds
2014-11-24 18:02 ` Alexei Starovoitov
2014-11-24 18:02 ` Alexei Starovoitov
2014-11-24 18:35 ` Linus Torvalds
2014-11-24 18:35 ` Linus Torvalds
2014-11-24 19:07 ` Christian Borntraeger
2014-11-24 19:07 ` Christian Borntraeger
2014-11-24 19:14 ` Linus Torvalds
2014-11-24 19:42 ` Paul E. McKenney
2014-11-24 20:19 ` Linus Torvalds
2014-11-24 20:46 ` Paul E. McKenney
2014-11-24 20:28 ` Christian Borntraeger
2014-11-24 20:28 ` Christian Borntraeger
2014-11-24 20:04 ` David Howells
2014-11-24 20:34 ` Linus Torvalds
2014-11-24 20:34 ` Linus Torvalds
2014-11-24 20:53 ` Christian Borntraeger [this message]
2014-11-24 21:02 ` Linus Torvalds
2014-11-24 21:16 ` Christian Borntraeger
2014-11-24 21:16 ` Christian Borntraeger
-- strict thread matches above, loose matches on Subject: below --
2014-11-24 20:29 Alexei Starovoitov
2014-11-24 20:45 ` Christian Borntraeger
2014-11-24 20:45 ` Christian Borntraeger
2014-11-24 20:48 ` Paul E. McKenney
2014-11-24 22:58 Alexei Starovoitov
2014-11-24 22:58 ` Alexei Starovoitov
2014-11-25 0:00 ` Linus Torvalds
2014-11-25 2:28 Alexei Starovoitov
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=54739AB2.8030002@de.ibm.com \
--to=borntraeger@de.ibm.com \
--cc=alexei.starovoitov@gmail.com \
--cc=catalin.marinas@arm.com \
--cc=dhowells@redhat.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@linux-mips.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux-x86_64@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=paulmck@linux.vnet.ibm.com \
--cc=pbonzini@redhat.com \
--cc=torvalds@linux-foundation.org \
--cc=will.deacon@arm.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 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).