All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Borntraeger <borntraeger@de.ibm.com>
To: paulmck@linux.vnet.ibm.com,
	Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	David Howells <dhowells@redhat.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>,
	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:28:05 +0100	[thread overview]
Message-ID: <547394D5.4020301@de.ibm.com> (raw)
In-Reply-To: <20141124194200.GR5050@linux.vnet.ibm.com>

Am 24.11.2014 um 20:42 schrieb Paul E. McKenney:
> On Mon, Nov 24, 2014 at 11:14:42AM -0800, Linus Torvalds wrote:
>> On Mon, Nov 24, 2014 at 11:07 AM, Christian Borntraeger
>> <borntraeger@de.ibm.com> wrote:
>>>
>>> Looks really nice, but does not work with ACCESS_ONCE is on the left-hand side:
>>
>> Oh, I forgot about that. And that was indeed why I had done that whole
>> helper macro originally, with ACCESS_ONCE() itself just being the
>> dereference of the pointer.
> 
> OK, how about the following?
> 
> It complains if the variable is too large, for example, long long on
> 32-bit systems or large structures.  It is OK loading from and storing
> to small structures as well, which I am having a hard time thinking of
> as a disadvantage.

Well, the motivation for this series was that gcc  4.6 and 4.7 might ignore volatile for
such a case, see the original thread and this data structure

union ipte_control {
        unsigned long val;
        struct {
                unsigned long k  : 1;
                unsigned long kh : 31;
                unsigned long kg : 32;
        };
};

> ------------------------------------------------------------------------
> 
> #define get_scalar_volatile_pointer(x) ({ \
> 	volatile typeof(x) *__vp = &(x); \
> 	BUILD_BUG_ON(sizeof(*__vp) != sizeof(char) && \
> 		     sizeof(*__vp) != sizeof(short) && \
> 		     sizeof(*__vp) != sizeof(int) && \
> 		     sizeof(*__vp) != sizeof(long)); \
> 	__vp; })
> #define ACCESS_ONCE(x) (*get_scalar_volatile_pointer(x))
> 

This gives also several compiler errors when accessing u64 on a 32bit system. This is expected, but more widespread than expected - ouch.

Christian

  parent reply	other threads:[~2014-11-24 20:28 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 13:03 ` Christian Borntraeger
2014-11-24 18:50   ` 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 [this message]
2014-11-24 20:04           ` David Howells
2014-11-24 20:34             ` Linus Torvalds
2014-11-24 20:53               ` Christian Borntraeger
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=547394D5.4020301@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.