linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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 :-) )

  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).