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 at redhat.com> > Acked-by: Arnd Bergmann <arnd at 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?
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 at redhat.com> > > Acked-by: Arnd Bergmann <arnd at 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
On Mon, 4 Jan 2016 22:18:58 +0200 "Michael S. Tsirkin" <mst at 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 at redhat.com> > > > Acked-by: Arnd Bergmann <arnd at 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.