* Re: [PATCH RFC] vhost: basic device IOTLB support
From: Jason Wang @ 2016-01-06 4:57 UTC (permalink / raw)
To: Yang Zhang, mst, kvm, virtualization, netdev, linux-kernel,
linux-api
In-Reply-To: <568B35F8.7080302@gmail.com>
On 01/05/2016 11:18 AM, Yang Zhang wrote:
> On 2016/1/4 14:22, Jason Wang wrote:
>>
>>
>> On 01/04/2016 09:39 AM, Yang Zhang wrote:
>>> On 2015/12/31 15:13, Jason Wang wrote:
>>>> This patch tries to implement an device IOTLB for vhost. This could be
>>>> used with for co-operation with userspace(qemu) implementation of
>>>> iommu for a secure DMA environment in guest.
>>>>
>>>> The idea is simple. When vhost meets an IOTLB miss, it will request
>>>> the assistance of userspace to do the translation, this is done
>>>> through:
>>>>
>>>> - Fill the translation request in a preset userspace address (This
>>>> address is set through ioctl VHOST_SET_IOTLB_REQUEST_ENTRY).
>>>> - Notify userspace through eventfd (This eventfd was set through ioctl
>>>> VHOST_SET_IOTLB_FD).
>>>>
>>>> When userspace finishes the translation, it will update the vhost
>>>> IOTLB through VHOST_UPDATE_IOTLB ioctl. Userspace is also in charge of
>>>> snooping the IOTLB invalidation of IOMMU IOTLB and use
>>>> VHOST_UPDATE_IOTLB to invalidate the possible entry in vhost.
>>>
>>> Is there any performance data shows the difference with IOTLB
>>> supporting?
>>
>> Basic testing show it was slower than without IOTLB.
>>
>>> I doubt we may see performance decrease since the flush code path is
>>> longer than before.
>>>
>>
>> Yes, it also depend on the TLB hit rate.
>>
>> If lots of dynamic mappings and unmappings are used in guest (e.g normal
>> Linux driver). This method should be much more slower since:
>>
>> - lots of invalidation and its path is slow.
>> - the hit rate is low and the high price of userspace assisted address
>> translation.
>> - limitation of userspace IOMMU/IOTLB implementation (qemu's vtd
>> emulation simply empty all entries when it's full).
>>
>> Another method is to implement kernel IOMMU (e.g vtd). But I'm not sure
>> vhost is the best place to do this, since vhost should be architecture
>> independent. Maybe we'd better to do it in kvm or have a pv IOMMU
>> implementation in vhost.
>
> Actually, i have the kernel IOMMU(virtual vtd) patch which can pass
> though the physical device to L2 guest on hand.
A little bit confused, I believe the first step is to exporting an IOMMU
to L1 guest for it to use for a assigned device?
> But it is just a draft patch which was written several years ago. If
> there is real requirement for it, I can rebase it and send out it for
> review.
Interesting but I think the goal is different. This patch tries to make
vhost/virtio works with emulated IOMMU.
>
>>
>> Another side, if fixed mappings were used in guest, (e.g dpdk in guest).
>> We have the possibility to have 100% hit rate with almost no
>> invalidation, the performance penalty should be ignorable, this should
>> be the main use case for this patch.
>>
>> The patch is just a prototype for discussion. Any other ideas are
>> welcomed.
>>
>> Thanks
>>
>
>
^ permalink raw reply
* Re: [PATCH v2 15/32] powerpc: define __smp_xxx
From: Boqun Feng @ 2016-01-06 1:51 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra,
Benjamin Herrenschmidt, virtualization, Paul Mackerras,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Davidlohr Bueso, Arnd Bergmann, Michael Ellerman, x86, xen-devel,
Ingo Molnar, Paul E. McKenney, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, Andrey Konovalov,
adi-buildroot-devel, Thomas Gleixner
In-Reply-To: <20160105180938-mutt-send-email-mst@redhat.com>
On Tue, Jan 05, 2016 at 06:16:48PM +0200, Michael S. Tsirkin wrote:
[snip]
> > > > 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?
>
Because in a patchset which implements atomic relaxed/acquire/release
variants on PPC I use smp_lwsync(), this makes it have another user,
please see this mail:
http://article.gmane.org/gmane.linux.ports.ppc.embedded/89877
in definition of PPC's __atomic_op_release().
But I think removing smp_lwsync() is a good idea and actually I think we
can go further to remove __smp_lwsync() and let __smp_load_acquire and
__smp_store_release call __lwsync() directly, but that is another thing.
Anyway, I will modify my patch.
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/
^ permalink raw reply
* Re: [PATCH v2 12/32] x86/um: reuse asm-generic/barrier.h
From: Richard Weinberger @ 2016-01-05 23:12 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: linux-mips, linux-ia64, user-mode-linux-user, linux-sh,
Peter Zijlstra, virtualization, H. Peter Anvin, sparclinux,
linux-arch, linux-s390, Arnd Bergmann, x86, Ingo Molnar,
xen-devel, Ingo Molnar, Borislav Petkov, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, Jeff Dike,
adi-buildroot-devel, Andy Lutomirski, Thomas Gleixner,
linux-metag, linux-arm-kernel, Andrew Cooper
In-Reply-To: <1451572003-2440-13-git-send-email-mst@redhat.com>
Am 31.12.2015 um 20:07 schrieb Michael S. Tsirkin:
> On x86/um CONFIG_SMP is never defined. As a result, several macros
> match the asm-generic variant 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>
Acked-by: Richard Weinberger <richard@nod.at>
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH v2 15/32] powerpc: define __smp_xxx
From: Michael S. Tsirkin @ 2016-01-05 16:16 UTC (permalink / raw)
To: Boqun Feng
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra,
Benjamin Herrenschmidt, virtualization, Paul Mackerras,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Davidlohr Bueso, Arnd Bergmann, Michael Ellerman, x86, xen-devel,
Ingo Molnar, Paul E. McKenney, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, Andrey Konovalov,
adi-buildroot-devel, Thomas Gleixner
In-Reply-To: <20160105095341.GA5321@fixme-laptop.cn.ibm.com>
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/
^ permalink raw reply
* Re: [PATCH v2 22/32] s390: define __smp_xxx
From: Michael S. Tsirkin @ 2016-01-05 16:04 UTC (permalink / raw)
To: Christian Borntraeger
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Heiko Carstens,
virtualization, H. Peter Anvin, sparclinux, Ingo Molnar,
linux-arch, linux-s390, Davidlohr Bueso, Arnd Bergmann, x86,
xen-devel, Ingo Molnar, linux-xtensa, user-mode-linux-devel,
Stefano Stabellini, Andrey Konovalov, adi-buildroot-devel,
Thomas Gleixner, linux-metag, linux-arm-kernel, Andrew Cooper,
linux-kernel
In-Reply-To: <568BE3B9.8020901@de.ibm.com>
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
^ permalink raw reply
* Re: [PATCH v2 22/32] s390: define __smp_xxx
From: Christian Borntraeger @ 2016-01-05 15:39 UTC (permalink / raw)
To: Michael S. Tsirkin, Martin Schwidefsky
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Heiko Carstens,
virtualization, H. Peter Anvin, sparclinux, Ingo Molnar,
linux-arch, linux-s390, Davidlohr Bueso, Arnd Bergmann, x86,
xen-devel, Ingo Molnar, linux-xtensa, user-mode-linux-devel,
Stefano Stabellini, Andrey Konovalov, adi-buildroot-devel,
Thomas Gleixner, linux-metag, linux-arm-kernel, Andrew Cooper,
linux-kernel
In-Reply-To: <20160105105335-mutt-send-email-mst@redhat.com>
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
^ permalink raw reply
* Re: [PATCH v4 2/2] virtio_balloon: Use a workqueue instead of "vballoon" kthread
From: Michael S. Tsirkin @ 2016-01-05 15:37 UTC (permalink / raw)
To: Petr Mladek
Cc: Jiri Kosina, linux-kernel, virtualization, Tejun Heo, Jeff Epler
In-Reply-To: <20160105144918.GF3178@pathway.suse.cz>
On Tue, Jan 05, 2016 at 03:49:18PM +0100, Petr Mladek wrote:
> On Sat 2016-01-02 23:36:03, Michael S. Tsirkin wrote:
> > On Sat, Jan 02, 2016 at 06:43:16AM -0500, Tejun Heo wrote:
> > > On Fri, Jan 01, 2016 at 12:18:17PM +0200, Michael S. Tsirkin wrote:
> > > > > My initial idea was to use a dedicated workqueue. Michael S. Tsirkin
> > > > > @@ -563,7 +534,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
> > > > > struct virtio_balloon *vb = vdev->priv;
> > > > >
> > > > > unregister_oom_notifier(&vb->nb);
> > > > > - kthread_stop(vb->thread);
> > > > > + cancel_work_sync(&vb->wq_work);
> > > >
> > > > OK but since job requeues itself, cancelling like this might not be enough.
> > >
> > > As long as there's no further external queueing, cancel_work_sync() is
> > > guaranteed to kill a self-requeueing work item.
> > >
> > > Thanks.
> >
> > I didn't realise this. Thanks!
> >
> > Unfortunately in this case, there can be further requeueing
> > if a stats request arrives.
>
> Please, is there any point where the stat requests are disabled for
> sure? I am not 100% sure but it might be after the reset() call:
>
> vb->vdev->config->reset(vb->vdev);
Yes.
> Then we could split the kthread into two works: resizing and stats.
> The resizing work still must be canceled before leaking the balloon.
> But the stats work might be canceled after the reset() call.
>
> In fact, the solution with the two works looks even cleaner.
>
>
> Thanks for feedback,
> Petr
I agree - in fact, not blocking stats call while inflate is blocked
would be very nice. As things then happen in parallel, we need to be
careful with locking and stuff.
That would be a good reason to switch to wq.
--
MST
^ permalink raw reply
* Re: [PATCH RFC] virtio_balloon: refill by config handler
From: Michael S. Tsirkin @ 2016-01-05 15:31 UTC (permalink / raw)
To: Petr Mladek
Cc: Tejun Heo, Jiri Kosina, Jeff Epler, linux-kernel, virtualization
In-Reply-To: <20160105143733.GE3178@pathway.suse.cz>
On Tue, Jan 05, 2016 at 03:37:33PM +0100, Petr Mladek wrote:
> On Fri 2016-01-01 12:13:48, Michael S. Tsirkin wrote:
> > Peter Mladek reported that balloon might never refill completely after
> > restore. This is because fill_balloon is only called once there.
> > Calling fill_balloon repeatedly seems too aggressive,
> > especially in light of the fact that it sleeps on failure: let's
> > wake the config change handler and fill it asynchronously.
> >
> > Reported-by: Petr Mladek <pmladek@suse.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > I was unable to test this - for some reason my test VM
> > doesn't resume (with or without the patch).
> > Petr, does this work for you?
>
> Ah, I have just realized that the problem happens only after the
> conversion to the workqueue. It works without this patch with the
> current code. The balloon kthread is waken when released from
> __refrigerator().
In fact, maybe we should drop fill_balloon and
update_balloon_size here?
Resume will finish faster ...
> I am sorry for the noise. I wanted to split the conversion to
> workqueues into two patches to make it better readable. We need to
> queue the balloon resizing work here. I wrongly assumed that the
> original kthread must have the same problem.
>
> Best Regards,
> Petr
>
> > drivers/virtio/virtio_balloon.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > index 7efc329..ee29473 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -589,7 +589,7 @@ static int virtballoon_restore(struct virtio_device *vdev)
> >
> > virtio_device_ready(vdev);
> >
> > - fill_balloon(vb, towards_target(vb));
> > + wake_up(&vb->config_change);
> > update_balloon_size(vb);
> > return 0;
> > }
> > --
> > MST
^ permalink raw reply
* Re: [PATCH v4 2/2] virtio_balloon: Use a workqueue instead of "vballoon" kthread
From: Petr Mladek @ 2016-01-05 14:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jiri Kosina, linux-kernel, virtualization, Tejun Heo, Jeff Epler
In-Reply-To: <20160102213603.GA1703@redhat.com>
On Sat 2016-01-02 23:36:03, Michael S. Tsirkin wrote:
> On Sat, Jan 02, 2016 at 06:43:16AM -0500, Tejun Heo wrote:
> > On Fri, Jan 01, 2016 at 12:18:17PM +0200, Michael S. Tsirkin wrote:
> > > > My initial idea was to use a dedicated workqueue. Michael S. Tsirkin
> > > > @@ -563,7 +534,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
> > > > struct virtio_balloon *vb = vdev->priv;
> > > >
> > > > unregister_oom_notifier(&vb->nb);
> > > > - kthread_stop(vb->thread);
> > > > + cancel_work_sync(&vb->wq_work);
> > >
> > > OK but since job requeues itself, cancelling like this might not be enough.
> >
> > As long as there's no further external queueing, cancel_work_sync() is
> > guaranteed to kill a self-requeueing work item.
> >
> > Thanks.
>
> I didn't realise this. Thanks!
>
> Unfortunately in this case, there can be further requeueing
> if a stats request arrives.
Please, is there any point where the stat requests are disabled for
sure? I am not 100% sure but it might be after the reset() call:
vb->vdev->config->reset(vb->vdev);
Then we could split the kthread into two works: resizing and stats.
The resizing work still must be canceled before leaking the balloon.
But the stats work might be canceled after the reset() call.
In fact, the solution with the two works looks even cleaner.
Thanks for feedback,
Petr
^ permalink raw reply
* Re: [PATCH v2 17/32] arm: define __smp_xxx
From: Michael S. Tsirkin @ 2016-01-05 14:38 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Arnd Bergmann, x86, Tony Lindgren, xen-devel, Ingo Molnar,
linux-xtensa, user-mode-linux-devel, Stefano Stabellini,
Andrey Konovalov, adi-buildroot-devel, Thomas Gleixner,
linux-metag, linux-arm-kernel, Andrew Cooper, linux-kernel,
linuxppc-dev
In-Reply-To: <20160104135934.GE19062@n2100.arm.linux.org.uk>
On Mon, Jan 04, 2016 at 01:59:34PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 04, 2016 at 02:54:20PM +0100, Peter Zijlstra wrote:
> > On Mon, Jan 04, 2016 at 02:36:58PM +0100, Peter Zijlstra wrote:
> > > On Sun, Jan 03, 2016 at 11:12:44AM +0200, Michael S. Tsirkin wrote:
> > > > On Sat, Jan 02, 2016 at 11:24:38AM +0000, Russell King - ARM Linux wrote:
> > >
> > > > > My only concern is that it gives people an additional handle onto a
> > > > > "new" set of barriers - just because they're prefixed with __*
> > > > > unfortunately doesn't stop anyone from using it (been there with
> > > > > other arch stuff before.)
> > > > >
> > > > > I wonder whether we should consider making the smp memory barriers
> > > > > inline functions, so these __smp_xxx() variants can be undef'd
> > > > > afterwards, thereby preventing drivers getting their hands on these
> > > > > new macros?
> > > >
> > > > That'd be tricky to do cleanly since asm-generic depends on
> > > > ifndef to add generic variants where needed.
> > > >
> > > > But it would be possible to add a checkpatch test for this.
> > >
> > > Wasn't the whole purpose of these things for 'drivers' (namely
> > > virtio/xen hypervisor interaction) to use these?
> >
> > Ah, I see, you add virt_*mb() stuff later on for that use case.
> >
> > So, assuming everybody does include asm-generic/barrier.h, you could
> > simply #undef the __smp version at the end of that, once we've generated
> > all the regular primitives from it, no?
>
> Not so simple - that's why I mentioned using inline functions.
>
> The new smp_* _macros_ are:
>
> +#define smp_mb() __smp_mb()
>
> which means if we simply #undef __smp_mb(), smp_mb() then points at
> something which is no longer available, and we'll end up with errors
> saying that __smp_mb() doesn't exist.
>
> My suggestion was to change:
>
> #ifndef smp_mb
> #define smp_mb() __smp_mb()
> #endif
>
> to:
>
> #ifndef smp_mb
> static inline void smp_mb(void)
> {
> __smp_mb();
> }
> #endif
>
> which then means __smp_mb() and friends can be #undef'd afterwards.
Absolutely, I got it.
The issue is that e.g. tile has:
#define __smp_mb__after_atomic() do { } while (0)
and this is cheaper than barrier().
For this reason I left
#define smp_mb__after_atomic() __smp_mb__after_atomic()
in place there.
Now, of course I can do (in asm-generic):
#ifndef smp_mb__after_atomic
static inline void smp_mb__after_atomic(void)
{
...
}
#endif
but this seems ugly: architectures do defines, generic
version does inline.
And that is not all: APIs like smp_store_mb can take
a variety of types as arguments so they pretty much
must be implemented as macros.
Teaching checkpatch.pl to complain about it seems like the cleanest
approach.
> --
> 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.
^ permalink raw reply
* Re: [PATCH RFC] virtio_balloon: refill by config handler
From: Petr Mladek @ 2016-01-05 14:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Tejun Heo, Jiri Kosina, Jeff Epler, linux-kernel, virtualization
In-Reply-To: <1451643122-21561-1-git-send-email-mst@redhat.com>
On Fri 2016-01-01 12:13:48, Michael S. Tsirkin wrote:
> Peter Mladek reported that balloon might never refill completely after
> restore. This is because fill_balloon is only called once there.
> Calling fill_balloon repeatedly seems too aggressive,
> especially in light of the fact that it sleeps on failure: let's
> wake the config change handler and fill it asynchronously.
>
> Reported-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> I was unable to test this - for some reason my test VM
> doesn't resume (with or without the patch).
> Petr, does this work for you?
Ah, I have just realized that the problem happens only after the
conversion to the workqueue. It works without this patch with the
current code. The balloon kthread is waken when released from
__refrigerator().
I am sorry for the noise. I wanted to split the conversion to
workqueues into two patches to make it better readable. We need to
queue the balloon resizing work here. I wrongly assumed that the
original kthread must have the same problem.
Best Regards,
Petr
> drivers/virtio/virtio_balloon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 7efc329..ee29473 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -589,7 +589,7 @@ static int virtballoon_restore(struct virtio_device *vdev)
>
> virtio_device_ready(vdev);
>
> - fill_balloon(vb, towards_target(vb));
> + wake_up(&vb->config_change);
> update_balloon_size(vb);
> return 0;
> }
> --
> MST
^ permalink raw reply
* Re: [PATCH v2 22/32] s390: define __smp_xxx
From: Martin Schwidefsky @ 2016-01-05 14:21 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Heiko Carstens,
virtualization, H. Peter Anvin, sparclinux, Carsten Otte,
Ingo Molnar, linux-arch, linux-s390, Davidlohr Bueso,
Arnd Bergmann, x86, Christian Borntraeger, xen-devel, Ingo Molnar,
linux-xtensa, user-mode-linux-devel, Stefano Stabellini,
Andrey Konovalov, adi-buildroot-devel, Thomas Gleixner,
linux-metag, linux-arm-kern
In-Reply-To: <20160105144842-mutt-send-email-mst@redhat.com>
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.
^ permalink raw reply
* Re: [PATCH v2 22/32] s390: define __smp_xxx
From: Michael S. Tsirkin @ 2016-01-05 13:04 UTC (permalink / raw)
To: Martin Schwidefsky
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Heiko Carstens,
virtualization, H. Peter Anvin, sparclinux, Carsten Otte,
Ingo Molnar, linux-arch, linux-s390, Davidlohr Bueso,
Arnd Bergmann, x86, Christian Borntraeger, xen-devel, Ingo Molnar,
linux-xtensa, Christian Ehrhardt, user-mode-linux-devel,
Stefano Stabellini, Andrey Konovalov, adi-buildroot-devel,
Thomas Gleixner
In-Reply-To: <20160105130852.11148a7f@mschwide>
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.
^ permalink raw reply
* Re: [PATCH v2 22/32] s390: define __smp_xxx
From: Martin Schwidefsky @ 2016-01-05 12:08 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Heiko Carstens,
virtualization, H. Peter Anvin, sparclinux, Carsten Otte,
Ingo Molnar, linux-arch, linux-s390, Davidlohr Bueso,
Arnd Bergmann, x86, Christian Borntraeger, xen-devel, Ingo Molnar,
linux-xtensa, Christian Ehrhardt, user-mode-linux-devel,
Stefano Stabellini, Andrey Konovalov, adi-buildroot-devel,
Thomas Gleixner
In-Reply-To: <20160105105335-mutt-send-email-mst@redhat.com>
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.
^ permalink raw reply
* Re: [PATCH v2 15/32] powerpc: define __smp_xxx
From: Boqun Feng @ 2016-01-05 9:53 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra,
Benjamin Herrenschmidt, virtualization, Paul Mackerras,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Davidlohr Bueso, Arnd Bergmann, Michael Ellerman, x86, xen-devel,
Ingo Molnar, Paul E. McKenney, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, Andrey Konovalov,
adi-buildroot-devel, Thomas Gleixner
In-Reply-To: <20160105085117.GA11858@redhat.com>
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/
^ permalink raw reply
* Re: [PATCH v2 22/32] s390: define __smp_xxx
From: Michael S. Tsirkin @ 2016-01-05 9:30 UTC (permalink / raw)
To: Martin Schwidefsky
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Heiko Carstens,
virtualization, H. Peter Anvin, sparclinux, Carsten Otte,
Ingo Molnar, linux-arch, linux-s390, Davidlohr Bueso,
Arnd Bergmann, x86, Christian Borntraeger, xen-devel, Ingo Molnar,
linux-xtensa, Christian Ehrhardt, user-mode-linux-devel,
Stefano Stabellini, Andrey Konovalov, adi-buildroot-devel,
Thomas Gleixner
In-Reply-To: <20160105091319.59ddefc7@mschwide>
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.
^ permalink raw reply
* Re: [PATCH v2 15/32] powerpc: define __smp_xxx
From: Michael S. Tsirkin @ 2016-01-05 8:51 UTC (permalink / raw)
To: Boqun Feng
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra,
Benjamin Herrenschmidt, virtualization, Paul Mackerras,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Davidlohr Bueso, Arnd Bergmann, Michael Ellerman, x86, xen-devel,
Ingo Molnar, Paul E. McKenney, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, Andrey Konovalov,
adi-buildroot-devel, Thomas Gleixner
In-Reply-To: <20160105013648.GA1256@fixme-laptop.cn.ibm.com>
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/
^ permalink raw reply
* Re: [PATCH v2 22/32] s390: define __smp_xxx
From: Martin Schwidefsky @ 2016-01-05 8:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Heiko Carstens,
virtualization, H. Peter Anvin, sparclinux, Ingo Molnar,
linux-arch, linux-s390, Davidlohr Bueso, Arnd Bergmann, x86,
Christian Borntraeger, xen-devel, Ingo Molnar, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, Andrey Konovalov,
adi-buildroot-devel, Thomas Gleixner, linux-metag,
linux-arm-kernel, Andrew
In-Reply-To: <20160104221323-mutt-send-email-mst@redhat.com>
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.
^ permalink raw reply
* Re: [PATCH v2 06/32] s390: reuse asm-generic/barrier.h
From: Martin Schwidefsky @ 2016-01-05 8:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Heiko Carstens,
virtualization, H. Peter Anvin, sparclinux, Ingo Molnar,
linux-arch, linux-s390, Davidlohr Bueso, Arnd Bergmann, x86,
Christian Borntraeger, xen-devel, Ingo Molnar, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, Andrey Konovalov,
adi-buildroot-devel, Thomas Gleixner, linux-metag,
linux-arm-kernel, Andrew
In-Reply-To: <20160104224135-mutt-send-email-mst@redhat.com>
On Mon, 4 Jan 2016 22:42:44 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Mon, Jan 04, 2016 at 04:03:39PM +0100, Martin Schwidefsky wrote:
> > On Mon, 4 Jan 2016 14:20:42 +0100
> > Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > > On Thu, Dec 31, 2015 at 09:06:30PM +0200, Michael S. Tsirkin wrote:
> > > > On s390 read_barrier_depends, smp_read_barrier_depends
> > > > smp_store_mb(), 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/s390/include/asm/barrier.h | 10 ++--------
> > > > 1 file changed, 2 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h
> > > > index 7ffd0b1..c358c31 100644
> > > > --- a/arch/s390/include/asm/barrier.h
> > > > +++ b/arch/s390/include/asm/barrier.h
> > > > @@ -30,14 +30,6 @@
> > > > #define smp_rmb() rmb()
> > > > #define smp_wmb() wmb()
> > > >
> > > > -#define read_barrier_depends() do { } while (0)
> > > > -#define smp_read_barrier_depends() do { } while (0)
> > > > -
> > > > -#define smp_mb__before_atomic() smp_mb()
> > > > -#define smp_mb__after_atomic() smp_mb()
> > >
> > > As per:
> > >
> > > lkml.kernel.org/r/20150921112252.3c2937e1@mschwide
> > >
> > > s390 should change this to barrier() instead of smp_mb() and hence
> > > should not use the generic versions.
> >
> > Yes, we wanted to simplify this. Thanks for the reminder, I'll queue
> > a patch.
>
> Could you base on my patchset maybe, to avoid conflicts,
> and I'll merge it?
> Or if it's just replacing these 2 with barrier() I can do this
> myself easily.
Probably the easiest solution if you do the patch yourself and
include it in your patch set.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
^ permalink raw reply
* Re: [PATCH RFC] vhost: basic device IOTLB support
From: Yang Zhang @ 2016-01-05 3:18 UTC (permalink / raw)
To: Jason Wang, mst, kvm, virtualization, netdev, linux-kernel,
linux-api
In-Reply-To: <568A0FA6.8080804@redhat.com>
On 2016/1/4 14:22, Jason Wang wrote:
>
>
> On 01/04/2016 09:39 AM, Yang Zhang wrote:
>> On 2015/12/31 15:13, Jason Wang wrote:
>>> This patch tries to implement an device IOTLB for vhost. This could be
>>> used with for co-operation with userspace(qemu) implementation of
>>> iommu for a secure DMA environment in guest.
>>>
>>> The idea is simple. When vhost meets an IOTLB miss, it will request
>>> the assistance of userspace to do the translation, this is done
>>> through:
>>>
>>> - Fill the translation request in a preset userspace address (This
>>> address is set through ioctl VHOST_SET_IOTLB_REQUEST_ENTRY).
>>> - Notify userspace through eventfd (This eventfd was set through ioctl
>>> VHOST_SET_IOTLB_FD).
>>>
>>> When userspace finishes the translation, it will update the vhost
>>> IOTLB through VHOST_UPDATE_IOTLB ioctl. Userspace is also in charge of
>>> snooping the IOTLB invalidation of IOMMU IOTLB and use
>>> VHOST_UPDATE_IOTLB to invalidate the possible entry in vhost.
>>
>> Is there any performance data shows the difference with IOTLB supporting?
>
> Basic testing show it was slower than without IOTLB.
>
>> I doubt we may see performance decrease since the flush code path is
>> longer than before.
>>
>
> Yes, it also depend on the TLB hit rate.
>
> If lots of dynamic mappings and unmappings are used in guest (e.g normal
> Linux driver). This method should be much more slower since:
>
> - lots of invalidation and its path is slow.
> - the hit rate is low and the high price of userspace assisted address
> translation.
> - limitation of userspace IOMMU/IOTLB implementation (qemu's vtd
> emulation simply empty all entries when it's full).
>
> Another method is to implement kernel IOMMU (e.g vtd). But I'm not sure
> vhost is the best place to do this, since vhost should be architecture
> independent. Maybe we'd better to do it in kvm or have a pv IOMMU
> implementation in vhost.
Actually, i have the kernel IOMMU(virtual vtd) patch which can pass
though the physical device to L2 guest on hand. But it is just a draft
patch which was written several years ago. If there is real requirement
for it, I can rebase it and send out it for review.
>
> Another side, if fixed mappings were used in guest, (e.g dpdk in guest).
> We have the possibility to have 100% hit rate with almost no
> invalidation, the performance penalty should be ignorable, this should
> be the main use case for this patch.
>
> The patch is just a prototype for discussion. Any other ideas are welcomed.
>
> Thanks
>
--
best regards
yang
^ permalink raw reply
* Re: [PATCH v2 15/32] powerpc: define __smp_xxx
From: Boqun Feng @ 2016-01-05 1:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra,
Benjamin Herrenschmidt, virtualization, Paul Mackerras,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Davidlohr Bueso, Arnd Bergmann, Michael Ellerman, x86, xen-devel,
Ingo Molnar, Paul E. McKenney, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, Andrey Konovalov,
adi-buildroot-devel, Thomas Gleixner
In-Reply-To: <1451572003-2440-16-git-send-email-mst@redhat.com>
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/
^ permalink raw reply
* Re: [PATCH v2 20/32] metag: define __smp_xxx
From: James Hogan @ 2016-01-05 0:09 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Arnd Bergmann, Davidlohr Bueso, x86, xen-devel, Ingo Molnar,
linux-xtensa, user-mode-linux-devel, Stefano Stabellini,
Andrey Konovalov, adi-buildroot-devel, Thomas Gleixner,
linux-metag, linux-arm-kernel, Andrew Cooper, linux-kernel,
linuxppc-dev
In-Reply-To: <1451572003-2440-21-git-send-email-mst@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 3506 bytes --]
Hi Michael,
On Thu, Dec 31, 2015 at 09:08:22PM +0200, Michael S. Tsirkin wrote:
> This defines __smp_xxx barriers for metag,
> for use by virtualization.
>
> smp_xxx barriers are removed as they are
> defined correctly by asm-generic/barriers.h
>
> Note: as __smp_XX macros should not depend on CONFIG_SMP, they can not
> use the existing fence() macro since that is defined differently between
> SMP and !SMP. For this reason, this patch introduces a wrapper
> metag_fence() that doesn't depend on CONFIG_SMP.
> fence() is then defined using that, depending on CONFIG_SMP.
I'm not a fan of the inconsistent commit message wrapping. I wrap to 72
columns (although I now notice SubmittingPatches says to use 75...).
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/metag/include/asm/barrier.h | 32 +++++++++++++++-----------------
> 1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/arch/metag/include/asm/barrier.h b/arch/metag/include/asm/barrier.h
> index b5b778b..84880c9 100644
> --- a/arch/metag/include/asm/barrier.h
> +++ b/arch/metag/include/asm/barrier.h
> @@ -44,13 +44,6 @@ static inline void wr_fence(void)
> #define rmb() barrier()
> #define wmb() mb()
>
> -#ifndef CONFIG_SMP
> -#define fence() do { } while (0)
> -#define smp_mb() barrier()
> -#define smp_rmb() barrier()
> -#define smp_wmb() barrier()
> -#else
!SMP kernel text differs, but only because of new presence of unused
metag_fence() inline function. If I #if 0 that out, then it matches, so
thats fine.
> -
> #ifdef CONFIG_METAG_SMP_WRITE_REORDERING
> /*
> * Write to the atomic memory unlock system event register (command 0). This is
> @@ -60,26 +53,31 @@ static inline void wr_fence(void)
> * incoherence). It is therefore ineffective if used after and on the same
> * thread as a write.
> */
> -static inline void fence(void)
> +static inline void metag_fence(void)
> {
> volatile int *flushptr = (volatile int *) LINSYSEVENT_WR_ATOMIC_UNLOCK;
> barrier();
> *flushptr = 0;
> barrier();
> }
> -#define smp_mb() fence()
> -#define smp_rmb() fence()
> -#define smp_wmb() barrier()
> +#define __smp_mb() metag_fence()
> +#define __smp_rmb() metag_fence()
> +#define __smp_wmb() barrier()
> #else
> -#define fence() do { } while (0)
> -#define smp_mb() barrier()
> -#define smp_rmb() barrier()
> -#define smp_wmb() barrier()
> +#define metag_fence() do { } while (0)
> +#define __smp_mb() barrier()
> +#define __smp_rmb() barrier()
> +#define __smp_wmb() barrier()
Whitespace is now messed up. Admitedly its already inconsistent
tabs/spaces, but it'd be nice if the definitions at least still all
lined up. You're touching all the definitions which use spaces anyway,
so feel free to convert them to tabs while you're at it.
Other than those niggles, it looks sensible to me:
Acked-by: James Hogan <james.hogan@imgtec.com>
Cheers
James
> #endif
> +
> +#ifdef CONFIG_SMP
> +#define fence() metag_fence()
> +#else
> +#define fence() do { } while (0)
> #endif
>
> -#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
>
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v2 10/32] metag: reuse asm-generic/barrier.h
From: James Hogan @ 2016-01-04 23:24 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Arnd Bergmann, Michael Ellerman, x86, xen-devel, Ingo Molnar,
linux-xtensa, user-mode-linux-devel, Stefano Stabellini,
Andrey Konovalov, adi-buildroot-devel, Thomas Gleixner,
linux-metag, linux-arm-kernel, Andrew Cooper, linux-kernel,
linuxppc-dev
In-Reply-To: <1451572003-2440-11-git-send-email-mst@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 2103 bytes --]
On Thu, Dec 31, 2015 at 09:07:02PM +0200, Michael S. Tsirkin wrote:
> On metag dma_rmb, dma_wmb, smp_store_mb, read_barrier_depends,
> smp_read_barrier_depends, smp_store_release and smp_load_acquire 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>
Looks good, and confirmed no text change (once patch 1 is applied that
is).
Acked-by: James Hogan <james.hogan@imgtec.com>
Thanks
James
> ---
> arch/metag/include/asm/barrier.h | 25 ++-----------------------
> 1 file changed, 2 insertions(+), 23 deletions(-)
>
> diff --git a/arch/metag/include/asm/barrier.h b/arch/metag/include/asm/barrier.h
> index 172b7e5..b5b778b 100644
> --- a/arch/metag/include/asm/barrier.h
> +++ b/arch/metag/include/asm/barrier.h
> @@ -44,9 +44,6 @@ static inline void wr_fence(void)
> #define rmb() barrier()
> #define wmb() mb()
>
> -#define dma_rmb() rmb()
> -#define dma_wmb() wmb()
> -
> #ifndef CONFIG_SMP
> #define fence() do { } while (0)
> #define smp_mb() barrier()
> @@ -81,27 +78,9 @@ static inline void fence(void)
> #endif
> #endif
>
> -#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_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 smp_mb__before_atomic() barrier()
> #define smp_mb__after_atomic() barrier()
>
> +#include <asm-generic/barrier.h>
> +
> #endif /* _ASM_METAG_BARRIER_H */
> --
> MST
>
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 1/3] checkpatch.pl: add missing memory barriers
From: Joe Perches @ 2016-01-04 22:15 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Russell King - ARM Linux, Arnd Bergmann, x86, Tony Lindgren,
xen-devel, Ingo Molnar, linux-xtensa, user-mode-linux-devel,
Stefano Stabellini, Andrey Konovalov, adi-buildroot-devel,
Cc : Andy Whitcroft, Thomas Gleixner, linux-metag,
linux-arm-kernel, Andrew
In-Reply-To: <20160104224415-mutt-send-email-mst@redhat.com>
On Mon, 2016-01-04 at 22:45 +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 04, 2016 at 08:07:40AM -0800, Joe Perches wrote:
> > On Mon, 2016-01-04 at 13:36 +0200, Michael S. Tsirkin wrote:
> > > SMP-only barriers were missing in checkpatch.pl
> > >
> > > Refactor code slightly to make adding more variants easier.
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > > scripts/checkpatch.pl | 9 ++++++++-
> > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > index 2b3c228..0245bbe 100755
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -5116,7 +5116,14 @@ sub process {
> > > }
> > > }
> > > # check for memory barriers without a comment.
> > > - if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) {
> > > +
> > > + my @barriers = ('mb', 'rmb', 'wmb', 'read_barrier_depends');
> > > + my @smp_barriers = ('smp_store_release', 'smp_load_acquire', 'smp_store_mb');
> > > +
> > > + @smp_barriers = (@smp_barriers, map {"smp_" . $_} @barriers);
> >
> > I think using map, which so far checkpatch doesn't use,
> > makes smp_barriers harder to understand and it'd be
> > better to enumerate them.
>
> Okay - I'll rewrite using foreach.
>
I think using the qr form (like 'our $Attribute' and
a bunch of others) is the general style that should
be used for checkpatch.
^ permalink raw reply
* Re: [PATCH 3/3] checkpatch: add virt barriers
From: Joe Perches @ 2016-01-04 22:11 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Russell King - ARM Linux, Arnd Bergmann, x86, Tony Lindgren,
xen-devel, Ingo Molnar, linux-xtensa, user-mode-linux-devel,
Stefano Stabellini, Andrey Konovalov, adi-buildroot-devel,
Cc : Andy Whitcroft, Thomas Gleixner, linux-metag,
linux-arm-kernel, Andrew
In-Reply-To: <20160104230528-mutt-send-email-mst@redhat.com>
On Mon, 2016-01-04 at 23:07 +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 04, 2016 at 08:47:53AM -0800, Joe Perches wrote:
> > On Mon, 2016-01-04 at 13:37 +0200, Michael S. Tsirkin wrote:
> > > Add virt_ barriers to list of barriers to check for
> > > presence of a comment.
> >
> > Are these virt_ barriers used anywhere?
> >
> > I see some virtio_ barrier like uses.
>
> They will be :) They are added and used by patchset
> arch: barrier cleanup + barriers for virt
>
> See
> http://article.gmane.org/gmane.linux.kernel.virtualization/26555
Ah, OK, thanks.
Are the virtio_ barriers going away?
If not, maybe those should be added too.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox