All of lore.kernel.org
 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 :-) )

  reply	other threads:[~2014-11-24 20:53 UTC|newest]

Thread overview: 34+ 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 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:35       ` Linus Torvalds
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:04           ` David Howells
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
  -- 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:48 ` Paul E. McKenney
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 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.