Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1299746 > unrolled thread
| Started by | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| First post | 2015-12-31 20:10 +0100 |
| Last post | 2016-01-01 11:30 +0100 |
| Articles | 20 on this page of 74 — 14 participants |
Back to article view | Back to linux.kernel
[PATCH v2 00/34] arch: barrier cleanup + barriers for virt "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:10 +0100
[PATCH v2 08/32] arm: reuse asm-generic/barrier.h "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:10 +0100
Re: [PATCH v2 08/32] arm: reuse asm-generic/barrier.h Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-01-02 12:30 +0100
[PATCH v2 27/32] x86: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:10 +0100
[PATCH v2 04/32] ia64: reuse asm-generic/barrier.h "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:10 +0100
[PATCH v2 22/32] s390: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:10 +0100
Re: [PATCH v2 22/32] s390: define __smp_xxx Peter Zijlstra <peterz@infradead.org> - 2016-01-04 14:50 +0100
Re: [PATCH v2 22/32] s390: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-04 21:20 +0100
Re: [PATCH v2 22/32] s390: define __smp_xxx Martin Schwidefsky <schwidefsky@de.ibm.com> - 2016-01-05 09:20 +0100
Re: [PATCH v2 22/32] s390: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-05 10:40 +0100
Re: [PATCH v2 22/32] s390: define __smp_xxx Martin Schwidefsky <schwidefsky@de.ibm.com> - 2016-01-05 13:10 +0100
Re: [PATCH v2 22/32] s390: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-05 14:10 +0100
Re: [PATCH v2 22/32] s390: define __smp_xxx Martin Schwidefsky <schwidefsky@de.ibm.com> - 2016-01-05 15:30 +0100
Re: [PATCH v2 22/32] s390: define __smp_xxx Christian Borntraeger <borntraeger@de.ibm.com> - 2016-01-05 16:50 +0100
Re: [PATCH v2 22/32] s390: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-05 17:10 +0100
[PATCH v2 15/32] powerpc: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:10 +0100
Re: [PATCH v2 15/32] powerpc: define __smp_xxx Boqun Feng <boqun.feng@gmail.com> - 2016-01-05 02:40 +0100
Re: [PATCH v2 15/32] powerpc: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-05 10:00 +0100
Re: [PATCH v2 15/32] powerpc: define __smp_xxx Boqun Feng <boqun.feng@gmail.com> - 2016-01-05 11:00 +0100
Re: [PATCH v2 15/32] powerpc: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-05 17:20 +0100
Re: [PATCH v2 15/32] powerpc: define __smp_xxx Boqun Feng <boqun.feng@gmail.com> - 2016-01-06 03:00 +0100
Re: [PATCH v2 15/32] powerpc: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-06 21:30 +0100
Re: [PATCH v2 15/32] powerpc: define __smp_xxx Boqun Feng <boqun.feng@gmail.com> - 2016-01-07 01:50 +0100
[PATCH v2 12/32] x86/um: reuse asm-generic/barrier.h "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:10 +0100
Re: [PATCH v2 12/32] x86/um: reuse asm-generic/barrier.h Richard Weinberger <richard@nod.at> - 2016-01-06 00:20 +0100
[PATCH v2 10/32] metag: reuse asm-generic/barrier.h "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:10 +0100
Re: [PATCH v2 10/32] metag: reuse asm-generic/barrier.h James Hogan <james.hogan@imgtec.com> - 2016-01-05 00:30 +0100
[PATCH v2 16/32] arm64: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:10 +0100
[PATCH v2 03/32] ia64: rename nop->iosapic_nop "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:10 +0100
[PATCH v2 06/32] s390: reuse asm-generic/barrier.h "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:10 +0100
Re: [PATCH v2 06/32] s390: reuse asm-generic/barrier.h Peter Zijlstra <peterz@infradead.org> - 2016-01-04 14:30 +0100
Re: [PATCH v2 06/32] s390: reuse asm-generic/barrier.h Martin Schwidefsky <schwidefsky@de.ibm.com> - 2016-01-04 16:10 +0100
Re: [PATCH v2 06/32] s390: reuse asm-generic/barrier.h "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-04 21:50 +0100
Re: [PATCH v2 06/32] s390: reuse asm-generic/barrier.h Martin Schwidefsky <schwidefsky@de.ibm.com> - 2016-01-05 09:10 +0100
Re: [PATCH v2 06/32] s390: reuse asm-generic/barrier.h "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-04 21:40 +0100
[PATCH v2 21/32] mips: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:20 +0100
[PATCH v2 09/32] arm64: reuse asm-generic/barrier.h "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:20 +0100
[PATCH v2 32/32] virtio_ring: use virt_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:20 +0100
Re: [PATCH v2 32/32] virtio_ring: use virt_store_mb Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-01-01 18:30 +0100
Re: [PATCH v2 32/32] virtio_ring: use virt_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-03 10:10 +0100
[PATCH v2 31/32] sh: support a 2-byte smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:20 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Peter Zijlstra <peterz@infradead.org> - 2016-01-04 15:10 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Rich Felker <dalias@libc.org> - 2016-01-06 00:40 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-06 12:20 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Peter Zijlstra <peterz@infradead.org> - 2016-01-06 12:50 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-06 13:00 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Peter Zijlstra <peterz@infradead.org> - 2016-01-06 15:40 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Rob Landley <rob@landley.net> - 2016-01-06 16:50 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Peter Zijlstra <peterz@infradead.org> - 2016-01-06 18:00 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Rob Landley <rob@landley.net> - 2016-01-06 21:30 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Geert Uytterhoeven <geert@linux-m68k.org> - 2016-01-06 20:00 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Rich Felker <dalias@libc.org> - 2016-01-06 19:30 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-06 21:30 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Rich Felker <dalias@libc.org> - 2016-01-07 01:00 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Peter Zijlstra <peterz@infradead.org> - 2016-01-07 14:40 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Rich Felker <dalias@libc.org> - 2016-01-07 20:10 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-07 17:00 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-07 18:50 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Rich Felker <dalias@libc.org> - 2016-01-07 20:20 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-07 23:50 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb Rich Felker <dalias@libc.org> - 2016-01-08 05:30 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-08 08:30 +0100
Re: [PATCH v2 31/32] sh: support a 2-byte smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-06 23:20 +0100
[PATCH v2 33/34] xenbus: use virt_xxx barriers "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:20 +0100
Re: [Xen-devel] [PATCH v2 33/34] xenbus: use virt_xxx barriers David Vrabel <david.vrabel@citrix.com> - 2016-01-04 12:40 +0100
Re: [PATCH v2 33/34] xenbus: use virt_xxx barriers Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-04 13:10 +0100
Re: [PATCH v2 33/34] xenbus: use virt_xxx barriers Peter Zijlstra <peterz@infradead.org> - 2016-01-04 15:20 +0100
[PATCH v2 26/32] xtensa: define __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:20 +0100
[PATCH v2 34/34] xen/io: use virt_xxx barriers "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:20 +0100
Re: [Xen-devel] [PATCH v2 34/34] xen/io: use virt_xxx barriers David Vrabel <david.vrabel@citrix.com> - 2016-01-04 12:40 +0100
Re: [PATCH v2 34/34] xen/io: use virt_xxx barriers Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-04 13:10 +0100
[PATCH v2 02/32] asm-generic: guard smp_store_release/load_acquire "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-31 20:20 +0100
[PATCH v2 30/32] virtio_ring: update weak barriers to use __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-01 10:40 +0100
Re: [PATCH v2 30/32] virtio_ring: update weak barriers to use __smp_xxx "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-01 11:30 +0100
Page 1 of 4 [1] 2 3 4 Next page →
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2015-12-31 20:10 +0100 |
| Subject | [PATCH v2 00/34] arch: barrier cleanup + barriers for virt |
| Message-ID | <qLQLL-8pu-3@gated-at.bofh.it> |
Changes since v1:
- replaced my asm-generic patch with an equivalent patch already in tip
- add wrappers with virt_ prefix for better code annotation,
as suggested by David Miller
- dropped XXX in patch names as this makes vger choke, Cc all relevant
mailing lists on all patches (not personal email, as the list becomes
too long then)
I parked this in vhost tree for now, but the inclusion of patch 1 from tip
creates a merge conflict (even though it's easy to resolve).
Would tip maintainers prefer merging it through tip tree instead
(including the virtio patches)?
Or should I just merge it all through my tree, including the
duplicate patch, and assume conflict will be resolved?
If the second, acks will be appreciated.
Thanks!
This is really trying to cleanup some virt code, as suggested by Peter, who
said
> You could of course go fix that instead of mutilating things into
> sort-of functional state.
This work is needed for virtio, so it's probably easiest to
merge it through my tree - is this fine by everyone?
Arnd, if you agree, could you ack this please?
Note to arch maintainers: please don't cherry-pick patches out of this patchset
as it's been structured in this order to avoid breaking bisect.
Please send acks instead!
Sometimes, virtualization is weird. For example, virtio does this (conceptually):
#ifdef CONFIG_SMP
smp_mb();
#else
mb();
#endif
Similarly, Xen calls mb() when it's not doing any MMIO at all.
Of course it's wrong in the sense that it's suboptimal. What we would really
like is to have, on UP, exactly the same barrier as on SMP. This is because a
UP guest can run on an SMP host.
But Linux doesn't provide this ability: if CONFIG_SMP is not defined is
optimizes most barriers out to a compiler barrier.
Consider for example x86: what we want is xchg (NOT mfence - there's no real IO
going on here - just switching out of the VM - more like a function call
really) but if built without CONFIG_SMP smp_store_mb does not include this.
Virt in general is probably the only use-case, because this really is an
artifact of interfacing with an SMP host while running an UP kernel,
but since we have (at least) two users, it seems to make sense to
put these APIs in a central place.
In fact, smp_ barriers are stubs on !SMP, so they can be defined as follows:
arch/XXX/include/asm/barrier.h:
#define __smp_mb() DOSOMETHING
include/asm-generic/barrier.h:
#ifdef CONFIG_SMP
#define smp_mb() __smp_mb()
#else
#define smp_mb() barrier()
#endif
This has the benefit of cleaning out a bunch of duplicated
ifdefs on a bunch of architectures - this patchset brings
about a net reduction in LOC, even with new barriers and extra documentation :)
Then virt can use __smp_XXX when talking to an SMP host.
To make those users explicit, this patchset adds virt_xxx wrappers
for them.
Touching all archs is a tad tedious, but its fairly straight forward.
The rest of the patchset is structured as follows:
-. Patch 1 fixes a bug in asm-generic.
It is already in tip, included here for completeness.
-. Patches 2-12 make sure barrier.h on all remaining
architectures includes asm-generic/barrier.h:
after the change in Patch 1, code there matches
asm-generic/barrier.h almost verbatim.
Minor code tweaks were required in a couple of places.
Macros duplicated from asm-generic/barrier.h are dropped
in the process.
After all that preparatory work, we are getting to the actual change.
-. Patches 13 adds generic smp_XXX wrappers in asm-generic
these select __smp_XXX or barrier() depending on CONFIG_SMP
-. Patches 14-27 change all architectures to
define __smp_XXX macros; the generic code in asm-generic/barrier.h
then defines smp_XXX macros
I compiled the affected arches before and after the changes,
dumped the .text section (using objdump -O binary) and
made sure that the object code is exactly identical
before and after the change.
I couldn't fully build sh,tile,xtensa but I did this test
kernel/rcu/tree.o kernel/sched/wait.o and
kernel/futex.o and tested these instead.
Unfortunately, I don't have a metag cross-build toolset ready.
Hoping for some acks on this architecture.
Finally, the following patches put the __smp_xxx APIs to work for virt:
-. Patch 28 adds virt_ wrappers for __smp_, and documents them.
After all this work, this requires very few lines of code in
the generic header.
-. Patches 29,30,33,34 convert virtio xen drivers to use the virt_xxx APIs
xen patches are untested
virtio ones have been tested on x86
-. Patches 31-32 teach virtio to use virt_store_mb
sh architecture was missing a 2-byte smp_store_mb,
the fix is trivial although my code is not optimal:
if anyone cares, pls send me a patch to apply on top.
I didn't build this architecture, but intel's 0-day
infrastructure builds it.
tested on x86
Davidlohr Bueso (1):
lcoking/barriers, arch: Use smp barriers in smp_store_release()
Michael S. Tsirkin (33):
asm-generic: guard smp_store_release/load_acquire
ia64: rename nop->iosapic_nop
ia64: reuse asm-generic/barrier.h
powerpc: reuse asm-generic/barrier.h
s390: reuse asm-generic/barrier.h
sparc: reuse asm-generic/barrier.h
arm: reuse asm-generic/barrier.h
arm64: reuse asm-generic/barrier.h
metag: reuse asm-generic/barrier.h
mips: reuse asm-generic/barrier.h
x86/um: reuse asm-generic/barrier.h
x86: reuse asm-generic/barrier.h
asm-generic: add __smp_xxx wrappers
powerpc: define __smp_xxx
arm64: define __smp_xxx
arm: define __smp_xxx
blackfin: define __smp_xxx
ia64: define __smp_xxx
metag: define __smp_xxx
mips: define __smp_xxx
s390: define __smp_xxx
sh: define __smp_xxx, fix smp_store_mb for !SMP
sparc: define __smp_xxx
tile: define __smp_xxx
xtensa: define __smp_xxx
x86: define __smp_xxx
asm-generic: implement virt_xxx memory barriers
Revert "virtio_ring: Update weak barriers to use dma_wmb/rmb"
virtio_ring: update weak barriers to use __smp_XXX
sh: support a 2-byte smp_store_mb
virtio_ring: use virt_store_mb
xenbus: use virt_xxx barriers
xen/io: use virt_xxx barriers
arch/arm/include/asm/barrier.h | 35 ++-----------
arch/arm64/include/asm/barrier.h | 19 +++----
arch/blackfin/include/asm/barrier.h | 4 +-
arch/ia64/include/asm/barrier.h | 24 +++------
arch/metag/include/asm/barrier.h | 55 ++++++-------------
arch/mips/include/asm/barrier.h | 51 ++++++------------
arch/powerpc/include/asm/barrier.h | 33 ++++--------
arch/s390/include/asm/barrier.h | 25 ++++-----
arch/sh/include/asm/barrier.h | 11 +++-
arch/sparc/include/asm/barrier_32.h | 1 -
arch/sparc/include/asm/barrier_64.h | 29 +++-------
arch/sparc/include/asm/processor.h | 3 --
arch/tile/include/asm/barrier.h | 9 ++--
arch/x86/include/asm/barrier.h | 36 +++++--------
arch/x86/um/asm/barrier.h | 9 +---
arch/xtensa/include/asm/barrier.h | 4 +-
include/asm-generic/barrier.h | 102 ++++++++++++++++++++++++++++++++----
include/linux/virtio_ring.h | 22 +++++---
include/xen/interface/io/ring.h | 16 +++---
arch/ia64/kernel/iosapic.c | 6 +--
drivers/virtio/virtio_ring.c | 15 +++---
drivers/xen/xenbus/xenbus_comms.c | 8 +--
Documentation/memory-barriers.txt | 28 ++++++++--
23 files changed, 266 insertions(+), 279 deletions(-)
--
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2015-12-31 20:10 +0100 |
| Subject | [PATCH v2 08/32] arm: reuse asm-generic/barrier.h |
| Message-ID | <qLQLM-8pu-37@gated-at.bofh.it> |
| In reply to | #1299746 |
On arm smp_store_mb, read_barrier_depends, smp_read_barrier_depends,
smp_store_release, smp_load_acquire, smp_mb__before_atomic and
smp_mb__after_atomic match the asm-generic variants exactly. Drop the
local definitions and pull in asm-generic/barrier.h instead.
This is in preparation to refactoring this code area.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/include/asm/barrier.h | 23 +----------------------
1 file changed, 1 insertion(+), 22 deletions(-)
diff --git a/arch/arm/include/asm/barrier.h b/arch/arm/include/asm/barrier.h
index 3ff5642..31152e8 100644
--- a/arch/arm/include/asm/barrier.h
+++ b/arch/arm/include/asm/barrier.h
@@ -70,28 +70,7 @@ extern void arm_heavy_mb(void);
#define smp_wmb() dmb(ishst)
#endif
-#define smp_store_release(p, v) \
-do { \
- compiletime_assert_atomic_type(*p); \
- smp_mb(); \
- WRITE_ONCE(*p, v); \
-} while (0)
-
-#define smp_load_acquire(p) \
-({ \
- typeof(*p) ___p1 = READ_ONCE(*p); \
- compiletime_assert_atomic_type(*p); \
- smp_mb(); \
- ___p1; \
-})
-
-#define read_barrier_depends() do { } while(0)
-#define smp_read_barrier_depends() do { } while(0)
-
-#define smp_store_mb(var, value) do { WRITE_ONCE(var, value); smp_mb(); } while (0)
-
-#define smp_mb__before_atomic() smp_mb()
-#define smp_mb__after_atomic() smp_mb()
+#include <asm-generic/barrier.h>
#endif /* !__ASSEMBLY__ */
#endif /* __ASM_BARRIER_H */
--
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@arm.linux.org.uk> |
|---|---|
| Date | 2016-01-02 12:30 +0100 |
| Subject | Re: [PATCH v2 08/32] arm: reuse asm-generic/barrier.h |
| Message-ID | <qMsxI-7tu-1@gated-at.bofh.it> |
| In reply to | #1299747 |
On Thu, Dec 31, 2015 at 09:06:46PM +0200, Michael S. Tsirkin wrote: > On arm smp_store_mb, read_barrier_depends, smp_read_barrier_depends, > smp_store_release, smp_load_acquire, smp_mb__before_atomic and > smp_mb__after_atomic match the asm-generic variants exactly. Drop the > local definitions and pull in asm-generic/barrier.h instead. > > This is in preparation to refactoring this code area. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > Acked-by: Arnd Bergmann <arnd@arndb.de> Thanks, the asm-generic versions looks identical to me, so this should result in no code generation difference. Acked-by: Russell King <rmk+kernel@arm.linux.org.uk> -- RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2015-12-31 20:10 +0100 |
| Subject | [PATCH v2 27/32] x86: define __smp_xxx |
| Message-ID | <qLQLM-8pu-35@gated-at.bofh.it> |
| In reply to | #1299746 |
This defines __smp_xxx barriers for x86,
for use by virtualization.
smp_xxx barriers are removed as they are
defined correctly by asm-generic/barriers.h
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/x86/include/asm/barrier.h | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
index cc4c2a7..a584e1c 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -31,17 +31,10 @@
#endif
#define dma_wmb() barrier()
-#ifdef CONFIG_SMP
-#define smp_mb() mb()
-#define smp_rmb() dma_rmb()
-#define smp_wmb() barrier()
-#define smp_store_mb(var, value) do { (void)xchg(&var, value); } while (0)
-#else /* !SMP */
-#define smp_mb() barrier()
-#define smp_rmb() barrier()
-#define smp_wmb() barrier()
-#define smp_store_mb(var, value) do { WRITE_ONCE(var, value); barrier(); } while (0)
-#endif /* SMP */
+#define __smp_mb() mb()
+#define __smp_rmb() dma_rmb()
+#define __smp_wmb() barrier()
+#define __smp_store_mb(var, value) do { (void)xchg(&var, value); } while (0)
#if defined(CONFIG_X86_PPRO_FENCE)
@@ -50,31 +43,31 @@
* model and we should fall back to full barriers.
*/
-#define smp_store_release(p, v) \
+#define __smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
- smp_mb(); \
+ __smp_mb(); \
WRITE_ONCE(*p, v); \
} while (0)
-#define smp_load_acquire(p) \
+#define __smp_load_acquire(p) \
({ \
typeof(*p) ___p1 = READ_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
- smp_mb(); \
+ __smp_mb(); \
___p1; \
})
#else /* regular x86 TSO memory ordering */
-#define smp_store_release(p, v) \
+#define __smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
barrier(); \
WRITE_ONCE(*p, v); \
} while (0)
-#define smp_load_acquire(p) \
+#define __smp_load_acquire(p) \
({ \
typeof(*p) ___p1 = READ_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
@@ -85,8 +78,8 @@ do { \
#endif
/* Atomic operations are already serializing on x86 */
-#define smp_mb__before_atomic() barrier()
-#define smp_mb__after_atomic() barrier()
+#define __smp_mb__before_atomic() barrier()
+#define __smp_mb__after_atomic() barrier()
#include <asm-generic/barrier.h>
--
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2015-12-31 20:10 +0100 |
| Subject | [PATCH v2 04/32] ia64: reuse asm-generic/barrier.h |
| Message-ID | <qLQLN-8pu-47@gated-at.bofh.it> |
| In reply to | #1299746 |
On ia64 smp_rmb, smp_wmb, read_barrier_depends, smp_read_barrier_depends
and smp_store_mb() match the asm-generic variants exactly. Drop the
local definitions and pull in asm-generic/barrier.h instead.
This is in preparation to refactoring this code area.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Tony Luck <tony.luck@intel.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/ia64/include/asm/barrier.h | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/arch/ia64/include/asm/barrier.h b/arch/ia64/include/asm/barrier.h
index 209c4b8..2f93348 100644
--- a/arch/ia64/include/asm/barrier.h
+++ b/arch/ia64/include/asm/barrier.h
@@ -48,12 +48,6 @@
# define smp_mb() barrier()
#endif
-#define smp_rmb() smp_mb()
-#define smp_wmb() smp_mb()
-
-#define read_barrier_depends() do { } while (0)
-#define smp_read_barrier_depends() do { } while (0)
-
#define smp_mb__before_atomic() barrier()
#define smp_mb__after_atomic() barrier()
@@ -77,12 +71,12 @@ do { \
___p1; \
})
-#define smp_store_mb(var, value) do { WRITE_ONCE(var, value); smp_mb(); } while (0)
-
/*
* The group barrier in front of the rsm & ssm are necessary to ensure
* that none of the previous instructions in the same group are
* affected by the rsm/ssm.
*/
+#include <asm-generic/barrier.h>
+
#endif /* _ASM_IA64_BARRIER_H */
--
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2015-12-31 20:10 +0100 |
| Subject | [PATCH v2 22/32] s390: define __smp_xxx |
| Message-ID | <qLQLN-8pu-49@gated-at.bofh.it> |
| In reply to | #1299746 |
This defines __smp_xxx barriers for s390,
for use by virtualization.
Some smp_xxx barriers are removed as they are
defined correctly by asm-generic/barriers.h
Note: smp_mb, smp_rmb and smp_wmb are defined as full barriers
unconditionally on this architecture.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/s390/include/asm/barrier.h | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h
index c358c31..fbd25b2 100644
--- a/arch/s390/include/asm/barrier.h
+++ b/arch/s390/include/asm/barrier.h
@@ -26,18 +26,21 @@
#define wmb() barrier()
#define dma_rmb() mb()
#define dma_wmb() mb()
-#define smp_mb() mb()
-#define smp_rmb() rmb()
-#define smp_wmb() wmb()
-
-#define smp_store_release(p, v) \
+#define __smp_mb() mb()
+#define __smp_rmb() rmb()
+#define __smp_wmb() wmb()
+#define smp_mb() __smp_mb()
+#define smp_rmb() __smp_rmb()
+#define smp_wmb() __smp_wmb()
+
+#define __smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
barrier(); \
WRITE_ONCE(*p, v); \
} while (0)
-#define smp_load_acquire(p) \
+#define __smp_load_acquire(p) \
({ \
typeof(*p) ___p1 = READ_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
--
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-01-04 14:50 +0100 |
| Subject | Re: [PATCH v2 22/32] s390: define __smp_xxx |
| Message-ID | <qNdGi-48u-19@gated-at.bofh.it> |
| In reply to | #1299750 |
On Thu, Dec 31, 2015 at 09:08:38PM +0200, Michael S. Tsirkin wrote: > This defines __smp_xxx barriers for s390, > for use by virtualization. > > Some smp_xxx barriers are removed as they are > defined correctly by asm-generic/barriers.h > > Note: smp_mb, smp_rmb and smp_wmb are defined as full barriers > unconditionally on this architecture. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > Acked-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/s390/include/asm/barrier.h | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h > index c358c31..fbd25b2 100644 > --- a/arch/s390/include/asm/barrier.h > +++ b/arch/s390/include/asm/barrier.h > @@ -26,18 +26,21 @@ > #define wmb() barrier() > #define dma_rmb() mb() > #define dma_wmb() mb() > -#define smp_mb() mb() > -#define smp_rmb() rmb() > -#define smp_wmb() wmb() > - > -#define smp_store_release(p, v) \ > +#define __smp_mb() mb() > +#define __smp_rmb() rmb() > +#define __smp_wmb() wmb() > +#define smp_mb() __smp_mb() > +#define smp_rmb() __smp_rmb() > +#define smp_wmb() __smp_wmb() Why define the smp_*mb() primitives here? Would not the inclusion of asm-generic/barrier.h do this? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2016-01-04 21:20 +0100 |
| Subject | Re: [PATCH v2 22/32] s390: define __smp_xxx |
| Message-ID | <qNjLL-8i4-69@gated-at.bofh.it> |
| In reply to | #1300754 |
On Mon, Jan 04, 2016 at 02:45:25PM +0100, Peter Zijlstra wrote: > On Thu, Dec 31, 2015 at 09:08:38PM +0200, Michael S. Tsirkin wrote: > > This defines __smp_xxx barriers for s390, > > for use by virtualization. > > > > Some smp_xxx barriers are removed as they are > > defined correctly by asm-generic/barriers.h > > > > Note: smp_mb, smp_rmb and smp_wmb are defined as full barriers > > unconditionally on this architecture. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > --- > > arch/s390/include/asm/barrier.h | 15 +++++++++------ > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h > > index c358c31..fbd25b2 100644 > > --- a/arch/s390/include/asm/barrier.h > > +++ b/arch/s390/include/asm/barrier.h > > @@ -26,18 +26,21 @@ > > #define wmb() barrier() > > #define dma_rmb() mb() > > #define dma_wmb() mb() > > -#define smp_mb() mb() > > -#define smp_rmb() rmb() > > -#define smp_wmb() wmb() > > - > > -#define smp_store_release(p, v) \ > > +#define __smp_mb() mb() > > +#define __smp_rmb() rmb() > > +#define __smp_wmb() wmb() > > +#define smp_mb() __smp_mb() > > +#define smp_rmb() __smp_rmb() > > +#define smp_wmb() __smp_wmb() > > Why define the smp_*mb() primitives here? Would not the inclusion of > asm-generic/barrier.h do this? No because the generic one is a nop on !SMP, this one isn't. Pls note this patch is just reordering code without making functional changes. And at the moment, on s390 smp_xxx barriers are always non empty. Some of this could be sub-optimal, but since on s390 Linux always runs on a hypervisor, I am not sure it's safe to use the generic version - in other words, it just might be that for s390 smp_ and virt_ barriers must be equivalent. If in fact this turns out to be wrong, I can pick up a patch to change this, but I'd rather make this a patch on top so that my patches are testable just by compiling and comparing the binary. -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Martin Schwidefsky <schwidefsky@de.ibm.com> |
|---|---|
| Date | 2016-01-05 09:20 +0100 |
| Subject | Re: [PATCH v2 22/32] s390: define __smp_xxx |
| Message-ID | <qNv0t-7OE-7@gated-at.bofh.it> |
| In reply to | #1301045 |
On Mon, 4 Jan 2016 22:18:58 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Mon, Jan 04, 2016 at 02:45:25PM +0100, Peter Zijlstra wrote: > > On Thu, Dec 31, 2015 at 09:08:38PM +0200, Michael S. Tsirkin wrote: > > > This defines __smp_xxx barriers for s390, > > > for use by virtualization. > > > > > > Some smp_xxx barriers are removed as they are > > > defined correctly by asm-generic/barriers.h > > > > > > Note: smp_mb, smp_rmb and smp_wmb are defined as full barriers > > > unconditionally on this architecture. > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > --- > > > arch/s390/include/asm/barrier.h | 15 +++++++++------ > > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > > > diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h > > > index c358c31..fbd25b2 100644 > > > --- a/arch/s390/include/asm/barrier.h > > > +++ b/arch/s390/include/asm/barrier.h > > > @@ -26,18 +26,21 @@ > > > #define wmb() barrier() > > > #define dma_rmb() mb() > > > #define dma_wmb() mb() > > > -#define smp_mb() mb() > > > -#define smp_rmb() rmb() > > > -#define smp_wmb() wmb() > > > - > > > -#define smp_store_release(p, v) \ > > > +#define __smp_mb() mb() > > > +#define __smp_rmb() rmb() > > > +#define __smp_wmb() wmb() > > > +#define smp_mb() __smp_mb() > > > +#define smp_rmb() __smp_rmb() > > > +#define smp_wmb() __smp_wmb() > > > > Why define the smp_*mb() primitives here? Would not the inclusion of > > asm-generic/barrier.h do this? > > No because the generic one is a nop on !SMP, this one isn't. > > Pls note this patch is just reordering code without making > functional changes. > And at the moment, on s390 smp_xxx barriers are always non empty. The s390 kernel is SMP to 99.99%, we just didn't bother with a non-smp variant for the memory-barriers. If the generic header is used we'd get the non-smp version for free. It will save a small amount of text space for CONFIG_SMP=n. > Some of this could be sub-optimal, but > since on s390 Linux always runs on a hypervisor, > I am not sure it's safe to use the generic version - > in other words, it just might be that for s390 smp_ and virt_ > barriers must be equivalent. The definition of the memory barriers is independent from the fact if the system is running on an hypervisor or not. Is there really an architecture where you need special virt_xxx barriers?!? -- blue skies, Martin. "Reality continues to ruin my life." - Calvin. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2016-01-05 10:40 +0100 |
| Subject | Re: [PATCH v2 22/32] s390: define __smp_xxx |
| Message-ID | <qNwfU-ag-29@gated-at.bofh.it> |
| In reply to | #1301322 |
On Tue, Jan 05, 2016 at 09:13:19AM +0100, Martin Schwidefsky wrote: > On Mon, 4 Jan 2016 22:18:58 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Mon, Jan 04, 2016 at 02:45:25PM +0100, Peter Zijlstra wrote: > > > On Thu, Dec 31, 2015 at 09:08:38PM +0200, Michael S. Tsirkin wrote: > > > > This defines __smp_xxx barriers for s390, > > > > for use by virtualization. > > > > > > > > Some smp_xxx barriers are removed as they are > > > > defined correctly by asm-generic/barriers.h > > > > > > > > Note: smp_mb, smp_rmb and smp_wmb are defined as full barriers > > > > unconditionally on this architecture. > > > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > --- > > > > arch/s390/include/asm/barrier.h | 15 +++++++++------ > > > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h > > > > index c358c31..fbd25b2 100644 > > > > --- a/arch/s390/include/asm/barrier.h > > > > +++ b/arch/s390/include/asm/barrier.h > > > > @@ -26,18 +26,21 @@ > > > > #define wmb() barrier() > > > > #define dma_rmb() mb() > > > > #define dma_wmb() mb() > > > > -#define smp_mb() mb() > > > > -#define smp_rmb() rmb() > > > > -#define smp_wmb() wmb() > > > > - > > > > -#define smp_store_release(p, v) \ > > > > +#define __smp_mb() mb() > > > > +#define __smp_rmb() rmb() > > > > +#define __smp_wmb() wmb() > > > > +#define smp_mb() __smp_mb() > > > > +#define smp_rmb() __smp_rmb() > > > > +#define smp_wmb() __smp_wmb() > > > > > > Why define the smp_*mb() primitives here? Would not the inclusion of > > > asm-generic/barrier.h do this? > > > > No because the generic one is a nop on !SMP, this one isn't. > > > > Pls note this patch is just reordering code without making > > functional changes. > > And at the moment, on s390 smp_xxx barriers are always non empty. > > The s390 kernel is SMP to 99.99%, we just didn't bother with a > non-smp variant for the memory-barriers. If the generic header > is used we'd get the non-smp version for free. It will save a > small amount of text space for CONFIG_SMP=n. OK, so I'll queue a patch to do this then? Just to make sure: the question would be, are smp_xxx barriers ever used in s390 arch specific code to flush in/out memory accesses for synchronization with the hypervisor? I went over s390 arch code and it seems to me the answer is no (except of course for virtio). But I also see a lot of weirdness on this architecture. I found these calls: arch/s390/include/asm/bitops.h: smp_mb__before_atomic(); arch/s390/include/asm/bitops.h: smp_mb(); Not used in arch specific code so this is likely OK. arch/s390/kernel/vdso.c: smp_mb(); Looking at Author: Christian Borntraeger <borntraeger@de.ibm.com> Date: Fri Sep 11 16:23:06 2015 +0200 s390/vdso: use correct memory barrier By definition smp_wmb only orders writes against writes. (Finish all previous writes, and do not start any future write). To protect the vdso init code against early reads on other CPUs, let's use a full smp_mb at the end of vdso init. As right now smp_wmb is implemented as full serialization, this needs no stable backport, but this change will be necessary if we reimplement smp_wmb. ok from hypervisor point of view, but it's also strange: 1. why isn't this paired with another mb somewhere? this seems to violate barrier pairing rules. 2. how does smp_mb protect against early reads on other CPUs? It normally does not: it orders reads from this CPU versus writes from same CPU. But init code does not appear to read anything. Maybe this is some s390 specific trick? I could not figure out the above commit. arch/s390/kvm/kvm-s390.c: smp_mb(); Does not appear to be paired with anything. arch/s390/lib/spinlock.c: smp_mb(); arch/s390/lib/spinlock.c: smp_mb(); Seems ok, and appears paired properly. Just to make sure - spinlock is not paravirtualized on s390, is it? rch/s390/kernel/time.c: smp_wmb(); arch/s390/kernel/time.c: smp_wmb(); arch/s390/kernel/time.c: smp_wmb(); arch/s390/kernel/time.c: smp_wmb(); It's all around vdso, so I'm guessing userspace is using this, this is why there's no pairing. > > Some of this could be sub-optimal, but > > since on s390 Linux always runs on a hypervisor, > > I am not sure it's safe to use the generic version - > > in other words, it just might be that for s390 smp_ and virt_ > > barriers must be equivalent. > > The definition of the memory barriers is independent from the fact > if the system is running on an hypervisor or not. > Is there really > an architecture where you need special virt_xxx barriers?!? It is whenever host and guest or two guests access memory at the same time. The optimization where smp_xxx barriers are compiled out when CONFIG_SMP is cleared means that two UP guests running on an SMP host can not use smp_xxx barriers for communication. See explanation here: http://thread.gmane.org/gmane.linux.kernel.virtualization/26555 > -- > blue skies, > Martin. > > "Reality continues to ruin my life." - Calvin. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Martin Schwidefsky <schwidefsky@de.ibm.com> |
|---|---|
| Date | 2016-01-05 13:10 +0100 |
| Subject | Re: [PATCH v2 22/32] s390: define __smp_xxx |
| Message-ID | <qNyB3-2yO-1@gated-at.bofh.it> |
| In reply to | #1301374 |
On Tue, 5 Jan 2016 11:30:19 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Jan 05, 2016 at 09:13:19AM +0100, Martin Schwidefsky wrote: > > On Mon, 4 Jan 2016 22:18:58 +0200 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Mon, Jan 04, 2016 at 02:45:25PM +0100, Peter Zijlstra wrote: > > > > On Thu, Dec 31, 2015 at 09:08:38PM +0200, Michael S. Tsirkin wrote: > > > > > This defines __smp_xxx barriers for s390, > > > > > for use by virtualization. > > > > > > > > > > Some smp_xxx barriers are removed as they are > > > > > defined correctly by asm-generic/barriers.h > > > > > > > > > > Note: smp_mb, smp_rmb and smp_wmb are defined as full barriers > > > > > unconditionally on this architecture. > > > > > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > > --- > > > > > arch/s390/include/asm/barrier.h | 15 +++++++++------ > > > > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h > > > > > index c358c31..fbd25b2 100644 > > > > > --- a/arch/s390/include/asm/barrier.h > > > > > +++ b/arch/s390/include/asm/barrier.h > > > > > @@ -26,18 +26,21 @@ > > > > > #define wmb() barrier() > > > > > #define dma_rmb() mb() > > > > > #define dma_wmb() mb() > > > > > -#define smp_mb() mb() > > > > > -#define smp_rmb() rmb() > > > > > -#define smp_wmb() wmb() > > > > > - > > > > > -#define smp_store_release(p, v) \ > > > > > +#define __smp_mb() mb() > > > > > +#define __smp_rmb() rmb() > > > > > +#define __smp_wmb() wmb() > > > > > +#define smp_mb() __smp_mb() > > > > > +#define smp_rmb() __smp_rmb() > > > > > +#define smp_wmb() __smp_wmb() > > > > > > > > Why define the smp_*mb() primitives here? Would not the inclusion of > > > > asm-generic/barrier.h do this? > > > > > > No because the generic one is a nop on !SMP, this one isn't. > > > > > > Pls note this patch is just reordering code without making > > > functional changes. > > > And at the moment, on s390 smp_xxx barriers are always non empty. > > > > The s390 kernel is SMP to 99.99%, we just didn't bother with a > > non-smp variant for the memory-barriers. If the generic header > > is used we'd get the non-smp version for free. It will save a > > small amount of text space for CONFIG_SMP=n. > > OK, so I'll queue a patch to do this then? Yes please. > Just to make sure: the question would be, are smp_xxx barriers ever used > in s390 arch specific code to flush in/out memory accesses for > synchronization with the hypervisor? > > I went over s390 arch code and it seems to me the answer is no > (except of course for virtio). Correct. Guest to host communication either uses instructions which imply a memory barrier or QDIO which uses atomics. > But I also see a lot of weirdness on this architecture. Mostly historical, s390 actually is one of the easiest architectures in regard to memory barriers. > I found these calls: > > arch/s390/include/asm/bitops.h: smp_mb__before_atomic(); > arch/s390/include/asm/bitops.h: smp_mb(); > > Not used in arch specific code so this is likely OK. This has been introduced with git commit 5402ea6af11dc5a9, the smp_mb and smp_mb__before_atomic are used in clear_bit_unlock and __clear_bit_unlock which are 1:1 copies from the code in include/asm-generic/bitops/lock.h. Only test_and_set_bit_lock differs from the generic implementation. > arch/s390/kernel/vdso.c: smp_mb(); > > Looking at > Author: Christian Borntraeger <borntraeger@de.ibm.com> > Date: Fri Sep 11 16:23:06 2015 +0200 > > s390/vdso: use correct memory barrier > > By definition smp_wmb only orders writes against writes. (Finish all > previous writes, and do not start any future write). To protect the > vdso init code against early reads on other CPUs, let's use a full > smp_mb at the end of vdso init. As right now smp_wmb is implemented > as full serialization, this needs no stable backport, but this change > will be necessary if we reimplement smp_wmb. > > ok from hypervisor point of view, but it's also strange: > 1. why isn't this paired with another mb somewhere? > this seems to violate barrier pairing rules. > 2. how does smp_mb protect against early reads on other CPUs? > It normally does not: it orders reads from this CPU versus writes > from same CPU. But init code does not appear to read anything. > Maybe this is some s390 specific trick? > > I could not figure out the above commit. That smp_mb can be removed. The initial s390 vdso code is heavily influenced by the powerpc version which does have a smp_wmb in vdso_init right before the vdso_ready=1 assignment. s390 has no need for that. > > arch/s390/kvm/kvm-s390.c: smp_mb(); > > Does not appear to be paired with anything. This one does not make sense to me. Imho can be removed as well. > arch/s390/lib/spinlock.c: smp_mb(); > arch/s390/lib/spinlock.c: smp_mb(); > > Seems ok, and appears paired properly. > Just to make sure - spinlock is not paravirtualized on s390, is it? s390 just uses the compare-and-swap instruction for the basic lock/unlock operation, this implies the memory barrier. We do call the hypervisor for contended locks if the lock can not be acquired after a number of retries. A while ago we did play with ticket spinlocks but they behaved badly in out usual virtualized environments. If we find the time we might take a closer look at the para-virtualized queued spinlocks. > rch/s390/kernel/time.c: smp_wmb(); > arch/s390/kernel/time.c: smp_wmb(); > arch/s390/kernel/time.c: smp_wmb(); > arch/s390/kernel/time.c: smp_wmb(); > > It's all around vdso, so I'm guessing userspace is using this, > this is why there's no pairing. Correct, this is the update count mechanics with the vdso user space code. > > > Some of this could be sub-optimal, but > > > since on s390 Linux always runs on a hypervisor, > > > I am not sure it's safe to use the generic version - > > > in other words, it just might be that for s390 smp_ and virt_ > > > barriers must be equivalent. > > > > The definition of the memory barriers is independent from the fact > > if the system is running on an hypervisor or not. > > Is there really > > an architecture where you need special virt_xxx barriers?!? > > It is whenever host and guest or two guests access memory at > the same time. > > The optimization where smp_xxx barriers are compiled out when > CONFIG_SMP is cleared means that two UP guests running > on an SMP host can not use smp_xxx barriers for communication. > > See explanation here: > http://thread.gmane.org/gmane.linux.kernel.virtualization/26555 Got it, makes sense. -- blue skies, Martin. "Reality continues to ruin my life." - Calvin. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2016-01-05 14:10 +0100 |
| Subject | Re: [PATCH v2 22/32] s390: define __smp_xxx |
| Message-ID | <qNzx9-3cW-21@gated-at.bofh.it> |
| In reply to | #1301471 |
On Tue, Jan 05, 2016 at 01:08:52PM +0100, Martin Schwidefsky wrote: > On Tue, 5 Jan 2016 11:30:19 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Tue, Jan 05, 2016 at 09:13:19AM +0100, Martin Schwidefsky wrote: > > > On Mon, 4 Jan 2016 22:18:58 +0200 > > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > On Mon, Jan 04, 2016 at 02:45:25PM +0100, Peter Zijlstra wrote: > > > > > On Thu, Dec 31, 2015 at 09:08:38PM +0200, Michael S. Tsirkin wrote: > > > > > > This defines __smp_xxx barriers for s390, > > > > > > for use by virtualization. > > > > > > > > > > > > Some smp_xxx barriers are removed as they are > > > > > > defined correctly by asm-generic/barriers.h > > > > > > > > > > > > Note: smp_mb, smp_rmb and smp_wmb are defined as full barriers > > > > > > unconditionally on this architecture. > > > > > > > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > > > --- > > > > > > arch/s390/include/asm/barrier.h | 15 +++++++++------ > > > > > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > > > > > > > > > diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h > > > > > > index c358c31..fbd25b2 100644 > > > > > > --- a/arch/s390/include/asm/barrier.h > > > > > > +++ b/arch/s390/include/asm/barrier.h > > > > > > @@ -26,18 +26,21 @@ > > > > > > #define wmb() barrier() > > > > > > #define dma_rmb() mb() > > > > > > #define dma_wmb() mb() > > > > > > -#define smp_mb() mb() > > > > > > -#define smp_rmb() rmb() > > > > > > -#define smp_wmb() wmb() > > > > > > - > > > > > > -#define smp_store_release(p, v) \ > > > > > > +#define __smp_mb() mb() > > > > > > +#define __smp_rmb() rmb() > > > > > > +#define __smp_wmb() wmb() > > > > > > +#define smp_mb() __smp_mb() > > > > > > +#define smp_rmb() __smp_rmb() > > > > > > +#define smp_wmb() __smp_wmb() > > > > > > > > > > Why define the smp_*mb() primitives here? Would not the inclusion of > > > > > asm-generic/barrier.h do this? > > > > > > > > No because the generic one is a nop on !SMP, this one isn't. > > > > > > > > Pls note this patch is just reordering code without making > > > > functional changes. > > > > And at the moment, on s390 smp_xxx barriers are always non empty. > > > > > > The s390 kernel is SMP to 99.99%, we just didn't bother with a > > > non-smp variant for the memory-barriers. If the generic header > > > is used we'd get the non-smp version for free. It will save a > > > small amount of text space for CONFIG_SMP=n. > > > > OK, so I'll queue a patch to do this then? > > Yes please. OK, I'll add a patch on top in v3. > > Just to make sure: the question would be, are smp_xxx barriers ever used > > in s390 arch specific code to flush in/out memory accesses for > > synchronization with the hypervisor? > > > > I went over s390 arch code and it seems to me the answer is no > > (except of course for virtio). > > Correct. Guest to host communication either uses instructions which > imply a memory barrier or QDIO which uses atomics. And atomics imply a barrier on s390, right? > > But I also see a lot of weirdness on this architecture. > > Mostly historical, s390 actually is one of the easiest architectures in > regard to memory barriers. > > > I found these calls: > > > > arch/s390/include/asm/bitops.h: smp_mb__before_atomic(); > > arch/s390/include/asm/bitops.h: smp_mb(); > > > > Not used in arch specific code so this is likely OK. > > This has been introduced with git commit 5402ea6af11dc5a9, the smp_mb > and smp_mb__before_atomic are used in clear_bit_unlock and > __clear_bit_unlock which are 1:1 copies from the code in > include/asm-generic/bitops/lock.h. Only test_and_set_bit_lock differs > from the generic implementation. something to keep in mind, but I'd rather not touch bitops at the moment - this patchset is already too big. > > arch/s390/kernel/vdso.c: smp_mb(); > > > > Looking at > > Author: Christian Borntraeger <borntraeger@de.ibm.com> > > Date: Fri Sep 11 16:23:06 2015 +0200 > > > > s390/vdso: use correct memory barrier > > > > By definition smp_wmb only orders writes against writes. (Finish all > > previous writes, and do not start any future write). To protect the > > vdso init code against early reads on other CPUs, let's use a full > > smp_mb at the end of vdso init. As right now smp_wmb is implemented > > as full serialization, this needs no stable backport, but this change > > will be necessary if we reimplement smp_wmb. > > > > ok from hypervisor point of view, but it's also strange: > > 1. why isn't this paired with another mb somewhere? > > this seems to violate barrier pairing rules. > > 2. how does smp_mb protect against early reads on other CPUs? > > It normally does not: it orders reads from this CPU versus writes > > from same CPU. But init code does not appear to read anything. > > Maybe this is some s390 specific trick? > > > > I could not figure out the above commit. > > That smp_mb can be removed. The initial s390 vdso code is heavily influenced > by the powerpc version which does have a smp_wmb in vdso_init right before > the vdso_ready=1 assignment. s390 has no need for that. > > > > > arch/s390/kvm/kvm-s390.c: smp_mb(); > > > > Does not appear to be paired with anything. > > This one does not make sense to me. Imho can be removed as well. > > > arch/s390/lib/spinlock.c: smp_mb(); > > arch/s390/lib/spinlock.c: smp_mb(); > > > > Seems ok, and appears paired properly. > > Just to make sure - spinlock is not paravirtualized on s390, is it? > > s390 just uses the compare-and-swap instruction for the basic lock/unlock > operation, this implies the memory barrier. We do call the hypervisor for > contended locks if the lock can not be acquired after a number of retries. > > A while ago we did play with ticket spinlocks but they behaved badly in > out usual virtualized environments. If we find the time we might take a > closer look at the para-virtualized queued spinlocks. > > > rch/s390/kernel/time.c: smp_wmb(); > > arch/s390/kernel/time.c: smp_wmb(); > > arch/s390/kernel/time.c: smp_wmb(); > > arch/s390/kernel/time.c: smp_wmb(); > > > > It's all around vdso, so I'm guessing userspace is using this, > > this is why there's no pairing. > > Correct, this is the update count mechanics with the vdso user space code. > > > > > Some of this could be sub-optimal, but > > > > since on s390 Linux always runs on a hypervisor, > > > > I am not sure it's safe to use the generic version - > > > > in other words, it just might be that for s390 smp_ and virt_ > > > > barriers must be equivalent. > > > > > > The definition of the memory barriers is independent from the fact > > > if the system is running on an hypervisor or not. > > > Is there really > > > an architecture where you need special virt_xxx barriers?!? > > > > It is whenever host and guest or two guests access memory at > > the same time. > > > > The optimization where smp_xxx barriers are compiled out when > > CONFIG_SMP is cleared means that two UP guests running > > on an SMP host can not use smp_xxx barriers for communication. > > > > See explanation here: > > http://thread.gmane.org/gmane.linux.kernel.virtualization/26555 > > Got it, makes sense. An ack would be appreciated. > -- > blue skies, > Martin. > > "Reality continues to ruin my life." - Calvin. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Martin Schwidefsky <schwidefsky@de.ibm.com> |
|---|---|
| Date | 2016-01-05 15:30 +0100 |
| Subject | Re: [PATCH v2 22/32] s390: define __smp_xxx |
| Message-ID | <qNAMy-3Zq-33@gated-at.bofh.it> |
| In reply to | #1301516 |
On Tue, 5 Jan 2016 15:04:43 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Jan 05, 2016 at 01:08:52PM +0100, Martin Schwidefsky wrote: > > On Tue, 5 Jan 2016 11:30:19 +0200 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Tue, Jan 05, 2016 at 09:13:19AM +0100, Martin Schwidefsky wrote: > > > > On Mon, 4 Jan 2016 22:18:58 +0200 > > > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > > > On Mon, Jan 04, 2016 at 02:45:25PM +0100, Peter Zijlstra wrote: > > > > > > On Thu, Dec 31, 2015 at 09:08:38PM +0200, Michael S. Tsirkin wrote: > > > > > > > This defines __smp_xxx barriers for s390, > > > > > > > for use by virtualization. > > > > > > > > > > > > > > Some smp_xxx barriers are removed as they are > > > > > > > defined correctly by asm-generic/barriers.h > > > > > > > > > > > > > > Note: smp_mb, smp_rmb and smp_wmb are defined as full barriers > > > > > > > unconditionally on this architecture. > > > > > > > > > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > > > > --- > > > > > > > arch/s390/include/asm/barrier.h | 15 +++++++++------ > > > > > > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > > > > > > > > > > > diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h > > > > > > > index c358c31..fbd25b2 100644 > > > > > > > --- a/arch/s390/include/asm/barrier.h > > > > > > > +++ b/arch/s390/include/asm/barrier.h > > > > > > > @@ -26,18 +26,21 @@ > > > > > > > #define wmb() barrier() > > > > > > > #define dma_rmb() mb() > > > > > > > #define dma_wmb() mb() > > > > > > > -#define smp_mb() mb() > > > > > > > -#define smp_rmb() rmb() > > > > > > > -#define smp_wmb() wmb() > > > > > > > - > > > > > > > -#define smp_store_release(p, v) \ > > > > > > > +#define __smp_mb() mb() > > > > > > > +#define __smp_rmb() rmb() > > > > > > > +#define __smp_wmb() wmb() > > > > > > > +#define smp_mb() __smp_mb() > > > > > > > +#define smp_rmb() __smp_rmb() > > > > > > > +#define smp_wmb() __smp_wmb() > > > > > > > > > > > > Why define the smp_*mb() primitives here? Would not the inclusion of > > > > > > asm-generic/barrier.h do this? > > > > > > > > > > No because the generic one is a nop on !SMP, this one isn't. > > > > > > > > > > Pls note this patch is just reordering code without making > > > > > functional changes. > > > > > And at the moment, on s390 smp_xxx barriers are always non empty. > > > > > > > > The s390 kernel is SMP to 99.99%, we just didn't bother with a > > > > non-smp variant for the memory-barriers. If the generic header > > > > is used we'd get the non-smp version for free. It will save a > > > > small amount of text space for CONFIG_SMP=n. > > > > > > OK, so I'll queue a patch to do this then? > > > > Yes please. > > OK, I'll add a patch on top in v3. Good, with this addition: Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com> > > > Just to make sure: the question would be, are smp_xxx barriers ever used > > > in s390 arch specific code to flush in/out memory accesses for > > > synchronization with the hypervisor? > > > > > > I went over s390 arch code and it seems to me the answer is no > > > (except of course for virtio). > > > > Correct. Guest to host communication either uses instructions which > > imply a memory barrier or QDIO which uses atomics. > > And atomics imply a barrier on s390, right? Yes they do. > > > But I also see a lot of weirdness on this architecture. > > > > Mostly historical, s390 actually is one of the easiest architectures in > > regard to memory barriers. > > > > > I found these calls: > > > > > > arch/s390/include/asm/bitops.h: smp_mb__before_atomic(); > > > arch/s390/include/asm/bitops.h: smp_mb(); > > > > > > Not used in arch specific code so this is likely OK. > > > > This has been introduced with git commit 5402ea6af11dc5a9, the smp_mb > > and smp_mb__before_atomic are used in clear_bit_unlock and > > __clear_bit_unlock which are 1:1 copies from the code in > > include/asm-generic/bitops/lock.h. Only test_and_set_bit_lock differs > > from the generic implementation. > > something to keep in mind, but > I'd rather not touch bitops at the moment - this patchset is already too big. With the conversion smp_mb__before_atomic to a barrier() it does the correct thing. I don't think that any chance is necessary. -- blue skies, Martin. "Reality continues to ruin my life." - Calvin. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Christian Borntraeger <borntraeger@de.ibm.com> |
|---|---|
| Date | 2016-01-05 16:50 +0100 |
| Subject | Re: [PATCH v2 22/32] s390: define __smp_xxx |
| Message-ID | <qNC1Y-4OF-19@gated-at.bofh.it> |
| In reply to | #1301374 |
On 01/05/2016 10:30 AM, Michael S. Tsirkin wrote: > > arch/s390/kernel/vdso.c: smp_mb(); > > Looking at > Author: Christian Borntraeger <borntraeger@de.ibm.com> > Date: Fri Sep 11 16:23:06 2015 +0200 > > s390/vdso: use correct memory barrier > > By definition smp_wmb only orders writes against writes. (Finish all > previous writes, and do not start any future write). To protect the > vdso init code against early reads on other CPUs, let's use a full > smp_mb at the end of vdso init. As right now smp_wmb is implemented > as full serialization, this needs no stable backport, but this change > will be necessary if we reimplement smp_wmb. > > ok from hypervisor point of view, but it's also strange: > 1. why isn't this paired with another mb somewhere? > this seems to violate barrier pairing rules. > 2. how does smp_mb protect against early reads on other CPUs? > It normally does not: it orders reads from this CPU versus writes > from same CPU. But init code does not appear to read anything. > Maybe this is some s390 specific trick? > > I could not figure out the above commit. It was probably me misreading the code. I change a wmb into a full mb here since I was changing the defintion of wmb to a compiler barrier. I tried to fixup all users of wmb that really pair with other code. I assumed that there must be some reader (as there was a wmb before) but I could not figure out which. So I just played safe here. But it probably can be removed. > arch/s390/kvm/kvm-s390.c: smp_mb(); This can go. If you have a patch, I can carry that via the kvms390 tree, or I will spin a new patch with you as suggested-by. Christian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2016-01-05 17:10 +0100 |
| Subject | Re: [PATCH v2 22/32] s390: define __smp_xxx |
| Message-ID | <qNCll-5bm-39@gated-at.bofh.it> |
| In reply to | #1301629 |
On Tue, Jan 05, 2016 at 04:39:37PM +0100, Christian Borntraeger wrote: > On 01/05/2016 10:30 AM, Michael S. Tsirkin wrote: > > > > > arch/s390/kernel/vdso.c: smp_mb(); > > > > Looking at > > Author: Christian Borntraeger <borntraeger@de.ibm.com> > > Date: Fri Sep 11 16:23:06 2015 +0200 > > > > s390/vdso: use correct memory barrier > > > > By definition smp_wmb only orders writes against writes. (Finish all > > previous writes, and do not start any future write). To protect the > > vdso init code against early reads on other CPUs, let's use a full > > smp_mb at the end of vdso init. As right now smp_wmb is implemented > > as full serialization, this needs no stable backport, but this change > > will be necessary if we reimplement smp_wmb. > > > > ok from hypervisor point of view, but it's also strange: > > 1. why isn't this paired with another mb somewhere? > > this seems to violate barrier pairing rules. > > 2. how does smp_mb protect against early reads on other CPUs? > > It normally does not: it orders reads from this CPU versus writes > > from same CPU. But init code does not appear to read anything. > > Maybe this is some s390 specific trick? > > > > I could not figure out the above commit. > > It was probably me misreading the code. I change a wmb into a full mb here > since I was changing the defintion of wmb to a compiler barrier. I tried to > fixup all users of wmb that really pair with other code. I assumed that there > must be some reader (as there was a wmb before) but I could not figure out > which. So I just played safe here. > > But it probably can be removed. > > > arch/s390/kvm/kvm-s390.c: smp_mb(); > > This can go. If you have a patch, I can carry that via the kvms390 tree, > or I will spin a new patch with you as suggested-by. > > Christian I have both, will post shortly. -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2015-12-31 20:10 +0100 |
| Subject | [PATCH v2 15/32] powerpc: define __smp_xxx |
| Message-ID | <qLQLN-8pu-41@gated-at.bofh.it> |
| In reply to | #1299746 |
This defines __smp_xxx barriers for powerpc
for use by virtualization.
smp_xxx barriers are removed as they are
defined correctly by asm-generic/barriers.h
This reduces the amount of arch-specific boiler-plate code.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/powerpc/include/asm/barrier.h | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index 980ad0c..c0deafc 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -44,19 +44,11 @@
#define dma_rmb() __lwsync()
#define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
-#ifdef CONFIG_SMP
-#define smp_lwsync() __lwsync()
+#define __smp_lwsync() __lwsync()
-#define smp_mb() mb()
-#define smp_rmb() __lwsync()
-#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
-#else
-#define smp_lwsync() barrier()
-
-#define smp_mb() barrier()
-#define smp_rmb() barrier()
-#define smp_wmb() barrier()
-#endif /* CONFIG_SMP */
+#define __smp_mb() mb()
+#define __smp_rmb() __lwsync()
+#define __smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
/*
* This is a barrier which prevents following instructions from being
@@ -67,18 +59,18 @@
#define data_barrier(x) \
asm volatile("twi 0,%0,0; isync" : : "r" (x) : "memory");
-#define smp_store_release(p, v) \
+#define __smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
- smp_lwsync(); \
+ __smp_lwsync(); \
WRITE_ONCE(*p, v); \
} while (0)
-#define smp_load_acquire(p) \
+#define __smp_load_acquire(p) \
({ \
typeof(*p) ___p1 = READ_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
- smp_lwsync(); \
+ __smp_lwsync(); \
___p1; \
})
--
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Boqun Feng <boqun.feng@gmail.com> |
|---|---|
| Date | 2016-01-05 02:40 +0100 |
| Subject | Re: [PATCH v2 15/32] powerpc: define __smp_xxx |
| Message-ID | <qNoLo-3er-17@gated-at.bofh.it> |
| In reply to | #1299751 |
Hi Michael,
On Thu, Dec 31, 2015 at 09:07:42PM +0200, Michael S. Tsirkin wrote:
> This defines __smp_xxx barriers for powerpc
> for use by virtualization.
>
> smp_xxx barriers are removed as they are
> defined correctly by asm-generic/barriers.h
>
> This reduces the amount of arch-specific boiler-plate code.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/powerpc/include/asm/barrier.h | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
> index 980ad0c..c0deafc 100644
> --- a/arch/powerpc/include/asm/barrier.h
> +++ b/arch/powerpc/include/asm/barrier.h
> @@ -44,19 +44,11 @@
> #define dma_rmb() __lwsync()
> #define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
>
> -#ifdef CONFIG_SMP
> -#define smp_lwsync() __lwsync()
> +#define __smp_lwsync() __lwsync()
>
so __smp_lwsync() is always mapped to lwsync, right?
> -#define smp_mb() mb()
> -#define smp_rmb() __lwsync()
> -#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> -#else
> -#define smp_lwsync() barrier()
> -
> -#define smp_mb() barrier()
> -#define smp_rmb() barrier()
> -#define smp_wmb() barrier()
> -#endif /* CONFIG_SMP */
> +#define __smp_mb() mb()
> +#define __smp_rmb() __lwsync()
> +#define __smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
>
> /*
> * This is a barrier which prevents following instructions from being
> @@ -67,18 +59,18 @@
> #define data_barrier(x) \
> asm volatile("twi 0,%0,0; isync" : : "r" (x) : "memory");
>
> -#define smp_store_release(p, v) \
> +#define __smp_store_release(p, v) \
> do { \
> compiletime_assert_atomic_type(*p); \
> - smp_lwsync(); \
> + __smp_lwsync(); \
, therefore this will emit an lwsync no matter SMP or UP.
Another thing is that smp_lwsync() may have a third user(other than
smp_load_acquire() and smp_store_release()):
http://article.gmane.org/gmane.linux.ports.ppc.embedded/89877
I'm OK to change my patch accordingly, but do we really want
smp_lwsync() get involved in this cleanup? If I understand you
correctly, this cleanup focuses on external API like smp_{r,w,}mb(),
while smp_lwsync() is internal to PPC.
Regards,
Boqun
> WRITE_ONCE(*p, v); \
> } while (0)
>
> -#define smp_load_acquire(p) \
> +#define __smp_load_acquire(p) \
> ({ \
> typeof(*p) ___p1 = READ_ONCE(*p); \
> compiletime_assert_atomic_type(*p); \
> - smp_lwsync(); \
> + __smp_lwsync(); \
> ___p1; \
> })
>
> --
> MST
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2016-01-05 10:00 +0100 |
| Subject | Re: [PATCH v2 15/32] powerpc: define __smp_xxx |
| Message-ID | <qNvDc-84L-21@gated-at.bofh.it> |
| In reply to | #1301226 |
On Tue, Jan 05, 2016 at 09:36:55AM +0800, Boqun Feng wrote:
> Hi Michael,
>
> On Thu, Dec 31, 2015 at 09:07:42PM +0200, Michael S. Tsirkin wrote:
> > This defines __smp_xxx barriers for powerpc
> > for use by virtualization.
> >
> > smp_xxx barriers are removed as they are
> > defined correctly by asm-generic/barriers.h
I think this is the part that was missed in review.
> > This reduces the amount of arch-specific boiler-plate code.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > arch/powerpc/include/asm/barrier.h | 24 ++++++++----------------
> > 1 file changed, 8 insertions(+), 16 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
> > index 980ad0c..c0deafc 100644
> > --- a/arch/powerpc/include/asm/barrier.h
> > +++ b/arch/powerpc/include/asm/barrier.h
> > @@ -44,19 +44,11 @@
> > #define dma_rmb() __lwsync()
> > #define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> >
> > -#ifdef CONFIG_SMP
> > -#define smp_lwsync() __lwsync()
> > +#define __smp_lwsync() __lwsync()
> >
>
> so __smp_lwsync() is always mapped to lwsync, right?
Yes.
> > -#define smp_mb() mb()
> > -#define smp_rmb() __lwsync()
> > -#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> > -#else
> > -#define smp_lwsync() barrier()
> > -
> > -#define smp_mb() barrier()
> > -#define smp_rmb() barrier()
> > -#define smp_wmb() barrier()
> > -#endif /* CONFIG_SMP */
> > +#define __smp_mb() mb()
> > +#define __smp_rmb() __lwsync()
> > +#define __smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> >
> > /*
> > * This is a barrier which prevents following instructions from being
> > @@ -67,18 +59,18 @@
> > #define data_barrier(x) \
> > asm volatile("twi 0,%0,0; isync" : : "r" (x) : "memory");
> >
> > -#define smp_store_release(p, v) \
> > +#define __smp_store_release(p, v) \
> > do { \
> > compiletime_assert_atomic_type(*p); \
> > - smp_lwsync(); \
> > + __smp_lwsync(); \
>
> , therefore this will emit an lwsync no matter SMP or UP.
Absolutely. But smp_store_release (without __) will not.
Please note I did test this: for ppc code before and after
this patch generates exactly the same binary on SMP and UP.
> Another thing is that smp_lwsync() may have a third user(other than
> smp_load_acquire() and smp_store_release()):
>
> http://article.gmane.org/gmane.linux.ports.ppc.embedded/89877
>
> I'm OK to change my patch accordingly, but do we really want
> smp_lwsync() get involved in this cleanup? If I understand you
> correctly, this cleanup focuses on external API like smp_{r,w,}mb(),
> while smp_lwsync() is internal to PPC.
>
> Regards,
> Boqun
I think you missed the leading ___ :)
smp_store_release is external and it needs __smp_lwsync as
defined here.
I can duplicate some code and have smp_lwsync *not* call __smp_lwsync
but why do this? Still, if you prefer it this way,
please let me know.
> > WRITE_ONCE(*p, v); \
> > } while (0)
> >
> > -#define smp_load_acquire(p) \
> > +#define __smp_load_acquire(p) \
> > ({ \
> > typeof(*p) ___p1 = READ_ONCE(*p); \
> > compiletime_assert_atomic_type(*p); \
> > - smp_lwsync(); \
> > + __smp_lwsync(); \
> > ___p1; \
> > })
> >
> > --
> > MST
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Boqun Feng <boqun.feng@gmail.com> |
|---|---|
| Date | 2016-01-05 11:00 +0100 |
| Subject | Re: [PATCH v2 15/32] powerpc: define __smp_xxx |
| Message-ID | <qNwzf-j5-5@gated-at.bofh.it> |
| In reply to | #1301335 |
On Tue, Jan 05, 2016 at 10:51:17AM +0200, Michael S. Tsirkin wrote:
> On Tue, Jan 05, 2016 at 09:36:55AM +0800, Boqun Feng wrote:
> > Hi Michael,
> >
> > On Thu, Dec 31, 2015 at 09:07:42PM +0200, Michael S. Tsirkin wrote:
> > > This defines __smp_xxx barriers for powerpc
> > > for use by virtualization.
> > >
> > > smp_xxx barriers are removed as they are
> > > defined correctly by asm-generic/barriers.h
>
> I think this is the part that was missed in review.
>
Yes, I realized my mistake after reread the series. But smp_lwsync() is
not defined in asm-generic/barriers.h, right?
> > > This reduces the amount of arch-specific boiler-plate code.
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > > arch/powerpc/include/asm/barrier.h | 24 ++++++++----------------
> > > 1 file changed, 8 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
> > > index 980ad0c..c0deafc 100644
> > > --- a/arch/powerpc/include/asm/barrier.h
> > > +++ b/arch/powerpc/include/asm/barrier.h
> > > @@ -44,19 +44,11 @@
> > > #define dma_rmb() __lwsync()
> > > #define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> > >
> > > -#ifdef CONFIG_SMP
> > > -#define smp_lwsync() __lwsync()
> > > +#define __smp_lwsync() __lwsync()
> > >
> >
> > so __smp_lwsync() is always mapped to lwsync, right?
>
> Yes.
>
> > > -#define smp_mb() mb()
> > > -#define smp_rmb() __lwsync()
> > > -#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> > > -#else
> > > -#define smp_lwsync() barrier()
> > > -
> > > -#define smp_mb() barrier()
> > > -#define smp_rmb() barrier()
> > > -#define smp_wmb() barrier()
> > > -#endif /* CONFIG_SMP */
> > > +#define __smp_mb() mb()
> > > +#define __smp_rmb() __lwsync()
> > > +#define __smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> > >
> > > /*
> > > * This is a barrier which prevents following instructions from being
> > > @@ -67,18 +59,18 @@
> > > #define data_barrier(x) \
> > > asm volatile("twi 0,%0,0; isync" : : "r" (x) : "memory");
> > >
> > > -#define smp_store_release(p, v) \
> > > +#define __smp_store_release(p, v) \
> > > do { \
> > > compiletime_assert_atomic_type(*p); \
> > > - smp_lwsync(); \
> > > + __smp_lwsync(); \
> >
> > , therefore this will emit an lwsync no matter SMP or UP.
>
> Absolutely. But smp_store_release (without __) will not.
>
> Please note I did test this: for ppc code before and after
> this patch generates exactly the same binary on SMP and UP.
>
Yes, you're right, sorry for my mistake...
>
> > Another thing is that smp_lwsync() may have a third user(other than
> > smp_load_acquire() and smp_store_release()):
> >
> > http://article.gmane.org/gmane.linux.ports.ppc.embedded/89877
> >
> > I'm OK to change my patch accordingly, but do we really want
> > smp_lwsync() get involved in this cleanup? If I understand you
> > correctly, this cleanup focuses on external API like smp_{r,w,}mb(),
> > while smp_lwsync() is internal to PPC.
> >
> > Regards,
> > Boqun
>
> I think you missed the leading ___ :)
>
What I mean here was smp_lwsync() was originally internal to PPC, but
never mind ;-)
> smp_store_release is external and it needs __smp_lwsync as
> defined here.
>
> I can duplicate some code and have smp_lwsync *not* call __smp_lwsync
You mean bringing smp_lwsync() back? because I haven't seen you defining
in asm-generic/barriers.h in previous patches and you just delete it in
this patch.
> but why do this? Still, if you prefer it this way,
> please let me know.
>
I think deleting smp_lwsync() is fine, though I need to change atomic
variants patches on PPC because of it ;-/
Regards,
Boqun
> > > WRITE_ONCE(*p, v); \
> > > } while (0)
> > >
> > > -#define smp_load_acquire(p) \
> > > +#define __smp_load_acquire(p) \
> > > ({ \
> > > typeof(*p) ___p1 = READ_ONCE(*p); \
> > > compiletime_assert_atomic_type(*p); \
> > > - smp_lwsync(); \
> > > + __smp_lwsync(); \
> > > ___p1; \
> > > })
> > >
> > > --
> > > MST
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2016-01-05 17:20 +0100 |
| Subject | Re: [PATCH v2 15/32] powerpc: define __smp_xxx |
| Message-ID | <qNCv1-5eI-23@gated-at.bofh.it> |
| In reply to | #1301382 |
On Tue, Jan 05, 2016 at 05:53:41PM +0800, Boqun Feng wrote:
> On Tue, Jan 05, 2016 at 10:51:17AM +0200, Michael S. Tsirkin wrote:
> > On Tue, Jan 05, 2016 at 09:36:55AM +0800, Boqun Feng wrote:
> > > Hi Michael,
> > >
> > > On Thu, Dec 31, 2015 at 09:07:42PM +0200, Michael S. Tsirkin wrote:
> > > > This defines __smp_xxx barriers for powerpc
> > > > for use by virtualization.
> > > >
> > > > smp_xxx barriers are removed as they are
> > > > defined correctly by asm-generic/barriers.h
> >
> > I think this is the part that was missed in review.
> >
>
> Yes, I realized my mistake after reread the series. But smp_lwsync() is
> not defined in asm-generic/barriers.h, right?
It isn't because as far as I could tell it is not used
outside arch/powerpc/include/asm/barrier.h
smp_store_release and smp_load_acquire.
And these are now gone.
Instead there are __smp_store_release and __smp_load_acquire
which call __smp_lwsync.
These are only used for virt and on SMP.
UP variants are generic - they just call barrier().
> > > > This reduces the amount of arch-specific boiler-plate code.
> > > >
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > > ---
> > > > arch/powerpc/include/asm/barrier.h | 24 ++++++++----------------
> > > > 1 file changed, 8 insertions(+), 16 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
> > > > index 980ad0c..c0deafc 100644
> > > > --- a/arch/powerpc/include/asm/barrier.h
> > > > +++ b/arch/powerpc/include/asm/barrier.h
> > > > @@ -44,19 +44,11 @@
> > > > #define dma_rmb() __lwsync()
> > > > #define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> > > >
> > > > -#ifdef CONFIG_SMP
> > > > -#define smp_lwsync() __lwsync()
> > > > +#define __smp_lwsync() __lwsync()
> > > >
> > >
> > > so __smp_lwsync() is always mapped to lwsync, right?
> >
> > Yes.
> >
> > > > -#define smp_mb() mb()
> > > > -#define smp_rmb() __lwsync()
> > > > -#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> > > > -#else
> > > > -#define smp_lwsync() barrier()
> > > > -
> > > > -#define smp_mb() barrier()
> > > > -#define smp_rmb() barrier()
> > > > -#define smp_wmb() barrier()
> > > > -#endif /* CONFIG_SMP */
> > > > +#define __smp_mb() mb()
> > > > +#define __smp_rmb() __lwsync()
> > > > +#define __smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> > > >
> > > > /*
> > > > * This is a barrier which prevents following instructions from being
> > > > @@ -67,18 +59,18 @@
> > > > #define data_barrier(x) \
> > > > asm volatile("twi 0,%0,0; isync" : : "r" (x) : "memory");
> > > >
> > > > -#define smp_store_release(p, v) \
> > > > +#define __smp_store_release(p, v) \
> > > > do { \
> > > > compiletime_assert_atomic_type(*p); \
> > > > - smp_lwsync(); \
> > > > + __smp_lwsync(); \
> > >
> > > , therefore this will emit an lwsync no matter SMP or UP.
> >
> > Absolutely. But smp_store_release (without __) will not.
> >
> > Please note I did test this: for ppc code before and after
> > this patch generates exactly the same binary on SMP and UP.
> >
>
> Yes, you're right, sorry for my mistake...
>
> >
> > > Another thing is that smp_lwsync() may have a third user(other than
> > > smp_load_acquire() and smp_store_release()):
> > >
> > > http://article.gmane.org/gmane.linux.ports.ppc.embedded/89877
> > >
> > > I'm OK to change my patch accordingly, but do we really want
> > > smp_lwsync() get involved in this cleanup? If I understand you
> > > correctly, this cleanup focuses on external API like smp_{r,w,}mb(),
> > > while smp_lwsync() is internal to PPC.
> > >
> > > Regards,
> > > Boqun
> >
> > I think you missed the leading ___ :)
> >
>
> What I mean here was smp_lwsync() was originally internal to PPC, but
> never mind ;-)
>
> > smp_store_release is external and it needs __smp_lwsync as
> > defined here.
> >
> > I can duplicate some code and have smp_lwsync *not* call __smp_lwsync
>
> You mean bringing smp_lwsync() back? because I haven't seen you defining
> in asm-generic/barriers.h in previous patches and you just delete it in
> this patch.
>
> > but why do this? Still, if you prefer it this way,
> > please let me know.
> >
>
> I think deleting smp_lwsync() is fine, though I need to change atomic
> variants patches on PPC because of it ;-/
>
> Regards,
> Boqun
Sorry, I don't understand - why do you have to do anything?
I changed all users of smp_lwsync so they
use __smp_lwsync on SMP and barrier() on !SMP.
This is exactly the current behaviour, I also tested that
generated code does not change at all.
Is there a patch in your tree that conflicts with this?
> > > > WRITE_ONCE(*p, v); \
> > > > } while (0)
> > > >
> > > > -#define smp_load_acquire(p) \
> > > > +#define __smp_load_acquire(p) \
> > > > ({ \
> > > > typeof(*p) ___p1 = READ_ONCE(*p); \
> > > > compiletime_assert_atomic_type(*p); \
> > > > - smp_lwsync(); \
> > > > + __smp_lwsync(); \
> > > > ___p1; \
> > > > })
> > > >
> > > > --
> > > > MST
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > > Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
Page 1 of 4 [1] 2 3 4 Next page →
Back to top | Article view | linux.kernel
csiph-web