Michael S. Tsirkin
2018-Apr-19 17:35 UTC
[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg
virtio is using barriers to order memory accesses, thus dma_wmb/rmb is a good match. Build-tested on x86: Before [mst at tuck linux]$ size drivers/virtio/virtio_ring.o text data bss dec hex filename 11392 820 0 12212 2fb4 drivers/virtio/virtio_ring.o After mst at tuck linux]$ size drivers/virtio/virtio_ring.o text data bss dec hex filename 11284 820 0 12104 2f48 drivers/virtio/virtio_ring.o Cc: Ohad Ben-Cohen <ohad at wizery.com> Cc: Bjorn Andersson <bjorn.andersson at linaro.org> Cc: linux-remoteproc at vger.kernel.org Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- It's good in theory, but could one of RPMSG maintainers please review and ack this patch? Or even better test it? All these barriers are useless on Intel anyway ... include/linux/virtio_ring.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index bbf3252..fab0213 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -35,7 +35,7 @@ static inline void virtio_rmb(bool weak_barriers) if (weak_barriers) virt_rmb(); else - rmb(); + dma_rmb(); } static inline void virtio_wmb(bool weak_barriers) @@ -43,7 +43,7 @@ static inline void virtio_wmb(bool weak_barriers) if (weak_barriers) virt_wmb(); else - wmb(); + dma_wmb(); } static inline void virtio_store_mb(bool weak_barriers, -- MST
Paolo Bonzini
2018-Apr-19 17:39 UTC
[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg
On 19/04/2018 19:35, Michael S. Tsirkin wrote:> virtio is using barriers to order memory accesses, thus > dma_wmb/rmb is a good match. > > Build-tested on x86: Before > > [mst at tuck linux]$ size drivers/virtio/virtio_ring.o > text data bss dec hex filename > 11392 820 0 12212 2fb4 drivers/virtio/virtio_ring.o > > After > mst at tuck linux]$ size drivers/virtio/virtio_ring.o > text data bss dec hex filename > 11284 820 0 12104 2f48 drivers/virtio/virtio_ring.o > > Cc: Ohad Ben-Cohen <ohad at wizery.com> > Cc: Bjorn Andersson <bjorn.andersson at linaro.org> > Cc: linux-remoteproc at vger.kernel.org > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > > It's good in theory, but could one of RPMSG maintainers please review > and ack this patch? Or even better test it? > > All these barriers are useless on Intel anyway ...This should be okay, but I wonder if there should be a virtio_wmb(...) or an "if (weak_barriers) wmb()" before the "writel" in vm_notify (drivers/virtio/virtio_mmio.c). Thanks, Paolo> > include/linux/virtio_ring.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index bbf3252..fab0213 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -35,7 +35,7 @@ static inline void virtio_rmb(bool weak_barriers) > if (weak_barriers) > virt_rmb(); > else > - rmb(); > + dma_rmb(); > } > > static inline void virtio_wmb(bool weak_barriers) > @@ -43,7 +43,7 @@ static inline void virtio_wmb(bool weak_barriers) > if (weak_barriers) > virt_wmb(); > else > - wmb(); > + dma_wmb(); > } > > static inline void virtio_store_mb(bool weak_barriers, >
Michael S. Tsirkin
2018-Apr-19 17:46 UTC
[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg
On Thu, Apr 19, 2018 at 07:39:21PM +0200, Paolo Bonzini wrote:> On 19/04/2018 19:35, Michael S. Tsirkin wrote: > > virtio is using barriers to order memory accesses, thus > > dma_wmb/rmb is a good match. > > > > Build-tested on x86: Before > > > > [mst at tuck linux]$ size drivers/virtio/virtio_ring.o > > text data bss dec hex filename > > 11392 820 0 12212 2fb4 drivers/virtio/virtio_ring.o > > > > After > > mst at tuck linux]$ size drivers/virtio/virtio_ring.o > > text data bss dec hex filename > > 11284 820 0 12104 2f48 drivers/virtio/virtio_ring.o > > > > Cc: Ohad Ben-Cohen <ohad at wizery.com> > > Cc: Bjorn Andersson <bjorn.andersson at linaro.org> > > Cc: linux-remoteproc at vger.kernel.org > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > > --- > > > > It's good in theory, but could one of RPMSG maintainers please review > > and ack this patch? Or even better test it? > > > > All these barriers are useless on Intel anyway ... > > This should be okay, but I wonder if there should be a virtio_wmb(...) > or an "if (weak_barriers) wmb()" before the "writel" in vm_notify > (drivers/virtio/virtio_mmio.c). > > Thanks, > > PaoloThat one uses weak barriers AFAIK. IIUC you mean rproc_virtio_notify. I suspect it works because specific kick callbacks have a barrier internally.> > > > include/linux/virtio_ring.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > > index bbf3252..fab0213 100644 > > --- a/include/linux/virtio_ring.h > > +++ b/include/linux/virtio_ring.h > > @@ -35,7 +35,7 @@ static inline void virtio_rmb(bool weak_barriers) > > if (weak_barriers) > > virt_rmb(); > > else > > - rmb(); > > + dma_rmb(); > > } > > > > static inline void virtio_wmb(bool weak_barriers) > > @@ -43,7 +43,7 @@ static inline void virtio_wmb(bool weak_barriers) > > if (weak_barriers) > > virt_wmb(); > > else > > - wmb(); > > + dma_wmb(); > > } > > > > static inline void virtio_store_mb(bool weak_barriers, > >
On 2018?04?20? 01:35, Michael S. Tsirkin wrote:> virtio is using barriers to order memory accesses, thus > dma_wmb/rmb is a good match. > > Build-tested on x86: Before > > [mst at tuck linux]$ size drivers/virtio/virtio_ring.o > text data bss dec hex filename > 11392 820 0 12212 2fb4 drivers/virtio/virtio_ring.o > > After > mst at tuck linux]$ size drivers/virtio/virtio_ring.o > text data bss dec hex filename > 11284 820 0 12104 2f48 drivers/virtio/virtio_ring.o > > Cc: Ohad Ben-Cohen <ohad at wizery.com> > Cc: Bjorn Andersson <bjorn.andersson at linaro.org> > Cc: linux-remoteproc at vger.kernel.org > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > > It's good in theory, but could one of RPMSG maintainers please review > and ack this patch? Or even better test it? > > All these barriers are useless on Intel anyway ... > > include/linux/virtio_ring.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index bbf3252..fab0213 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -35,7 +35,7 @@ static inline void virtio_rmb(bool weak_barriers) > if (weak_barriers) > virt_rmb(); > else > - rmb(); > + dma_rmb(); > } > > static inline void virtio_wmb(bool weak_barriers) > @@ -43,7 +43,7 @@ static inline void virtio_wmb(bool weak_barriers) > if (weak_barriers) > virt_wmb(); > else > - wmb(); > + dma_wmb(); > } > > static inline void virtio_store_mb(bool weak_barriers,Acked-by: Jason Wang <jasowang at redhat.com>
Bjorn Andersson
2018-Jun-04 20:02 UTC
[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg
On Thu 19 Apr 10:35 PDT 2018, Michael S. Tsirkin wrote:> virtio is using barriers to order memory accesses, thus > dma_wmb/rmb is a good match. > > Build-tested on x86: Before > > [mst at tuck linux]$ size drivers/virtio/virtio_ring.o > text data bss dec hex filename > 11392 820 0 12212 2fb4 drivers/virtio/virtio_ring.o > > After > mst at tuck linux]$ size drivers/virtio/virtio_ring.o > text data bss dec hex filename > 11284 820 0 12104 2f48 drivers/virtio/virtio_ring.o > > Cc: Ohad Ben-Cohen <ohad at wizery.com> > Cc: Bjorn Andersson <bjorn.andersson at linaro.org> > Cc: linux-remoteproc at vger.kernel.org > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>Acked-by: Bjorn Andersson <bjorn.andersson at linaro.org> Regards, Bjorn> --- > > It's good in theory, but could one of RPMSG maintainers please review > and ack this patch? Or even better test it? > > All these barriers are useless on Intel anyway ... > > include/linux/virtio_ring.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index bbf3252..fab0213 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -35,7 +35,7 @@ static inline void virtio_rmb(bool weak_barriers) > if (weak_barriers) > virt_rmb(); > else > - rmb(); > + dma_rmb(); > } > > static inline void virtio_wmb(bool weak_barriers) > @@ -43,7 +43,7 @@ static inline void virtio_wmb(bool weak_barriers) > if (weak_barriers) > virt_wmb(); > else > - wmb(); > + dma_wmb(); > } > > static inline void virtio_store_mb(bool weak_barriers, > -- > MST
Maybe Matching Threads
- [PATCH] virtio_ring: switch to dma_XX barriers for rpmsg
- [PATCH] virtio_ring: switch to dma_XX barriers for rpmsg
- [PATCH] virtio_ring: switch to dma_XX barriers for rpmsg
- [PATCH] virtio_ring: switch to dma_XX barriers for rpmsg
- [PATCH v2 00/34] arch: barrier cleanup + barriers for virt