* Re: [PATCH v2 08/32] arm: reuse asm-generic/barrier.h
From: Russell King - ARM Linux @ 2016-01-02 11:20 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, 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: <1451572003-2440-9-git-send-email-mst@redhat.com>
On Thu, Dec 31, 2015 at 09:06:46PM +0200, Michael S. Tsirkin wrote:
> On arm smp_store_mb, read_barrier_depends, smp_read_barrier_depends,
> smp_store_release, smp_load_acquire, smp_mb__before_atomic and
> smp_mb__after_atomic match the asm-generic variants exactly. Drop the
> local definitions and pull in asm-generic/barrier.h instead.
>
> This is in preparation to refactoring this code area.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
Thanks, the asm-generic versions looks identical to me, so this should
result in no code generation difference.
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH v2 32/32] virtio_ring: use virt_store_mb
From: Sergei Shtylyov @ 2016-01-01 17:23 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, linux-arch, linux-s390, Arnd Bergmann,
x86, xen-devel, Ingo Molnar, linux-xtensa, user-mode-linux-devel,
Stefano Stabellini, adi-buildroot-devel, Thomas Gleixner,
linux-metag, linux-arm-kernel, Andrew Cooper, linuxppc-dev,
David Miller
In-Reply-To: <1451572003-2440-33-git-send-email-mst@redhat.com>
Hello.
On 12/31/2015 10:09 PM, Michael S. Tsirkin wrote:
> We need a full barrier after writing out event index, using
> virt_store_mb there seems better than open-coding. As usual, we need a
> wrapper to account for strong barriers.
>
> It's tempting to use this in vhost as well, for that, we'll
> need a variant of smp_store_mb that works on __user pointers.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> include/linux/virtio_ring.h | 12 ++++++++++++
> drivers/virtio/virtio_ring.c | 15 +++++++++------
> 2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
> index f3fa55b..3a74d91 100644
> --- a/include/linux/virtio_ring.h
> +++ b/include/linux/virtio_ring.h
> @@ -45,6 +45,18 @@ static inline void virtio_wmb(bool weak_barriers)
> wmb();
> }
>
> +static inline void virtio_store_mb(bool weak_barriers,
> + __virtio16 *p, __virtio16 v)
> +{
> + if (weak_barriers)
> + virt_store_mb(*p, v);
> + else
> + {
The kernel coding style dictates:
if (weak_barriers) {
virt_store_mb(*p, v);
} else {
> + WRITE_ONCE(*p, v);
> + mb();
> + }
> +}
> +
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v2 30/32] virtio_ring: update weak barriers to use __smp_xxx
From: Michael S. Tsirkin @ 2016-01-01 10:21 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Alexander Duyck,
virtualization, H. Peter Anvin, sparclinux, linux-arch,
linux-s390, Arnd Bergmann, x86, xen-devel, Ingo Molnar,
linux-xtensa, user-mode-linux-devel, Stefano Stabellini,
adi-buildroot-devel, Thomas Gleixner, linux-metag,
linux-arm-kernel, Andrew Cooper, linuxppc-dev, David Miller
In-Reply-To: <1451572003-2440-31-git-send-email-mst@redhat.com>
On Fri, Jan 01, 2016 at 11:39:40AM +0200, Michael S. Tsirkin wrote:
> virtio ring uses smp_wmb on SMP and wmb on !SMP,
> the reason for the later being that it might be
> talking to another kernel on the same SMP machine.
>
> This is exactly what __smp_XXX barriers do,
> so switch to these instead of homegrown ifdef hacks.
>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Alexander Duyck <alexander.duyck@gmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
The subject and commit log should say
virt_xxx and not __smp_xxx - I fixed this up in
my tree.
> ---
> include/linux/virtio_ring.h | 25 ++++---------------------
> 1 file changed, 4 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
> index 67e06fe..f3fa55b 100644
> --- a/include/linux/virtio_ring.h
> +++ b/include/linux/virtio_ring.h
> @@ -12,7 +12,7 @@
> * anyone care?
> *
> * For virtio_pci on SMP, we don't need to order with respect to MMIO
> - * accesses through relaxed memory I/O windows, so smp_mb() et al are
> + * accesses through relaxed memory I/O windows, so virt_mb() et al are
> * sufficient.
> *
> * For using virtio to talk to real devices (eg. other heterogeneous
> @@ -21,11 +21,10 @@
> * actually quite cheap.
> */
>
> -#ifdef CONFIG_SMP
> static inline void virtio_mb(bool weak_barriers)
> {
> if (weak_barriers)
> - smp_mb();
> + virt_mb();
> else
> mb();
> }
> @@ -33,7 +32,7 @@ static inline void virtio_mb(bool weak_barriers)
> static inline void virtio_rmb(bool weak_barriers)
> {
> if (weak_barriers)
> - smp_rmb();
> + virt_rmb();
> else
> rmb();
> }
> @@ -41,26 +40,10 @@ static inline void virtio_rmb(bool weak_barriers)
> static inline void virtio_wmb(bool weak_barriers)
> {
> if (weak_barriers)
> - smp_wmb();
> + virt_wmb();
> else
> wmb();
> }
> -#else
> -static inline void virtio_mb(bool weak_barriers)
> -{
> - mb();
> -}
> -
> -static inline void virtio_rmb(bool weak_barriers)
> -{
> - rmb();
> -}
> -
> -static inline void virtio_wmb(bool weak_barriers)
> -{
> - wmb();
> -}
> -#endif
>
> struct virtio_device;
> struct virtqueue;
> --
> MST
^ permalink raw reply
* Re: [PATCH v4 2/2] virtio_balloon: Use a workqueue instead of "vballoon" kthread
From: Michael S. Tsirkin @ 2016-01-01 10:18 UTC (permalink / raw)
To: Petr Mladek
Cc: Jiri Kosina, linux-kernel, Petr Mladek, virtualization, Tejun Heo,
Jeff Epler
In-Reply-To: <1449236271-10133-3-git-send-email-pmladek@suse.com>
On Fri, Dec 04, 2015 at 02:37:51PM +0100, Petr Mladek wrote:
> From: Petr Mladek <pmladek@suse.cz>
>
> This patch moves the deferred work from the "vballoon" kthread into a
> system freezable workqueue.
>
> We do not need to maintain and run a dedicated kthread. Also the event
> driven workqueues API makes the logic much easier. Especially, we do
> not longer need an own wait queue, wait function, and freeze point.
>
> The conversion is pretty straightforward. One cycle of the main loop
> is put into a work. The work is queued instead of waking the kthread.
>
> fill_balloon() and leak_balloon() have a limit for the amount of modified
> pages. The work re-queues itself when necessary.
>
> My initial idea was to use a dedicated workqueue. Michael S. Tsirkin
> suggested using a system one. Tejun Heo confirmed that the system
> workqueue has a pretty high concurrency level (256) by default.
> Therefore we need not be afraid of too long blocking.
Right but fill has a 1/5 second sleep on failure - *that*
is problematic for a system queue.
There's also a race introduced on remove, see below.
I'm inclined to tread carefully with this conversion.
>
> Signed-off-by: Petr Mladek <pmladek@suse.cz>
> ---
> drivers/virtio/virtio_balloon.c | 82 +++++++++++++----------------------------
> 1 file changed, 26 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index d73a86db2490..960e54b1d0c1 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -22,8 +22,7 @@
> #include <linux/virtio.h>
> #include <linux/virtio_balloon.h>
> #include <linux/swap.h>
> -#include <linux/kthread.h>
> -#include <linux/freezer.h>
> +#include <linux/workqueue.h>
> #include <linux/delay.h>
> #include <linux/slab.h>
> #include <linux/module.h>
> @@ -49,11 +48,8 @@ struct virtio_balloon {
> struct virtio_device *vdev;
> struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
>
> - /* Where the ballooning thread waits for config to change. */
> - wait_queue_head_t config_change;
> -
> - /* The thread servicing the balloon. */
> - struct task_struct *thread;
> + /* The balloon servicing is delegated to a freezable workqueue. */
> + struct work_struct wq_work;
>
> /* Waiting for host to ack the pages we released. */
> wait_queue_head_t acked;
> @@ -255,14 +251,15 @@ static void update_balloon_stats(struct virtio_balloon *vb)
> * with a single buffer. From that point forward, all conversations consist of
> * a hypervisor request (a call to this function) which directs us to refill
> * the virtqueue with a fresh stats buffer. Since stats collection can sleep,
> - * we notify our kthread which does the actual work via stats_handle_request().
> + * we delegate the job to a freezable workqueue that will do the actual work via
> + * stats_handle_request().
> */
> static void stats_request(struct virtqueue *vq)
> {
> struct virtio_balloon *vb = vq->vdev->priv;
>
> vb->need_stats_update = 1;
> - wake_up(&vb->config_change);
> + queue_work(system_freezable_wq, &vb->wq_work);
> }
>
> static void stats_handle_request(struct virtio_balloon *vb)
> @@ -286,7 +283,7 @@ static void virtballoon_changed(struct virtio_device *vdev)
> {
> struct virtio_balloon *vb = vdev->priv;
>
> - wake_up(&vb->config_change);
> + queue_work(system_freezable_wq, &vb->wq_work);
> }
>
> static inline s64 towards_target(struct virtio_balloon *vb)
> @@ -349,43 +346,25 @@ static int virtballoon_oom_notify(struct notifier_block *self,
> return NOTIFY_OK;
> }
>
> -static int balloon(void *_vballoon)
> +static void balloon(struct work_struct *work)
> {
> - struct virtio_balloon *vb = _vballoon;
> - DEFINE_WAIT_FUNC(wait, woken_wake_function);
> -
> - set_freezable();
> - while (!kthread_should_stop()) {
> - s64 diff;
> -
> - try_to_freeze();
> -
> - add_wait_queue(&vb->config_change, &wait);
> - for (;;) {
> - if ((diff = towards_target(vb)) != 0 ||
> - vb->need_stats_update ||
> - kthread_should_stop() ||
> - freezing(current))
> - break;
> - wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
> - }
> - remove_wait_queue(&vb->config_change, &wait);
> + struct virtio_balloon *vb;
> + s64 diff;
>
> - if (vb->need_stats_update)
> - stats_handle_request(vb);
> - if (diff > 0)
> - fill_balloon(vb, diff);
> - else if (diff < 0)
> - leak_balloon(vb, -diff);
> - update_balloon_size(vb);
> + vb = container_of(work, struct virtio_balloon, wq_work);
> + diff = towards_target(vb);
>
> - /*
> - * For large balloon changes, we could spend a lot of time
> - * and always have work to do. Be nice if preempt disabled.
> - */
> - cond_resched();
> - }
> - return 0;
> + if (vb->need_stats_update)
> + stats_handle_request(vb);
> +
> + if (diff > 0)
> + diff -= fill_balloon(vb, diff);
> + else if (diff < 0)
> + diff += leak_balloon(vb, -diff);
> + update_balloon_size(vb);
> +
> + if (diff)
> + queue_work(system_freezable_wq, work);
> }
>
> static int init_vqs(struct virtio_balloon *vb)
> @@ -503,9 +482,9 @@ static int virtballoon_probe(struct virtio_device *vdev)
> goto out;
> }
>
> + INIT_WORK(&vb->wq_work, balloon);
> vb->num_pages = 0;
> mutex_init(&vb->balloon_lock);
> - init_waitqueue_head(&vb->config_change);
> init_waitqueue_head(&vb->acked);
> vb->vdev = vdev;
> vb->need_stats_update = 0;
> @@ -527,16 +506,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
>
> virtio_device_ready(vdev);
>
> - vb->thread = kthread_run(balloon, vb, "vballoon");
> - if (IS_ERR(vb->thread)) {
> - err = PTR_ERR(vb->thread);
> - goto out_del_vqs;
> - }
> -
> return 0;
>
> -out_del_vqs:
> - unregister_oom_notifier(&vb->nb);
> out_oom_notify:
> vdev->config->del_vqs(vdev);
> out_free_vb:
> @@ -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.
> remove_common(vb);
> kfree(vb);
> }
> @@ -574,10 +545,9 @@ static int virtballoon_freeze(struct virtio_device *vdev)
> struct virtio_balloon *vb = vdev->priv;
>
> /*
> - * The kthread is already frozen by the PM core before this
> + * The workqueue is already frozen by the PM core before this
> * function is called.
> */
> -
> remove_common(vb);
> return 0;
> }
> --
> 1.8.5.6
^ permalink raw reply
* [PATCH RFC] virtio_balloon: refill by config handler
From: Michael S. Tsirkin @ 2016-01-01 10:13 UTC (permalink / raw)
To: linux-kernel
Cc: Tejun Heo, Petr Mladek, Jeff Epler, Jiri Kosina, virtualization
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?
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 related
* Re: [PATCH v4 1/2] virtio_balloon: Restore the entire balloon after the system freeze
From: Michael S. Tsirkin @ 2016-01-01 10:11 UTC (permalink / raw)
To: Petr Mladek
Cc: Jiri Kosina, linux-kernel, virtualization, Tejun Heo, Jeff Epler
In-Reply-To: <20160101115424-mutt-send-email-mst@redhat.com>
On Fri, Jan 01, 2016 at 12:11:02PM +0200, Michael S. Tsirkin wrote:
> On Fri, Dec 04, 2015 at 02:37:50PM +0100, Petr Mladek wrote:
> > fill_balloon() and leak_balloon() manipulate only a limited number
> > of pages in one call. This is the reason why remove_common() calls
> > leak_balloon() in a while cycle.
> >
> > remove_common() is called also when the system is being frozen.
> > But fill_balloon() is called only once when the system is being
> > restored. It means that most of the balloon stays leaked after
> > the system freeze and restore.
>
> Right, but refilling might take a long while.
> In fact, we sleep for 200msec on refill failure,
> stalling system resume - which is already a bug.
>
> > This patch adds the missing while cycle also into virtballoon_restore().
> > Also it makes fill_balloon() to return the number of really modified
> > pages. Note that leak_balloon() already did this.
> >
> > Signed-off-by: Petr Mladek <pmladek@suse.com>
>
> This is a replacement for:
> virtio_balloon: Restore the entire balloon after the system freeze
oops, typo
I meant to write I'll send a replacement patch shortly :)
> > ---
> > drivers/virtio/virtio_balloon.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > index 7efc32945810..d73a86db2490 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -135,9 +135,10 @@ static void set_page_pfns(u32 pfns[], struct page *page)
> > pfns[i] = page_to_balloon_pfn(page) + i;
> > }
> >
> > -static void fill_balloon(struct virtio_balloon *vb, size_t num)
> > +static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> > {
> > struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> > + unsigned num_allocated_pages;
> >
> > /* We can only do one array worth at a time. */
> > num = min(num, ARRAY_SIZE(vb->pfns));
> > @@ -162,10 +163,13 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
> > adjust_managed_page_count(page, -1);
> > }
> >
> > + num_allocated_pages = vb->num_pfns;
> > /* Did we get any? */
> > if (vb->num_pfns != 0)
> > tell_host(vb, vb->inflate_vq);
> > mutex_unlock(&vb->balloon_lock);
> > +
> > + return num_allocated_pages;
> > }
> >
> > static void release_pages_balloon(struct virtio_balloon *vb)
> > @@ -581,6 +585,7 @@ static int virtballoon_freeze(struct virtio_device *vdev)
> > static int virtballoon_restore(struct virtio_device *vdev)
> > {
> > struct virtio_balloon *vb = vdev->priv;
> > + s64 diff;
> > int ret;
> >
> > ret = init_vqs(vdev->priv);
> > @@ -589,7 +594,9 @@ static int virtballoon_restore(struct virtio_device *vdev)
> >
> > virtio_device_ready(vdev);
> >
> > - fill_balloon(vb, towards_target(vb));
> > + diff = towards_target(vb);
> > + while (diff > 0)
> > + diff -= fill_balloon(vb, diff);
> > update_balloon_size(vb);
> > return 0;
> > }
> > --
> > 1.8.5.6
^ permalink raw reply
* Re: [PATCH v4 1/2] virtio_balloon: Restore the entire balloon after the system freeze
From: Michael S. Tsirkin @ 2016-01-01 10:11 UTC (permalink / raw)
To: Petr Mladek
Cc: Jiri Kosina, linux-kernel, virtualization, Tejun Heo, Jeff Epler
In-Reply-To: <1449236271-10133-2-git-send-email-pmladek@suse.com>
On Fri, Dec 04, 2015 at 02:37:50PM +0100, Petr Mladek wrote:
> fill_balloon() and leak_balloon() manipulate only a limited number
> of pages in one call. This is the reason why remove_common() calls
> leak_balloon() in a while cycle.
>
> remove_common() is called also when the system is being frozen.
> But fill_balloon() is called only once when the system is being
> restored. It means that most of the balloon stays leaked after
> the system freeze and restore.
Right, but refilling might take a long while.
In fact, we sleep for 200msec on refill failure,
stalling system resume - which is already a bug.
> This patch adds the missing while cycle also into virtballoon_restore().
> Also it makes fill_balloon() to return the number of really modified
> pages. Note that leak_balloon() already did this.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
This is a replacement for:
virtio_balloon: Restore the entire balloon after the system freeze
> ---
> drivers/virtio/virtio_balloon.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 7efc32945810..d73a86db2490 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -135,9 +135,10 @@ static void set_page_pfns(u32 pfns[], struct page *page)
> pfns[i] = page_to_balloon_pfn(page) + i;
> }
>
> -static void fill_balloon(struct virtio_balloon *vb, size_t num)
> +static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> {
> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> + unsigned num_allocated_pages;
>
> /* We can only do one array worth at a time. */
> num = min(num, ARRAY_SIZE(vb->pfns));
> @@ -162,10 +163,13 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
> adjust_managed_page_count(page, -1);
> }
>
> + num_allocated_pages = vb->num_pfns;
> /* Did we get any? */
> if (vb->num_pfns != 0)
> tell_host(vb, vb->inflate_vq);
> mutex_unlock(&vb->balloon_lock);
> +
> + return num_allocated_pages;
> }
>
> static void release_pages_balloon(struct virtio_balloon *vb)
> @@ -581,6 +585,7 @@ static int virtballoon_freeze(struct virtio_device *vdev)
> static int virtballoon_restore(struct virtio_device *vdev)
> {
> struct virtio_balloon *vb = vdev->priv;
> + s64 diff;
> int ret;
>
> ret = init_vqs(vdev->priv);
> @@ -589,7 +594,9 @@ static int virtballoon_restore(struct virtio_device *vdev)
>
> virtio_device_ready(vdev);
>
> - fill_balloon(vb, towards_target(vb));
> + diff = towards_target(vb);
> + while (diff > 0)
> + diff -= fill_balloon(vb, diff);
> update_balloon_size(vb);
> return 0;
> }
> --
> 1.8.5.6
^ permalink raw reply
* [PATCH RFC] balloon: fix page list locking
From: Michael S. Tsirkin @ 2016-01-01 9:50 UTC (permalink / raw)
To: linux-kernel
Cc: Rafael Aquini, linux-mm, Hugh Dickins, stable, virtualization,
Minchan Kim, Andrew Morton, Konstantin Khlebnikov
Minchan Kim noticed that balloon_page_dequeue walks the pages list
without holding the pages_lock. This can race e.g. with isolation, which
has been reported to cause list corruption and crashes in leak_balloon.
Page can also in theory get freed before it's locked, corrupting memory.
To fix, make sure list accesses are done under lock, and
always take a page reference before trying to lock it.
Reported-by: Minchan Kim <minchan@kernel.org>
Cc: <stable@vger.kernel.org>
Cc: Rafael Aquini <aquini@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
This is an alternative to patch
virtio_balloon: fix race between migration and ballooning
by Minchan Kim in -mm.
Untested - Minchan, could you pls confirm this fixes the issue for you?
mm/balloon_compaction.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
index d3116be..66d69c5 100644
--- a/mm/balloon_compaction.c
+++ b/mm/balloon_compaction.c
@@ -56,12 +56,34 @@ EXPORT_SYMBOL_GPL(balloon_page_enqueue);
*/
struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info)
{
- struct page *page, *tmp;
+ struct page *page;
unsigned long flags;
bool dequeued_page;
+ LIST_HEAD(processed); /* protected by b_dev_info->pages_lock */
dequeued_page = false;
- list_for_each_entry_safe(page, tmp, &b_dev_info->pages, lru) {
+ /*
+ * We need to go over b_dev_info->pages and lock each page,
+ * but b_dev_info->pages_lock must nest within page lock.
+ *
+ * To make this safe, remove each page from b_dev_info->pages list
+ * under b_dev_info->pages_lock, then drop this lock. Once list is
+ * empty, re-add them also under b_dev_info->pages_lock.
+ */
+ spin_lock_irqsave(&b_dev_info->pages_lock, flags);
+ while (!list_empty(&b_dev_info->pages)) {
+ page = list_first_entry(&b_dev_info->pages, typeof(*page), lru);
+ /* move to processed list to avoid going over it another time */
+ list_move(&page->lru, &processed);
+
+ if (!get_page_unless_zero(page))
+ continue;
+ /*
+ * pages_lock nests within page lock,
+ * so drop it before trylock_page
+ */
+ spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
+
/*
* Block others from accessing the 'page' while we get around
* establishing additional references and preparing the 'page'
@@ -72,6 +94,7 @@ struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info)
if (!PagePrivate(page)) {
/* raced with isolation */
unlock_page(page);
+ put_page(page);
continue;
}
#endif
@@ -80,11 +103,18 @@ struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info)
__count_vm_event(BALLOON_DEFLATE);
spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
unlock_page(page);
+ put_page(page);
dequeued_page = true;
break;
}
+ put_page(page);
+ spin_lock_irqsave(&b_dev_info->pages_lock, flags);
}
+ /* re-add remaining entries */
+ list_splice(&processed, &b_dev_info->pages);
+ spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
+
if (!dequeued_page) {
/*
* If we are unable to dequeue a balloon page because the page
--
MST
^ permalink raw reply related
* [PATCH v2 30/32] virtio_ring: update weak barriers to use __smp_xxx
From: Michael S. Tsirkin @ 2016-01-01 9:39 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Alexander Duyck,
virtualization, H. Peter Anvin, sparclinux, linux-arch,
linux-s390, Arnd Bergmann, x86, xen-devel, Ingo Molnar,
linux-xtensa, user-mode-linux-devel, Stefano Stabellini,
adi-buildroot-devel, Thomas Gleixner, linux-metag,
linux-arm-kernel, Andrew Cooper, linuxppc-dev, David Miller
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
virtio ring uses smp_wmb on SMP and wmb on !SMP,
the reason for the later being that it might be
talking to another kernel on the same SMP machine.
This is exactly what __smp_XXX barriers do,
so switch to these instead of homegrown ifdef hacks.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/linux/virtio_ring.h | 25 ++++---------------------
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 67e06fe..f3fa55b 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -12,7 +12,7 @@
* anyone care?
*
* For virtio_pci on SMP, we don't need to order with respect to MMIO
- * accesses through relaxed memory I/O windows, so smp_mb() et al are
+ * accesses through relaxed memory I/O windows, so virt_mb() et al are
* sufficient.
*
* For using virtio to talk to real devices (eg. other heterogeneous
@@ -21,11 +21,10 @@
* actually quite cheap.
*/
-#ifdef CONFIG_SMP
static inline void virtio_mb(bool weak_barriers)
{
if (weak_barriers)
- smp_mb();
+ virt_mb();
else
mb();
}
@@ -33,7 +32,7 @@ static inline void virtio_mb(bool weak_barriers)
static inline void virtio_rmb(bool weak_barriers)
{
if (weak_barriers)
- smp_rmb();
+ virt_rmb();
else
rmb();
}
@@ -41,26 +40,10 @@ static inline void virtio_rmb(bool weak_barriers)
static inline void virtio_wmb(bool weak_barriers)
{
if (weak_barriers)
- smp_wmb();
+ virt_wmb();
else
wmb();
}
-#else
-static inline void virtio_mb(bool weak_barriers)
-{
- mb();
-}
-
-static inline void virtio_rmb(bool weak_barriers)
-{
- rmb();
-}
-
-static inline void virtio_wmb(bool weak_barriers)
-{
- wmb();
-}
-#endif
struct virtio_device;
struct virtqueue;
--
MST
^ permalink raw reply related
* Re: [PATCH 2/2] virtio_balloon: fix race between migration and ballooning
From: Michael S. Tsirkin @ 2016-01-01 9:36 UTC (permalink / raw)
To: Minchan Kim
Cc: Rafael Aquini, linux-kernel, stable, virtualization, linux-mm,
Andrew Morton, Konstantin Khlebnikov
In-Reply-To: <1451259313-26353-2-git-send-email-minchan@kernel.org>
On Mon, Dec 28, 2015 at 08:35:13AM +0900, Minchan Kim wrote:
> In balloon_page_dequeue, pages_lock should cover the loop
> (ie, list_for_each_entry_safe). Otherwise, the cursor page could
> be isolated by compaction and then list_del by isolation could
> poison the page->lru.{prev,next} so the loop finally could
> access wrong address like this. This patch fixes the bug.
>
> general protection fault: 0000 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 2 PID: 82 Comm: vballoon Not tainted 4.4.0-rc5-mm1-access_bit+ #1906
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> task: ffff8800a7ff0000 ti: ffff8800a7fec000 task.ti: ffff8800a7fec000
> RIP: 0010:[<ffffffff8115e754>] [<ffffffff8115e754>] balloon_page_dequeue+0x54/0x130
> RSP: 0018:ffff8800a7fefdc0 EFLAGS: 00010246
> RAX: ffff88013fff9a70 RBX: ffffea000056fe00 RCX: 0000000000002b7d
> RDX: ffff88013fff9a70 RSI: ffffea000056fe00 RDI: ffff88013fff9a68
> RBP: ffff8800a7fefde8 R08: ffffea000056fda0 R09: 0000000000000000
> R10: ffff8800a7fefd90 R11: 0000000000000001 R12: dead0000000000e0
> R13: ffffea000056fe20 R14: ffff880138809070 R15: ffff880138809060
> FS: 0000000000000000(0000) GS:ffff88013fc40000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 00007f229c10e000 CR3: 00000000b8b53000 CR4: 00000000000006a0
> Stack:
> 0000000000000100 ffff880138809088 ffff880138809000 ffff880138809060
> 0000000000000046 ffff8800a7fefe28 ffffffff812c86d3 ffff880138809020
> ffff880138809000 fffffffffff91900 0000000000000100 ffff880138809060
> Call Trace:
> [<ffffffff812c86d3>] leak_balloon+0x93/0x1a0
> [<ffffffff812c8bc7>] balloon+0x217/0x2a0
> [<ffffffff8143739e>] ? __schedule+0x31e/0x8b0
> [<ffffffff81078160>] ? abort_exclusive_wait+0xb0/0xb0
> [<ffffffff812c89b0>] ? update_balloon_stats+0xf0/0xf0
> [<ffffffff8105b6e9>] kthread+0xc9/0xe0
> [<ffffffff8105b620>] ? kthread_park+0x60/0x60
> [<ffffffff8143b4af>] ret_from_fork+0x3f/0x70
> [<ffffffff8105b620>] ? kthread_park+0x60/0x60
> Code: 8d 60 e0 0f 84 af 00 00 00 48 8b 43 20 a8 01 75 3b 48 89 d8 f0 0f ba 28 00 72 10 48 8b 03 f6 c4 08 75 2f 48 89 df e8 8c 83 f9 ff <49> 8b 44 24 20 4d 8d 6c 24 20 48 83 e8 20 4d 39 f5 74 7a 4c 89
> RIP [<ffffffff8115e754>] balloon_page_dequeue+0x54/0x130
> RSP <ffff8800a7fefdc0>
> ---[ end trace 43cf28060d708d5f ]---
> Kernel panic - not syncing: Fatal exception
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Kernel Offset: disabled
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
> mm/balloon_compaction.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
> index d3116be5a00f..300117f1a08f 100644
> --- a/mm/balloon_compaction.c
> +++ b/mm/balloon_compaction.c
> @@ -61,6 +61,7 @@ struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info)
> bool dequeued_page;
>
> dequeued_page = false;
> + spin_lock_irqsave(&b_dev_info->pages_lock, flags);
> list_for_each_entry_safe(page, tmp, &b_dev_info->pages, lru) {
> /*
> * Block others from accessing the 'page' while we get around
> @@ -75,15 +76,14 @@ struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info)
> continue;
> }
> #endif
> - spin_lock_irqsave(&b_dev_info->pages_lock, flags);
> balloon_page_delete(page);
> __count_vm_event(BALLOON_DEFLATE);
> - spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
> unlock_page(page);
> dequeued_page = true;
> break;
> }
> }
> + spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
>
> if (!dequeued_page) {
> /*
I think this will cause deadlocks.
pages_lock now nests within page lock, balloon_page_putback
nests them in the reverse order.
Did you test this with lockdep? You really should for
locking changes, and I'd expect it to warn about this.
Also, there's another issue there I think: after isolation page could
also get freed before we try to lock it.
We really must take a page reference before touching
the page.
I think we need something like the below to fix this issue.
Could you please try this out, and send Tested-by?
I will repost as a proper patch if this works for you.
diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
index d3116be..66d69c5 100644
--- a/mm/balloon_compaction.c
+++ b/mm/balloon_compaction.c
@@ -56,12 +56,34 @@ EXPORT_SYMBOL_GPL(balloon_page_enqueue);
*/
struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info)
{
- struct page *page, *tmp;
+ struct page *page;
unsigned long flags;
bool dequeued_page;
+ LIST_HEAD(processed); /* protected by b_dev_info->pages_lock */
dequeued_page = false;
- list_for_each_entry_safe(page, tmp, &b_dev_info->pages, lru) {
+ /*
+ * We need to go over b_dev_info->pages and lock each page,
+ * but b_dev_info->pages_lock must nest within page lock.
+ *
+ * To make this safe, remove each page from b_dev_info->pages list
+ * under b_dev_info->pages_lock, then drop this lock. Once list is
+ * empty, re-add them also under b_dev_info->pages_lock.
+ */
+ spin_lock_irqsave(&b_dev_info->pages_lock, flags);
+ while (!list_empty(&b_dev_info->pages)) {
+ page = list_first_entry(&b_dev_info->pages, typeof(*page), lru);
+ /* move to processed list to avoid going over it another time */
+ list_move(&page->lru, &processed);
+
+ if (!get_page_unless_zero(page))
+ continue;
+ /*
+ * pages_lock nests within page lock,
+ * so drop it before trylock_page
+ */
+ spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
+
/*
* Block others from accessing the 'page' while we get around
* establishing additional references and preparing the 'page'
@@ -72,6 +94,7 @@ struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info)
if (!PagePrivate(page)) {
/* raced with isolation */
unlock_page(page);
+ put_page(page);
continue;
}
#endif
@@ -80,11 +103,18 @@ struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info)
__count_vm_event(BALLOON_DEFLATE);
spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
unlock_page(page);
+ put_page(page);
dequeued_page = true;
break;
}
+ put_page(page);
+ spin_lock_irqsave(&b_dev_info->pages_lock, flags);
}
+ /* re-add remaining entries */
+ list_splice(&processed, &b_dev_info->pages);
+ spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
+
if (!dequeued_page) {
/*
* If we are unable to dequeue a balloon page because the page
^ permalink raw reply related
* Re: [PATCH 1/2] virtio_balloon: fix race by fill and leak
From: Michael S. Tsirkin @ 2016-01-01 8:26 UTC (permalink / raw)
To: Minchan Kim
Cc: Rafael Aquini, linux-kernel, stable, virtualization, linux-mm,
Andrew Morton, Konstantin Khlebnikov
In-Reply-To: <1451259313-26353-1-git-send-email-minchan@kernel.org>
On Mon, Dec 28, 2015 at 08:35:12AM +0900, Minchan Kim wrote:
> During my compaction-related stuff, I encountered a bug
> with ballooning.
>
> With repeated inflating and deflating cycle, guest memory(
> ie, cat /proc/meminfo | grep MemTotal) is decreased and
> couldn't be recovered.
>
> The reason is balloon_lock doesn't cover release_pages_balloon
> so struct virtio_balloon fields could be overwritten by race
> of fill_balloon(e,g, vb->*pfns could be critical).
>
> This patch fixes it in my test.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 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 7efc32945810..7d3e5d0e9aa4 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -209,8 +209,8 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
> */
> if (vb->num_pfns != 0)
> tell_host(vb, vb->deflate_vq);
> - mutex_unlock(&vb->balloon_lock);
> release_pages_balloon(vb);
> + mutex_unlock(&vb->balloon_lock);
> return num_freed_pages;
> }
>
> --
> 1.9.1
^ permalink raw reply
* Re: [PATCH v2 24/32] sparc: define __smp_xxx
From: David Miller @ 2015-12-31 19:44 UTC (permalink / raw)
To: mst
Cc: linux-mips, linux-ia64, linux-sh, peterz, virtualization, hpa,
sparclinux, mingo, linux-arch, linux-s390, arnd, x86, xen-devel,
mingo, linux-xtensa, user-mode-linux-devel, stefano.stabellini,
andreyknvl, adi-buildroot-devel, tglx, linux-metag,
linux-arm-kernel, andrew.cooper3, linux-kernel, ralf,
linuxppc-dev
In-Reply-To: <1451572003-2440-25-git-send-email-mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 31 Dec 2015 21:08:53 +0200
> This defines __smp_xxx barriers for sparc,
> for use by virtualization.
>
> smp_xxx barriers are removed as they are
> defined correctly by asm-generic/barriers.h
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH v2 07/32] sparc: reuse asm-generic/barrier.h
From: David Miller @ 2015-12-31 19:43 UTC (permalink / raw)
To: mst
Cc: linux-mips, linux-ia64, linux-sh, peterz, virtualization, hpa,
sparclinux, mingo, linux-arch, linux-s390, arnd, x86, xen-devel,
mingo, linux-xtensa, user-mode-linux-devel, stefano.stabellini,
andreyknvl, adi-buildroot-devel, tglx, linux-metag,
linux-arm-kernel, andrew.cooper3, linux-kernel, ralf,
linuxppc-dev
In-Reply-To: <1451572003-2440-8-git-send-email-mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 31 Dec 2015 21:06:38 +0200
> On sparc 64 bit dma_rmb, dma_wmb, smp_store_mb, smp_mb, smp_rmb,
> smp_wmb, read_barrier_depends and smp_read_barrier_depends match the
> asm-generic variants exactly. Drop the local definitions and pull in
> asm-generic/barrier.h instead.
>
> nop uses __asm__ __volatile but is otherwise identical to
> the generic version, drop that as well.
>
> This is in preparation to refactoring this code area.
>
> Note: nop() was in processor.h and not in barrier.h as on other
> architectures. Nothing seems to depend on it being there though.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* [PATCH v2 34/34] xen/io: use virt_xxx barriers
From: Michael S. Tsirkin @ 2015-12-31 19:10 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, Boris Ostrovsky, linux-arch,
linux-s390, Arnd Bergmann, x86, xen-devel, Ingo Molnar,
linux-xtensa, user-mode-linux-devel, Stefano Stabellini,
adi-buildroot-devel, Thomas Gleixner, linux-metag,
linux-arm-kernel, Konrad Rzeszutek Wilk, Andrew Cooper,
David Vrabel, linuxppc-dev
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
include/xen/interface/io/ring.h uses
full memory barriers to communicate with the other side.
For guests compiled with CONFIG_SMP, smp_wmb and smp_mb
would be sufficient, so mb() and wmb() here are only needed if
a non-SMP guest runs on an SMP host.
Switch to virt_xxx barriers which serve this exact purpose.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/xen/interface/io/ring.h | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/xen/interface/io/ring.h b/include/xen/interface/io/ring.h
index 7dc685b..21f4fbd 100644
--- a/include/xen/interface/io/ring.h
+++ b/include/xen/interface/io/ring.h
@@ -208,12 +208,12 @@ struct __name##_back_ring { \
#define RING_PUSH_REQUESTS(_r) do { \
- wmb(); /* back sees requests /before/ updated producer index */ \
+ virt_wmb(); /* back sees requests /before/ updated producer index */ \
(_r)->sring->req_prod = (_r)->req_prod_pvt; \
} while (0)
#define RING_PUSH_RESPONSES(_r) do { \
- wmb(); /* front sees responses /before/ updated producer index */ \
+ virt_wmb(); /* front sees responses /before/ updated producer index */ \
(_r)->sring->rsp_prod = (_r)->rsp_prod_pvt; \
} while (0)
@@ -250,9 +250,9 @@ struct __name##_back_ring { \
#define RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(_r, _notify) do { \
RING_IDX __old = (_r)->sring->req_prod; \
RING_IDX __new = (_r)->req_prod_pvt; \
- wmb(); /* back sees requests /before/ updated producer index */ \
+ virt_wmb(); /* back sees requests /before/ updated producer index */ \
(_r)->sring->req_prod = __new; \
- mb(); /* back sees new requests /before/ we check req_event */ \
+ virt_mb(); /* back sees new requests /before/ we check req_event */ \
(_notify) = ((RING_IDX)(__new - (_r)->sring->req_event) < \
(RING_IDX)(__new - __old)); \
} while (0)
@@ -260,9 +260,9 @@ struct __name##_back_ring { \
#define RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(_r, _notify) do { \
RING_IDX __old = (_r)->sring->rsp_prod; \
RING_IDX __new = (_r)->rsp_prod_pvt; \
- wmb(); /* front sees responses /before/ updated producer index */ \
+ virt_wmb(); /* front sees responses /before/ updated producer index */ \
(_r)->sring->rsp_prod = __new; \
- mb(); /* front sees new responses /before/ we check rsp_event */ \
+ virt_mb(); /* front sees new responses /before/ we check rsp_event */ \
(_notify) = ((RING_IDX)(__new - (_r)->sring->rsp_event) < \
(RING_IDX)(__new - __old)); \
} while (0)
@@ -271,7 +271,7 @@ struct __name##_back_ring { \
(_work_to_do) = RING_HAS_UNCONSUMED_REQUESTS(_r); \
if (_work_to_do) break; \
(_r)->sring->req_event = (_r)->req_cons + 1; \
- mb(); \
+ virt_mb(); \
(_work_to_do) = RING_HAS_UNCONSUMED_REQUESTS(_r); \
} while (0)
@@ -279,7 +279,7 @@ struct __name##_back_ring { \
(_work_to_do) = RING_HAS_UNCONSUMED_RESPONSES(_r); \
if (_work_to_do) break; \
(_r)->sring->rsp_event = (_r)->rsp_cons + 1; \
- mb(); \
+ virt_mb(); \
(_work_to_do) = RING_HAS_UNCONSUMED_RESPONSES(_r); \
} while (0)
--
MST
^ permalink raw reply related
* [PATCH v2 33/34] xenbus: use virt_xxx barriers
From: Michael S. Tsirkin @ 2015-12-31 19:10 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, Boris Ostrovsky, linux-arch,
linux-s390, Arnd Bergmann, x86, xen-devel, Ingo Molnar,
linux-xtensa, user-mode-linux-devel, Stefano Stabellini,
adi-buildroot-devel, Thomas Gleixner, linux-metag,
linux-arm-kernel, Konrad Rzeszutek Wilk, Andrew Cooper,
David Vrabel, linuxppc-dev
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
drivers/xen/xenbus/xenbus_comms.c uses
full memory barriers to communicate with the other side.
For guests compiled with CONFIG_SMP, smp_wmb and smp_mb
would be sufficient, so mb() and wmb() here are only needed if
a non-SMP guest runs on an SMP host.
Switch to virt_xxx barriers which serve this exact purpose.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/xen/xenbus/xenbus_comms.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c
index fdb0f33..ecdecce 100644
--- a/drivers/xen/xenbus/xenbus_comms.c
+++ b/drivers/xen/xenbus/xenbus_comms.c
@@ -123,14 +123,14 @@ int xb_write(const void *data, unsigned len)
avail = len;
/* Must write data /after/ reading the consumer index. */
- mb();
+ virt_mb();
memcpy(dst, data, avail);
data += avail;
len -= avail;
/* Other side must not see new producer until data is there. */
- wmb();
+ virt_wmb();
intf->req_prod += avail;
/* Implies mb(): other side will see the updated producer. */
@@ -180,14 +180,14 @@ int xb_read(void *data, unsigned len)
avail = len;
/* Must read data /after/ reading the producer index. */
- rmb();
+ virt_rmb();
memcpy(data, src, avail);
data += avail;
len -= avail;
/* Other side must not see free space until we've copied out */
- mb();
+ virt_mb();
intf->rsp_cons += avail;
pr_debug("Finished read of %i bytes (%i to go)\n", avail, len);
--
MST
^ permalink raw reply related
* [PATCH v2 32/32] virtio_ring: use virt_store_mb
From: Michael S. Tsirkin @ 2015-12-31 19:09 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, linux-arch, linux-s390, Arnd Bergmann,
x86, xen-devel, Ingo Molnar, linux-xtensa, user-mode-linux-devel,
Stefano Stabellini, adi-buildroot-devel, Thomas Gleixner,
linux-metag, linux-arm-kernel, Andrew Cooper, linuxppc-dev,
David Miller
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
We need a full barrier after writing out event index, using
virt_store_mb there seems better than open-coding. As usual, we need a
wrapper to account for strong barriers.
It's tempting to use this in vhost as well, for that, we'll
need a variant of smp_store_mb that works on __user pointers.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/linux/virtio_ring.h | 12 ++++++++++++
drivers/virtio/virtio_ring.c | 15 +++++++++------
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index f3fa55b..3a74d91 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -45,6 +45,18 @@ static inline void virtio_wmb(bool weak_barriers)
wmb();
}
+static inline void virtio_store_mb(bool weak_barriers,
+ __virtio16 *p, __virtio16 v)
+{
+ if (weak_barriers)
+ virt_store_mb(*p, v);
+ else
+ {
+ WRITE_ONCE(*p, v);
+ mb();
+ }
+}
+
struct virtio_device;
struct virtqueue;
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index ee663c4..e12e385 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -517,10 +517,10 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
/* If we expect an interrupt for the next entry, tell host
* by writing event index and flush out the write before
* the read in the next get_buf call. */
- if (!(vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT)) {
- vring_used_event(&vq->vring) = cpu_to_virtio16(_vq->vdev, vq->last_used_idx);
- virtio_mb(vq->weak_barriers);
- }
+ if (!(vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT))
+ virtio_store_mb(vq->weak_barriers,
+ &vring_used_event(&vq->vring),
+ cpu_to_virtio16(_vq->vdev, vq->last_used_idx));
#ifdef DEBUG
vq->last_add_time_valid = false;
@@ -653,8 +653,11 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
}
/* TODO: tune this threshold */
bufs = (u16)(vq->avail_idx_shadow - vq->last_used_idx) * 3 / 4;
- vring_used_event(&vq->vring) = cpu_to_virtio16(_vq->vdev, vq->last_used_idx + bufs);
- virtio_mb(vq->weak_barriers);
+
+ virtio_store_mb(vq->weak_barriers,
+ &vring_used_event(&vq->vring),
+ cpu_to_virtio16(_vq->vdev, vq->last_used_idx + bufs));
+
if (unlikely((u16)(virtio16_to_cpu(_vq->vdev, vq->vring.used->idx) - vq->last_used_idx) > bufs)) {
END_USE(vq);
return false;
--
MST
^ permalink raw reply related
* [PATCH v2 31/32] sh: support a 2-byte smp_store_mb
From: Michael S. Tsirkin @ 2015-12-31 19:09 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Arnd Bergmann, x86, xen-devel, Ingo Molnar, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, adi-buildroot-devel,
Thomas Gleixner, linux-metag, linux-arm-kernel, Andrew Cooper,
linuxppc-dev, David Miller
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
At the moment, xchg on sh only supports 4 and 1 byte values, so using it
from smp_store_mb means attempts to store a 2 byte value using this
macro fail.
And happens to be exactly what virtio drivers want to do.
Check size and fall back to a slower, but safe, WRITE_ONCE+smp_mb.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
arch/sh/include/asm/barrier.h | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/sh/include/asm/barrier.h b/arch/sh/include/asm/barrier.h
index f887c64..0cc5735 100644
--- a/arch/sh/include/asm/barrier.h
+++ b/arch/sh/include/asm/barrier.h
@@ -32,7 +32,15 @@
#define ctrl_barrier() __asm__ __volatile__ ("nop;nop;nop;nop;nop;nop;nop;nop")
#endif
-#define __smp_store_mb(var, value) do { (void)xchg(&var, value); } while (0)
+#define __smp_store_mb(var, value) do { \
+ if (sizeof(var) != 4 && sizeof(var) != 1) { \
+ WRITE_ONCE(var, value); \
+ __smp_mb(); \
+ } else { \
+ (void)xchg(&var, value); \
+ } \
+} while (0)
+
#define smp_store_mb(var, value) __smp_store_mb(var, value)
#include <asm-generic/barrier.h>
--
MST
^ permalink raw reply related
* [PATCH v2 30/32] virtio_ring: update weak barriers to use __smp_XXX
From: Michael S. Tsirkin @ 2015-12-31 19:09 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Alexander Duyck,
virtualization, H. Peter Anvin, sparclinux, linux-arch,
linux-s390, Arnd Bergmann, x86, xen-devel, Ingo Molnar,
linux-xtensa, user-mode-linux-devel, Stefano Stabellini,
adi-buildroot-devel, Thomas Gleixner, linux-metag,
linux-arm-kernel, Andrew Cooper, linuxppc-dev, David Miller
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
virtio ring uses smp_wmb on SMP and wmb on !SMP,
the reason for the later being that it might be
talking to another kernel on the same SMP machine.
This is exactly what __smp_XXX barriers do,
so switch to these instead of homegrown ifdef hacks.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/linux/virtio_ring.h | 25 ++++---------------------
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 67e06fe..f3fa55b 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -12,7 +12,7 @@
* anyone care?
*
* For virtio_pci on SMP, we don't need to order with respect to MMIO
- * accesses through relaxed memory I/O windows, so smp_mb() et al are
+ * accesses through relaxed memory I/O windows, so virt_mb() et al are
* sufficient.
*
* For using virtio to talk to real devices (eg. other heterogeneous
@@ -21,11 +21,10 @@
* actually quite cheap.
*/
-#ifdef CONFIG_SMP
static inline void virtio_mb(bool weak_barriers)
{
if (weak_barriers)
- smp_mb();
+ virt_mb();
else
mb();
}
@@ -33,7 +32,7 @@ static inline void virtio_mb(bool weak_barriers)
static inline void virtio_rmb(bool weak_barriers)
{
if (weak_barriers)
- smp_rmb();
+ virt_rmb();
else
rmb();
}
@@ -41,26 +40,10 @@ static inline void virtio_rmb(bool weak_barriers)
static inline void virtio_wmb(bool weak_barriers)
{
if (weak_barriers)
- smp_wmb();
+ virt_wmb();
else
wmb();
}
-#else
-static inline void virtio_mb(bool weak_barriers)
-{
- mb();
-}
-
-static inline void virtio_rmb(bool weak_barriers)
-{
- rmb();
-}
-
-static inline void virtio_wmb(bool weak_barriers)
-{
- wmb();
-}
-#endif
struct virtio_device;
struct virtqueue;
--
MST
^ permalink raw reply related
* [PATCH v2 29/32] Revert "virtio_ring: Update weak barriers to use dma_wmb/rmb"
From: Michael S. Tsirkin @ 2015-12-31 19:09 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, Alexander Duyck,
virtualization, H. Peter Anvin, sparclinux, linux-arch,
linux-s390, Arnd Bergmann, x86, xen-devel, Ingo Molnar,
linux-xtensa, user-mode-linux-devel, Stefano Stabellini,
adi-buildroot-devel, Thomas Gleixner, linux-metag,
linux-arm-kernel, Andrew Cooper, linuxppc-dev, David Miller
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
This reverts commit 9e1a27ea42691429e31f158cce6fc61bc79bb2e9.
While that commit optimizes !CONFIG_SMP, it mixes
up DMA and SMP concepts, making the code hard
to figure out.
A better way to optimize this is with the new __smp_XXX
barriers.
As a first step, go back to full rmb/wmb barriers
for !SMP.
We switch to __smp_XXX barriers in the next patch.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/linux/virtio_ring.h | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 8e50888..67e06fe 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -21,20 +21,19 @@
* actually quite cheap.
*/
+#ifdef CONFIG_SMP
static inline void virtio_mb(bool weak_barriers)
{
-#ifdef CONFIG_SMP
if (weak_barriers)
smp_mb();
else
-#endif
mb();
}
static inline void virtio_rmb(bool weak_barriers)
{
if (weak_barriers)
- dma_rmb();
+ smp_rmb();
else
rmb();
}
@@ -42,10 +41,26 @@ static inline void virtio_rmb(bool weak_barriers)
static inline void virtio_wmb(bool weak_barriers)
{
if (weak_barriers)
- dma_wmb();
+ smp_wmb();
else
wmb();
}
+#else
+static inline void virtio_mb(bool weak_barriers)
+{
+ mb();
+}
+
+static inline void virtio_rmb(bool weak_barriers)
+{
+ rmb();
+}
+
+static inline void virtio_wmb(bool weak_barriers)
+{
+ wmb();
+}
+#endif
struct virtio_device;
struct virtqueue;
--
MST
^ permalink raw reply related
* [PATCH v2 28/32] asm-generic: implement virt_xxx memory barriers
From: Michael S. Tsirkin @ 2015-12-31 19:09 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, linux-arch, linux-s390, Arnd Bergmann,
Jonathan Corbet, x86, xen-devel, Ingo Molnar, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, adi-buildroot-devel,
Thomas Gleixner, linux-metag, linux-arm-kernel, Andrew Cooper,
linux-doc, linuxppc-dev, David Miller
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
Guests running within virtual machines might be affected by SMP effects even if
the guest itself is compiled without SMP support. This is an artifact of
interfacing with an SMP host while running an UP kernel. Using mandatory
barriers for this use-case would be possible but is often suboptimal.
In particular, virtio uses a bunch of confusing ifdefs to work around
this, while xen just uses the mandatory barriers.
To better handle this case, low-level virt_mb() etc macros are made available.
These are implemented trivially using the low-level __smp_xxx macros,
the purpose of these wrappers is to annotate those specific cases.
These have the same effect as smp_mb() etc when SMP is enabled, but generate
identical code for SMP and non-SMP systems. For example, virtual machine guests
should use virt_mb() rather than smp_mb() when synchronizing against a
(possibly SMP) host.
Suggested-by: David Miller <davem@davemloft.net>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/asm-generic/barrier.h | 11 +++++++++++
Documentation/memory-barriers.txt | 28 +++++++++++++++++++++++-----
2 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
index 8752964..1cceca14 100644
--- a/include/asm-generic/barrier.h
+++ b/include/asm-generic/barrier.h
@@ -196,5 +196,16 @@ do { \
#endif
+/* Barriers for virtual machine guests when talking to an SMP host */
+#define virt_mb() __smp_mb()
+#define virt_rmb() __smp_rmb()
+#define virt_wmb() __smp_wmb()
+#define virt_read_barrier_depends() __smp_read_barrier_depends()
+#define virt_store_mb(var, value) __smp_store_mb(var, value)
+#define virt_mb__before_atomic() __smp_mb__before_atomic()
+#define virt_mb__after_atomic() __smp_mb__after_atomic()
+#define virt_store_release(p, v) __smp_store_release(p, v)
+#define virt_load_acquire(p) __smp_load_acquire(p)
+
#endif /* !__ASSEMBLY__ */
#endif /* __ASM_GENERIC_BARRIER_H */
diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index aef9487..8f4a93a 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1655,17 +1655,18 @@ macro is a good place to start looking.
SMP memory barriers are reduced to compiler barriers on uniprocessor compiled
systems because it is assumed that a CPU will appear to be self-consistent,
and will order overlapping accesses correctly with respect to itself.
+However, see the subsection on "Virtual Machine Guests" below.
[!] Note that SMP memory barriers _must_ be used to control the ordering of
references to shared memory on SMP systems, though the use of locking instead
is sufficient.
Mandatory barriers should not be used to control SMP effects, since mandatory
-barriers unnecessarily impose overhead on UP systems. They may, however, be
-used to control MMIO effects on accesses through relaxed memory I/O windows.
-These are required even on non-SMP systems as they affect the order in which
-memory operations appear to a device by prohibiting both the compiler and the
-CPU from reordering them.
+barriers impose unnecessary overhead on both SMP and UP systems. They may,
+however, be used to control MMIO effects on accesses through relaxed memory I/O
+windows. These barriers are required even on non-SMP systems as they affect
+the order in which memory operations appear to a device by prohibiting both the
+compiler and the CPU from reordering them.
There are some more advanced barrier functions:
@@ -2948,6 +2949,23 @@ The Alpha defines the Linux kernel's memory barrier model.
See the subsection on "Cache Coherency" above.
+VIRTUAL MACHINE GUESTS
+-------------------
+
+Guests running within virtual machines might be affected by SMP effects even if
+the guest itself is compiled without SMP support. This is an artifact of
+interfacing with an SMP host while running an UP kernel. Using mandatory
+barriers for this use-case would be possible but is often suboptimal.
+
+To handle this case optimally, low-level virt_mb() etc macros are available.
+These have the same effect as smp_mb() etc when SMP is enabled, but generate
+identical code for SMP and non-SMP systems. For example, virtual machine guests
+should use virt_mb() rather than smp_mb() when synchronizing against a
+(possibly SMP) host.
+
+These are equivalent to smp_mb() etc counterparts in all other respects,
+in particular, they do not control MMIO effects: to control
+MMIO effects, use mandatory barriers.
============
EXAMPLE USES
--
MST
^ permalink raw reply related
* [PATCH v2 27/32] x86: define __smp_xxx
From: Michael S. Tsirkin @ 2015-12-31 19:09 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, 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,
Andrey Konovalov, adi-buildroot-devel, Andy Lutomirski,
Thomas Gleixner, linux-metag, linux-arm-kernel, Andrew Cooper,
linuxppc-dev
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
This defines __smp_xxx barriers for x86,
for use by virtualization.
smp_xxx barriers are removed as they are
defined correctly by asm-generic/barriers.h
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/x86/include/asm/barrier.h | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
index cc4c2a7..a584e1c 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -31,17 +31,10 @@
#endif
#define dma_wmb() barrier()
-#ifdef CONFIG_SMP
-#define smp_mb() mb()
-#define smp_rmb() dma_rmb()
-#define smp_wmb() barrier()
-#define smp_store_mb(var, value) do { (void)xchg(&var, value); } while (0)
-#else /* !SMP */
-#define smp_mb() barrier()
-#define smp_rmb() barrier()
-#define smp_wmb() barrier()
-#define smp_store_mb(var, value) do { WRITE_ONCE(var, value); barrier(); } while (0)
-#endif /* SMP */
+#define __smp_mb() mb()
+#define __smp_rmb() dma_rmb()
+#define __smp_wmb() barrier()
+#define __smp_store_mb(var, value) do { (void)xchg(&var, value); } while (0)
#if defined(CONFIG_X86_PPRO_FENCE)
@@ -50,31 +43,31 @@
* model and we should fall back to full barriers.
*/
-#define smp_store_release(p, v) \
+#define __smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
- smp_mb(); \
+ __smp_mb(); \
WRITE_ONCE(*p, v); \
} while (0)
-#define smp_load_acquire(p) \
+#define __smp_load_acquire(p) \
({ \
typeof(*p) ___p1 = READ_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
- smp_mb(); \
+ __smp_mb(); \
___p1; \
})
#else /* regular x86 TSO memory ordering */
-#define smp_store_release(p, v) \
+#define __smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
barrier(); \
WRITE_ONCE(*p, v); \
} while (0)
-#define smp_load_acquire(p) \
+#define __smp_load_acquire(p) \
({ \
typeof(*p) ___p1 = READ_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
@@ -85,8 +78,8 @@ do { \
#endif
/* Atomic operations are already serializing on x86 */
-#define smp_mb__before_atomic() barrier()
-#define smp_mb__after_atomic() barrier()
+#define __smp_mb__before_atomic() barrier()
+#define __smp_mb__after_atomic() barrier()
#include <asm-generic/barrier.h>
--
MST
^ permalink raw reply related
* [PATCH v2 26/32] xtensa: define __smp_xxx
From: Michael S. Tsirkin @ 2015-12-31 19:09 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
Max Filippov, H. Peter Anvin, sparclinux, linux-arch, linux-s390,
Arnd Bergmann, x86, xen-devel, Ingo Molnar, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, adi-buildroot-devel,
Thomas Gleixner, linux-metag, linux-arm-kernel, Chris Zankel,
Andrew Cooper, linuxppc-dev, David Miller
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
This defines __smp_xxx barriers for xtensa,
for use by virtualization.
smp_xxx barriers are removed as they are
defined correctly by asm-generic/barriers.h
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/xtensa/include/asm/barrier.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/xtensa/include/asm/barrier.h b/arch/xtensa/include/asm/barrier.h
index 5b88774..956596e 100644
--- a/arch/xtensa/include/asm/barrier.h
+++ b/arch/xtensa/include/asm/barrier.h
@@ -13,8 +13,8 @@
#define rmb() barrier()
#define wmb() mb()
-#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
^ permalink raw reply related
* [PATCH v2 25/32] tile: define __smp_xxx
From: Michael S. Tsirkin @ 2015-12-31 19:09 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
Chris Metcalf, H. Peter Anvin, sparclinux, linux-arch, linux-s390,
Arnd Bergmann, x86, xen-devel, Ingo Molnar, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, adi-buildroot-devel,
Thomas Gleixner, linux-metag, linux-arm-kernel, Andrew Cooper,
linuxppc-dev, David Miller
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
This defines __smp_xxx barriers for tile,
for use by virtualization.
Some smp_xxx barriers are removed as they are
defined correctly by asm-generic/barriers.h
Note: for 32 bit, keep smp_mb__after_atomic around since it's faster
than the generic implementation.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/tile/include/asm/barrier.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/tile/include/asm/barrier.h b/arch/tile/include/asm/barrier.h
index 96a42ae..d552228 100644
--- a/arch/tile/include/asm/barrier.h
+++ b/arch/tile/include/asm/barrier.h
@@ -79,11 +79,12 @@ mb_incoherent(void)
* But after the word is updated, the routine issues an "mf" before returning,
* and since it's a function call, we don't even need a compiler barrier.
*/
-#define smp_mb__before_atomic() smp_mb()
-#define smp_mb__after_atomic() do { } while (0)
+#define __smp_mb__before_atomic() __smp_mb()
+#define __smp_mb__after_atomic() do { } while (0)
+#define smp_mb__after_atomic() __smp_mb__after_atomic()
#else /* 64 bit */
-#define smp_mb__before_atomic() smp_mb()
-#define smp_mb__after_atomic() smp_mb()
+#define __smp_mb__before_atomic() __smp_mb()
+#define __smp_mb__after_atomic() __smp_mb()
#endif
#include <asm-generic/barrier.h>
--
MST
^ permalink raw reply related
* [PATCH v2 24/32] sparc: define __smp_xxx
From: Michael S. Tsirkin @ 2015-12-31 19:08 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
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, Ralf Baechle, linuxppc-dev,
David Miller
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
This defines __smp_xxx barriers for sparc,
for use by virtualization.
smp_xxx barriers are removed as they are
defined correctly by asm-generic/barriers.h
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/sparc/include/asm/barrier_64.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/sparc/include/asm/barrier_64.h b/arch/sparc/include/asm/barrier_64.h
index 26c3f72..c9f6ee6 100644
--- a/arch/sparc/include/asm/barrier_64.h
+++ b/arch/sparc/include/asm/barrier_64.h
@@ -37,14 +37,14 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \
#define rmb() __asm__ __volatile__("":::"memory")
#define wmb() __asm__ __volatile__("":::"memory")
-#define smp_store_release(p, v) \
+#define __smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
barrier(); \
WRITE_ONCE(*p, v); \
} while (0)
-#define smp_load_acquire(p) \
+#define __smp_load_acquire(p) \
({ \
typeof(*p) ___p1 = READ_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
@@ -52,8 +52,8 @@ do { \
___p1; \
})
-#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
^ permalink raw reply related
* [PATCH v2 23/32] sh: define __smp_xxx, fix smp_store_mb for !SMP
From: Michael S. Tsirkin @ 2015-12-31 19:08 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, linux-ia64, linux-sh, Peter Zijlstra, virtualization,
H. Peter Anvin, sparclinux, Ingo Molnar, linux-arch, linux-s390,
Arnd Bergmann, x86, xen-devel, Ingo Molnar, linux-xtensa,
user-mode-linux-devel, Stefano Stabellini, adi-buildroot-devel,
Thomas Gleixner, linux-metag, linux-arm-kernel, Andrew Cooper,
linuxppc-dev, David Miller
In-Reply-To: <1451572003-2440-1-git-send-email-mst@redhat.com>
sh variant of smp_store_mb() calls xchg() on !SMP which is stronger than
implied by both the name and the documentation.
define __smp_store_mb instead: code in asm-generic/barrier.h
will then define smp_store_mb correctly depending on
CONFIG_SMP.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/sh/include/asm/barrier.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/sh/include/asm/barrier.h b/arch/sh/include/asm/barrier.h
index bf91037..f887c64 100644
--- a/arch/sh/include/asm/barrier.h
+++ b/arch/sh/include/asm/barrier.h
@@ -32,7 +32,8 @@
#define ctrl_barrier() __asm__ __volatile__ ("nop;nop;nop;nop;nop;nop;nop;nop")
#endif
-#define smp_store_mb(var, value) do { (void)xchg(&var, value); } while (0)
+#define __smp_store_mb(var, value) do { (void)xchg(&var, value); } while (0)
+#define smp_store_mb(var, value) __smp_store_mb(var, value)
#include <asm-generic/barrier.h>
--
MST
^ permalink raw reply related
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