* [PATCH v3 2/2] virtio_mmio: add cleanup for virtio_mmio_remove
From: weiping zhang @ 2017-12-06 13:59 UTC (permalink / raw)
To: cohuck, mst, jasowang; +Cc: virtualization
In-Reply-To: <cover.1512568542.git.zhangweiping@didichuxing.com>
cleanup all resource allocated by virtio_mmio_probe.
Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
---
drivers/virtio/virtio_mmio.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index ec40104..a9192fe 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -610,7 +610,13 @@ static int virtio_mmio_probe(struct platform_device *pdev)
static int virtio_mmio_remove(struct platform_device *pdev)
{
struct virtio_mmio_device *vm_dev = platform_get_drvdata(pdev);
+ struct resource *mem;
+ iounmap(vm_dev->base);
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (mem)
+ devm_release_mem_region(&pdev->dev, mem->start,
+ resource_size(mem));
unregister_virtio_device(&vm_dev->vdev);
return 0;
--
2.9.4
^ permalink raw reply related
* Re: [PATCH] ptr_ring: Add barriers to fix NULL-pointer exception
From: Cherian, George @ 2017-12-06 14:08 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, edumazet@google.com,
davem@davemloft.net
In-Reply-To: <20171206153349-mutt-send-email-mst@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 9391 bytes --]
Hi Michael,
_____________________________
From: Michael S. Tsirkin <mst@redhat.com>
Sent: Wednesday, December 6, 2017 7:05 PM
Subject: Re: [PATCH] ptr_ring: Add barriers to fix NULL-pointer exception
To: Cherian, George <george.cherian@cavium.com>
Cc: <linux-kernel@vger.kernel.org>, <davem@davemloft.net>, <jasowang@redhat.com>, <edumazet@google.com>, <netdev@vger.kernel.org>, <virtualization@lists.linux-foundation.org>
On Wed, Dec 06, 2017 at 09:57:41AM +0000, George Cherian wrote:
> While running a multiple VM testscase with each VM running iperf
> traffic between others the following kernel NULL pointer exception
> was seen.
>
> Race appears when the tun driver instance of one VM calls skb_array_produce
> (from tun_net_xmit) and the the destined VM's skb_array_consume
> (from tun_ring_recv), which could run concurrently on another core. Due to
> which the sock_wfree gets called again from the tun_ring_recv context.
>
> The fix is to add write/read barrier calls to be sure that we get proper
> values in the tun_ring_recv context.
>
> Crash log
> [35321.580227] Unable to handle kernel NULL pointer dereference at virtual address 00000060
> [35321.596720] pgd = ffff809ee552f000
> [35321.603723] [00000060] *pgd=0000009f514ac003, *pud=0000009f54f7c003, *pmd=0000000000000000
> [35321.620588] Internal error: Oops: 96000006 1 SMP
> [35321.630461] Modules linked in: xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_4
> [35321.728501] CPU: 114 PID: 5560 Comm: qemu-system-aar Not tainted 4.11.8-4k-vhe-lse+ #3
> [35321.744510] Hardware name: Cavium Inc. Unknown/Unknown, BIOS 1.0 07/24/2017
> [35321.758602] task: ffff80bed7fca880 task.stack: ffff80beb5128000
> [35321.770600] PC is at sock_wfree+0x24/0x80
> [35321.778746] LR is at skb_release_head_state+0x68/0xf8
> [35321.788979] pc : [<ffff000008a772fc>] lr : [<ffff000008a79238>] pstate: 40400149
> [35321.803930] sp : ffff80beb512bc30
> [35321.810648] x29: ffff80beb512bc30 x28: ffff80bed7fca880
> [35321.821400] x27: 000000000000004e x26: 0000000000000000
> [35321.832156] x25: 000000000000000c x24: 0000000000000000
> [35321.842947] x23: ffff809ece3e4900 x22: ffff80beb512be00
> [35321.853729] x21: ffff000009138000 x20: 0000000000000144
> [35321.864507] x19: 0000000000000000 x18: 0000000000000014
> [35321.875276] x17: 0000ffff9d9689a0 x16: ffff00000828b3f8
> [35321.886048] x15: 0000504d7b000000 x14: e90ab50c48680a08
> [35321.896824] x13: 0101000017773f52 x12: 1080d422c00e5db6
> [35321.907595] x11: 68c322bd3930cf7a x10: a8c0d07aa8c0ad16
> [35321.918352] x9 : 000000001da4ed90 x8 : b50c48680a080101
> [35321.929099] x7 : 000017770c521080 x6 : 000000001d6c13f2
> [35321.939865] x5 : 000000001d6c13f2 x4 : 000000000000000e
> [35321.950619] x3 : 000000085ff97d82 x2 : 0000000000000000
> [35321.961376] x1 : ffff000008a772d8 x0 : 0000000000000500
> [35321.975193] Process qemu-system-aar (pid: 5560, stack limit = 0xffff80beb5128000)
> [35321.990347] Stack: (0xffff80beb512bc30 to 0xffff80beb512c000)
> [35322.001982] bc20: ffff80beb512bc50 ffff000008a79238
> [35322.017817] bc40: ffff809e8fd7be00 000000000000004e ffff80beb512bc70 ffff000008a79488
> [35322.033651] bc60: ffff809e8fd7be00 ffff00000881307c ffff80beb512bc90 ffff000008a79678
> [35322.049489] bc80: ffff809e8fd7be00 ffff80beb512be00 ffff80beb512bcb0 ffff000008812f24
> [35322.065321] bca0: ffff809e8fd7be00 000000000000004e ffff80beb512bd50 ffff0000088133f0
> [35322.081165] bcc0: ffff809ece3e4900 0000000000011000 ffff80beb512bdd8 ffff80beb512be00
> [35322.097001] bce0: 000000001d6c13a4 0000000000000015 0000000000000124 000000000000003f
> [35322.112866] bd00: ffff000008bc2000 ffff00000847b5ac 0000000000020000 ffff80be00080000
> [35322.128701] bd20: 0022000000000001 ffff80bed7fc0010 ffff000008100c38 0000000000000000
> [35322.144539] bd40: 0000000000000000 0000000000040b08 ffff80beb512bd80 ffff000008288f80
> [35322.160395] bd60: ffff000009138000 ffff809ee7cd3500 0000000000011000 ffff80beb512beb0
> [35322.176255] bd80: ffff80beb512be30 ffff00000828a224 0000000000011000 ffff809ee7cd3500
> [35322.192109] bda0: 000000001d6c13a4 ffff80beb512beb0 0000000000011000 0000000000000000
> [35322.207974] bdc0: 0000000000000000 000000001d6c13a4 0000000000011000 ffff809ee7cd3500
> [35322.223822] bde0: 000000000000004e 0000000000000000 0000000000000000 0000000000000000
> [35322.239661] be00: ffff80be00000000 000000000000004e 0000000000010fb2 ffff80beb512bdc8
> [35322.255519] be20: 0000000000000001 0000000000040b08 ffff80beb512be70 ffff00000828b464
> [35322.271392] be40: ffff000009138000 ffff809ee7cd3501 ffff809ee7cd3500 000000001d6c13a4
> [35322.287255] be60: 0000000000011000 0000000000000015 0000000000000000 ffff0000080833f0
> [35322.303090] be80: 0000000000000000 000080bef0071000 ffffffffffffffff 0000ffff9d9689cc
> [35322.318951] bea0: 0000000080000000 000080bef0071000 000000000000004e 0000000000040b08
> [35322.334771] bec0: 000000000000000e 000000001d6c13a4 0000000000011000 0000ffff9cc89108
> [35322.350640] bee0: 0000000000000002 0000ffff9cc89000 0000ffff9cc896f0 0000000000000000
> [35322.366500] bf00: 000000000000003f 000000001da4ed90 a8c0d07aa8c0ad16 68c322bd3930cf7a
> [35322.382358] bf20: 1080d422c00e5db6 0101000017773f52 e90ab50c48680a08 0000504d7b000000
> [35322.398209] bf40: 0000000000000000 0000ffff9d9689a0 0000000000000014 0000000000000030
> [35322.414063] bf60: 000000001d6c13a4 000000001d6c0db0 000000001d6d1db0 00000000006fbbc8
> [35322.429901] bf80: 0000000000011000 000000001d6ab3e8 000000001d6ab3a4 00000000007ee4c8
> [35322.445751] bfa0: 0000000000000000 0000fffff363ab70 0000ffff9d9689b8 0000fffff363ab30
> [35322.461588] bfc0: 0000ffff9d9689cc 0000000080000000 000000000000000e 000000000000003f
> [35322.477445] bfe0: 0000000000000000 0000000000000000 ffff809ed043d758 0000000000000000
> [35322.493283] Call trace:
> [35322.498275] Exception stack(0xffff80beb512ba40 to 0xffff80beb512bb70)
> [35322.511303] ba40: 0000000000000000 0001000000000000 ffff80beb512bc30 ffff000008a772fc
> [35322.527152] ba60: 0000000040400149 0000000000000049 ffff000008bc2000 ffff80bed7fca880
> [35322.542992] ba80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [35322.558863] baa0: 0000000000000000 000000001d895758 ffff80beb512bb78 0000000000000000
> [35322.574702] bac0: 000000050000000b 0000000800000001 0000000a00000001 0000000b00000001
> [35322.590550] bae0: 0000000e00000001 0000001800010001 ffff80beb512bbf0 0000000000040b08
> [35322.606416] bb00: 0000000000000500 ffff000008a772d8 0000000000000000 000000085ff97d82
> [35322.622287] bb20: 000000000000000e 000000001d6c13f2 000000001d6c13f2 000017770c521080
> [35322.638144] bb40: b50c48680a080101 000000001da4ed90 a8c0d07aa8c0ad16 68c322bd3930cf7a
> [35322.653992] bb60: 1080d422c00e5db6 0101000017773f52
> [35322.663908] [<ffff000008a772fc>] sock_wfree+0x24/0x80
> [35322.674168] [<ffff000008a79238>] skb_release_head_state+0x68/0xf8
> [35322.686535] [<ffff000008a79488>] skb_release_all+0x20/0x40
> [35322.697663] [<ffff000008a79678>] consume_skb+0x38/0xd8
> [35322.708120] [<ffff000008812f24>] tun_do_read.part.9+0x20c/0x4f0
> [35322.720149] [<ffff0000088133f0>] tun_chr_read_iter+0xc0/0xe0
> [35322.731638] [<ffff000008288f80>] __vfs_read+0x100/0x160
> [35322.742249] [<ffff00000828a224>] vfs_read+0x8c/0x148
> [35322.752344] [<ffff00000828b464>] SyS_read+0x6c/0xd8
> [35322.762263] [<ffff0000080833f0>] el0_svc_naked+0x24/0x28
> [35322.773042] Code: d503201f f9400e93 b940e280 91051274 (f9403261)
>
> Reported-by: Joseph DeVincentis <Joseph.DeVincentis@cavium.com>
> Signed-off-by: George Cherian <george.cherian@cavium.com>
> ---
> include/linux/ptr_ring.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
> index 37b4bb2..bc3b36b 100644
> --- a/include/linux/ptr_ring.h
> +++ b/include/linux/ptr_ring.h
> @@ -106,6 +106,12 @@ static inline int __ptr_ring_produce(struct ptr_ring *r, void *ptr)
> {
> if (unlikely(!r->size) || r->queue[r->producer])
> return -ENOSPC;
> + /*
> + * This barrier is necessary in order to prevent race condition with
> + * with __ptr_ring_consume(). By this we make sure all the prior
> + * writes to *ptr elements are updated.
> + */
> + wmb();
>
> r->queue[r->producer++] = ptr;
> if (unlikely(r->producer >= r->size))
> @@ -275,6 +281,13 @@ static inline void *__ptr_ring_consume(struct ptr_ring *r)
> if (ptr)
> __ptr_ring_discard_one(r);
>
> + /*
> + * This barrier is necessary in order to prevent race condition with
> + * with __ptr_ring_produce(). Make sure all the elements of ptr is
> + * in sync with the earlier writes which was done prior to pushing
> + * it to ring
> + */
> + rmb();
> return ptr;
> }
You are trying to synchronise two CPUs so non-smp barriers make no
sense. wmb/rmb are for synchronising with MMIO.
What happens when CONFIG_SMP is not set. smp_wmb/rmb becomes compiler barriers(atleast for arm64). I guess that is not what we need. An SMP barrier cannot replace a mandatory barrier, but a mandatory barrier can replace an SMP barrier.
I will try out your patch too and update the results.
But I would need couple of days time. Sorry for the delay.
> --
> 2.1.4
[-- Attachment #1.2: Type: text/html, Size: 11619 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] ptr_ring: Add barriers to fix NULL-pointer exception
From: Michael S. Tsirkin @ 2017-12-06 15:57 UTC (permalink / raw)
To: Cherian, George
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, edumazet@google.com,
davem@davemloft.net
In-Reply-To: <BN3PR0701MB1702F47F81C945950E284DE0F9320@BN3PR0701MB1702.namprd07.prod.outlook.com>
On Wed, Dec 06, 2017 at 02:08:54PM +0000, Cherian, George wrote:
> > @@ -275,6 +281,13 @@ static inline void *__ptr_ring_consume(struct ptr_ring
> *r)
> > if (ptr)
> > __ptr_ring_discard_one(r);
> >
> > + /*
> > + * This barrier is necessary in order to prevent race condition with
> > + * with __ptr_ring_produce(). Make sure all the elements of ptr is
> > + * in sync with the earlier writes which was done prior to pushing
> > + * it to ring
> > + */
> > + rmb();
> > return ptr;
> > }
>
> You are trying to synchronise two CPUs so non-smp barriers make no
> sense. wmb/rmb are for synchronising with MMIO.
>
> What happens when CONFIG_SMP is not set. smp_wmb/rmb becomes compiler barriers
> (atleast for arm64).
And that is because all read and writes always appear in order when done
from the same CPU.
In case of reads, we do not need a barrier at all (except on dec alpha)
because a read through a pointer can't bypass a read of a pointer.
> I guess that is not what we need.
Maybe, but I don't yet see why not.
> An SMP barrier cannot
> replace a mandatory barrier, but a mandatory barrier can replace an SMP
> barrier.
This does imply that you can always replace a weak barrier with a strong
one, but does not mean you should.
> I will try out your patch too and update the results.
> But I would need couple of days time. Sorry for the delay.
Thanks for the testing.
--
MST
^ permalink raw reply
* [PATCH net-next] virtio_net: Disable interrupts if napi_complete_done rescheduled napi
From: Toshiaki Makita @ 2017-12-07 4:15 UTC (permalink / raw)
To: David S . Miller, Michael S . Tsirkin, Jason Wang
Cc: netdev, Eric Dumazet, virtualization
Since commit 39e6c8208d7b ("net: solve a NAPI race") napi has been able
to be rescheduled within napi_complete_done() even in non-busypoll case,
but virtnet_poll() always enabled interrupts before complete, and when
napi was rescheduled within napi_complete_done() it did not disable
interrupts.
This caused more interrupts when event idx is disabled.
According to commit cbdadbbf0c79 ("virtio_net: fix race in RX VQ
processing") we cannot place virtqueue_enable_cb_prepare() after
NAPI_STATE_SCHED is cleared, so disable interrupts again if
napi_complete_done() returned false.
Tested with vhost-user of OVS 2.7 on host, which does not have the event
idx feature.
* Before patch:
$ netperf -t UDP_STREAM -H 192.168.150.253 -l 60 -- -m 1472
MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.150.253 () port 0 AF_INET
Socket Message Elapsed Messages
Size Size Time Okay Errors Throughput
bytes bytes secs # # 10^6bits/sec
212992 1472 60.00 32763206 0 6430.32
212992 60.00 23384299 4589.56
Interrupts on guest: 9872369
Packets/interrupt: 2.37
* After patch
$ netperf -t UDP_STREAM -H 192.168.150.253 -l 60 -- -m 1472
MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.150.253 () port 0 AF_INET
Socket Message Elapsed Messages
Size Size Time Okay Errors Throughput
bytes bytes secs # # 10^6bits/sec
212992 1472 60.00 32794646 0 6436.49
212992 60.00 32793501 6436.27
Interrupts on guest: 4941299
Packets/interrupt: 6.64
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/net/virtio_net.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 19a985e..c0db48d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -261,9 +261,12 @@ static void virtqueue_napi_complete(struct napi_struct *napi,
int opaque;
opaque = virtqueue_enable_cb_prepare(vq);
- if (napi_complete_done(napi, processed) &&
- unlikely(virtqueue_poll(vq, opaque)))
- virtqueue_napi_schedule(napi, vq);
+ if (napi_complete_done(napi, processed)) {
+ if (unlikely(virtqueue_poll(vq, opaque)))
+ virtqueue_napi_schedule(napi, vq);
+ } else {
+ virtqueue_disable_cb(vq);
+ }
}
static void skb_xmit_done(struct virtqueue *vq)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next] virtio_net: Disable interrupts if napi_complete_done rescheduled napi
From: Michael S. Tsirkin @ 2017-12-07 5:09 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: netdev, David S . Miller, Eric Dumazet, virtualization
In-Reply-To: <1512620115-7569-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On Thu, Dec 07, 2017 at 01:15:15PM +0900, Toshiaki Makita wrote:
> Since commit 39e6c8208d7b ("net: solve a NAPI race") napi has been able
> to be rescheduled within napi_complete_done() even in non-busypoll case,
> but virtnet_poll() always enabled interrupts before complete, and when
> napi was rescheduled within napi_complete_done() it did not disable
> interrupts.
> This caused more interrupts when event idx is disabled.
>
> According to commit cbdadbbf0c79 ("virtio_net: fix race in RX VQ
> processing") we cannot place virtqueue_enable_cb_prepare() after
> NAPI_STATE_SCHED is cleared, so disable interrupts again if
> napi_complete_done() returned false.
>
> Tested with vhost-user of OVS 2.7 on host, which does not have the event
> idx feature.
>
> * Before patch:
>
> $ netperf -t UDP_STREAM -H 192.168.150.253 -l 60 -- -m 1472
> MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.150.253 () port 0 AF_INET
> Socket Message Elapsed Messages
> Size Size Time Okay Errors Throughput
> bytes bytes secs # # 10^6bits/sec
>
> 212992 1472 60.00 32763206 0 6430.32
> 212992 60.00 23384299 4589.56
>
> Interrupts on guest: 9872369
> Packets/interrupt: 2.37
>
> * After patch
>
> $ netperf -t UDP_STREAM -H 192.168.150.253 -l 60 -- -m 1472
> MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.150.253 () port 0 AF_INET
> Socket Message Elapsed Messages
> Size Size Time Okay Errors Throughput
> bytes bytes secs # # 10^6bits/sec
>
> 212992 1472 60.00 32794646 0 6436.49
> 212992 60.00 32793501 6436.27
>
> Interrupts on guest: 4941299
> Packets/interrupt: 6.64
>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
it might make sense in net and not -next since tx napi regressed performance
in some configs, this might bring it back at least partially.
Jason - what do you think?
> ---
> drivers/net/virtio_net.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 19a985e..c0db48d 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -261,9 +261,12 @@ static void virtqueue_napi_complete(struct napi_struct *napi,
> int opaque;
>
> opaque = virtqueue_enable_cb_prepare(vq);
> - if (napi_complete_done(napi, processed) &&
> - unlikely(virtqueue_poll(vq, opaque)))
> - virtqueue_napi_schedule(napi, vq);
> + if (napi_complete_done(napi, processed)) {
> + if (unlikely(virtqueue_poll(vq, opaque)))
> + virtqueue_napi_schedule(napi, vq);
> + } else {
> + virtqueue_disable_cb(vq);
> + }
> }
>
> static void skb_xmit_done(struct virtqueue *vq)
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: Disable interrupts if napi_complete_done rescheduled napi
From: Jason Wang @ 2017-12-07 7:08 UTC (permalink / raw)
To: Michael S. Tsirkin, Toshiaki Makita
Cc: netdev, Eric Dumazet, David S . Miller, virtualization
In-Reply-To: <20171207070618-mutt-send-email-mst@kernel.org>
On 2017年12月07日 13:09, Michael S. Tsirkin wrote:
> On Thu, Dec 07, 2017 at 01:15:15PM +0900, Toshiaki Makita wrote:
>> Since commit 39e6c8208d7b ("net: solve a NAPI race") napi has been able
>> to be rescheduled within napi_complete_done() even in non-busypoll case,
>> but virtnet_poll() always enabled interrupts before complete, and when
>> napi was rescheduled within napi_complete_done() it did not disable
>> interrupts.
>> This caused more interrupts when event idx is disabled.
>>
>> According to commit cbdadbbf0c79 ("virtio_net: fix race in RX VQ
>> processing") we cannot place virtqueue_enable_cb_prepare() after
>> NAPI_STATE_SCHED is cleared, so disable interrupts again if
>> napi_complete_done() returned false.
>>
>> Tested with vhost-user of OVS 2.7 on host, which does not have the event
>> idx feature.
>>
>> * Before patch:
>>
>> $ netperf -t UDP_STREAM -H 192.168.150.253 -l 60 -- -m 1472
>> MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.150.253 () port 0 AF_INET
>> Socket Message Elapsed Messages
>> Size Size Time Okay Errors Throughput
>> bytes bytes secs # # 10^6bits/sec
>>
>> 212992 1472 60.00 32763206 0 6430.32
>> 212992 60.00 23384299 4589.56
>>
>> Interrupts on guest: 9872369
>> Packets/interrupt: 2.37
>>
>> * After patch
>>
>> $ netperf -t UDP_STREAM -H 192.168.150.253 -l 60 -- -m 1472
>> MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.150.253 () port 0 AF_INET
>> Socket Message Elapsed Messages
>> Size Size Time Okay Errors Throughput
>> bytes bytes secs # # 10^6bits/sec
>>
>> 212992 1472 60.00 32794646 0 6436.49
>> 212992 60.00 32793501 6436.27
>>
>> Interrupts on guest: 4941299
>> Packets/interrupt: 6.64
>>
>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> it might make sense in net and not -next since tx napi regressed performance
> in some configs, this might bring it back at least partially.
> Jason - what do you think?
No sure, the regression I saw was tested with event idx on. And
virtqueue_disable_cb() does almost nothing for event idx (or even a
little bit slower).
The patch looks good.
Acked-by: Jason Wang <jasowang@redhat.com>
>
>> ---
>> drivers/net/virtio_net.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 19a985e..c0db48d 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -261,9 +261,12 @@ static void virtqueue_napi_complete(struct napi_struct *napi,
>> int opaque;
>>
>> opaque = virtqueue_enable_cb_prepare(vq);
>> - if (napi_complete_done(napi, processed) &&
>> - unlikely(virtqueue_poll(vq, opaque)))
>> - virtqueue_napi_schedule(napi, vq);
>> + if (napi_complete_done(napi, processed)) {
>> + if (unlikely(virtqueue_poll(vq, opaque)))
>> + virtqueue_napi_schedule(napi, vq);
>> + } else {
>> + virtqueue_disable_cb(vq);
>> + }
>> }
>>
>> static void skb_xmit_done(struct virtqueue *vq)
>> --
>> 1.8.3.1
>>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] virtio-net: help live migrate SR-IOV devices
From: achiad shochat @ 2017-12-07 7:28 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jakub Kicinski, Hannes Frederic Sowa, Sridhar Samudrala,
Alexander Duyck, virtualization, Shannon Nelson, Achiad,
Peter Waskiewicz Jr, netdev, Singhai, Anjali, Andy Gospodarek,
Or Gerlitz
In-Reply-To: <20171205211603-mutt-send-email-mst@kernel.org>
On 5 December 2017 at 21:20, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Dec 05, 2017 at 11:59:17AM +0200, achiad shochat wrote:
>> Then we'll have a single solution for both netvsc and virtio (and any
>> other PV device).
>> And we could handle the VF DMA dirt issue agnostically.
>
> For the record, I won't block patches adding this kist to virtio
> on the basis that they must be generic. It's not a lot
> of code, implementation can come first, prettify later.
It's not a lot of code either way.
So I fail to understand why not to do it right from the beginning.
For the record...
>
> But we do need to have a discussion about how devices are paired.
> I am not sure using just MAC works. E.g. some passthrough
> devices don't give host ability to set the MAC.
> Are these worth worrying about?
>
> --
> MST
^ permalink raw reply
* Re: [PATCH RFC 1/4] crypto: engine - Permit to enqueue all async requests
From: Corentin Labbe @ 2017-12-07 9:24 UTC (permalink / raw)
To: Fabien DESSENNE
Cc: herbert@gondor.apana.org.au, mst@redhat.com,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org,
linux-crypto@vger.kernel.org, mcoquelin.stm32@gmail.com,
davem@davemloft.net, linux-arm-kernel@lists.infradead.org,
Alexandre TORGUE
In-Reply-To: <091ff463-9958-7f96-7ebb-87b36d63cce4@st.com>
On Wed, Dec 06, 2017 at 11:02:23AM +0000, Fabien DESSENNE wrote:
>
>
> On 29/11/17 09:41, Corentin Labbe wrote:
> > The crypto engine could actually only enqueue hash and ablkcipher request.
> > This patch permit it to enqueue any type of crypto_async_request.
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> > crypto/crypto_engine.c | 188 +++++++++++-------------------------------------
> > include/crypto/engine.h | 46 +++++-------
> > 2 files changed, 60 insertions(+), 174 deletions(-)
> >
> > diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
> > index 61e7c4e02fd2..f7c4c4c1f41b 100644
> > --- a/crypto/crypto_engine.c
> > +++ b/crypto/crypto_engine.c
> > @@ -34,11 +34,10 @@ static void crypto_pump_requests(struct crypto_engine *engine,
> > bool in_kthread)
> > {
> > struct crypto_async_request *async_req, *backlog;
> > - struct ahash_request *hreq;
> > - struct ablkcipher_request *breq;
> > unsigned long flags;
> > bool was_busy = false;
> > - int ret, rtype;
> > + int ret;
> > + struct crypto_engine_reqctx *enginectx;
> >
> > spin_lock_irqsave(&engine->queue_lock, flags);
> >
> > @@ -94,7 +93,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
> >
> > spin_unlock_irqrestore(&engine->queue_lock, flags);
> >
> > - rtype = crypto_tfm_alg_type(engine->cur_req->tfm);
> > /* Until here we get the request need to be encrypted successfully */
> > if (!was_busy && engine->prepare_crypt_hardware) {
> > ret = engine->prepare_crypt_hardware(engine);
> > @@ -104,57 +102,31 @@ static void crypto_pump_requests(struct crypto_engine *engine,
> > }
> > }
> >
> > - switch (rtype) {
> > - case CRYPTO_ALG_TYPE_AHASH:
> > - hreq = ahash_request_cast(engine->cur_req);
> > - if (engine->prepare_hash_request) {
> > - ret = engine->prepare_hash_request(engine, hreq);
> > - if (ret) {
> > - dev_err(engine->dev, "failed to prepare request: %d\n",
> > - ret);
> > - goto req_err;
> > - }
> > - engine->cur_req_prepared = true;
> > - }
> > - ret = engine->hash_one_request(engine, hreq);
> > - if (ret) {
> > - dev_err(engine->dev, "failed to hash one request from queue\n");
> > - goto req_err;
> > - }
> > - return;
> > - case CRYPTO_ALG_TYPE_ABLKCIPHER:
> > - breq = ablkcipher_request_cast(engine->cur_req);
> > - if (engine->prepare_cipher_request) {
> > - ret = engine->prepare_cipher_request(engine, breq);
> > - if (ret) {
> > - dev_err(engine->dev, "failed to prepare request: %d\n",
> > - ret);
> > - goto req_err;
> > - }
> > - engine->cur_req_prepared = true;
> > - }
> > - ret = engine->cipher_one_request(engine, breq);
> > + enginectx = crypto_tfm_ctx(async_req->tfm);
> > +
> > + if (enginectx->op.prepare_request) {
> > + ret = enginectx->op.prepare_request(engine, async_req);
> > if (ret) {
> > - dev_err(engine->dev, "failed to cipher one request from queue\n");
> > + dev_err(engine->dev, "failed to prepare request: %d\n",
> > + ret);
> > goto req_err;
> > }
> > - return;
> > - default:
> > - dev_err(engine->dev, "failed to prepare request of unknown type\n");
> > - return;
> > + engine->cur_req_prepared = true;
> > + }
> > + if (!enginectx->op.do_one_request) {
> > + dev_err(engine->dev, "failed to do request\n");
> > + ret = -EINVAL;
> > + goto req_err;
> > + }
> > + ret = enginectx->op.do_one_request(engine, async_req);
> > + if (ret) {
> > + dev_err(engine->dev, "failed to hash one request from queue\n");
> > + goto req_err;
> > }
> > + return;
> >
> > req_err:
> > - switch (rtype) {
> > - case CRYPTO_ALG_TYPE_AHASH:
> > - hreq = ahash_request_cast(engine->cur_req);
> > - crypto_finalize_hash_request(engine, hreq, ret);
> > - break;
> > - case CRYPTO_ALG_TYPE_ABLKCIPHER:
> > - breq = ablkcipher_request_cast(engine->cur_req);
> > - crypto_finalize_cipher_request(engine, breq, ret);
> > - break;
> > - }
> > + crypto_finalize_request(engine, async_req, ret);
> > return;
> >
> > out:
> > @@ -170,59 +142,16 @@ static void crypto_pump_work(struct kthread_work *work)
> > }
> >
> > /**
> > - * crypto_transfer_cipher_request - transfer the new request into the
> > - * enginequeue
> > + * crypto_transfer_request - transfer the new request into the engine queue
> > * @engine: the hardware engine
> > * @req: the request need to be listed into the engine queue
> > */
> > -int crypto_transfer_cipher_request(struct crypto_engine *engine,
> > - struct ablkcipher_request *req,
> > - bool need_pump)
> > +int crypto_transfer_request(struct crypto_engine *engine,
> > + struct crypto_async_request *req, bool need_pump)
> > {
> > unsigned long flags;
> > int ret;
> >
> > - spin_lock_irqsave(&engine->queue_lock, flags);
> > -
> > - if (!engine->running) {
> > - spin_unlock_irqrestore(&engine->queue_lock, flags);
> > - return -ESHUTDOWN;
> > - }
> > -
> > - ret = ablkcipher_enqueue_request(&engine->queue, req);
> > -
> > - if (!engine->busy && need_pump)
> > - kthread_queue_work(engine->kworker, &engine->pump_requests);
> > -
> > - spin_unlock_irqrestore(&engine->queue_lock, flags);
> > - return ret;
> > -}
> > -EXPORT_SYMBOL_GPL(crypto_transfer_cipher_request);
> > -
> > -/**
> > - * crypto_transfer_cipher_request_to_engine - transfer one request to list
> > - * into the engine queue
> > - * @engine: the hardware engine
> > - * @req: the request need to be listed into the engine queue
> > - */
> > -int crypto_transfer_cipher_request_to_engine(struct crypto_engine *engine,
> > - struct ablkcipher_request *req)
> > -{
> > - return crypto_transfer_cipher_request(engine, req, true);
> > -}
> > -EXPORT_SYMBOL_GPL(crypto_transfer_cipher_request_to_engine);
> > -
> > -/**
> > - * crypto_transfer_hash_request - transfer the new request into the
> > - * enginequeue
> > - * @engine: the hardware engine
> > - * @req: the request need to be listed into the engine queue
> > - */
> > -int crypto_transfer_hash_request(struct crypto_engine *engine,
> > - struct ahash_request *req, bool need_pump)
> > -{
> > - unsigned long flags;
> > - int ret;
> >
> > spin_lock_irqsave(&engine->queue_lock, flags);
> >
> > @@ -231,7 +160,7 @@ int crypto_transfer_hash_request(struct crypto_engine *engine,
> > return -ESHUTDOWN;
> > }
> >
> > - ret = ahash_enqueue_request(&engine->queue, req);
> > + ret = crypto_enqueue_request(&engine->queue, req);
> >
> > if (!engine->busy && need_pump)
> > kthread_queue_work(engine->kworker, &engine->pump_requests);
> > @@ -239,80 +168,45 @@ int crypto_transfer_hash_request(struct crypto_engine *engine,
> > spin_unlock_irqrestore(&engine->queue_lock, flags);
> > return ret;
> > }
> > -EXPORT_SYMBOL_GPL(crypto_transfer_hash_request);
> > +EXPORT_SYMBOL_GPL(crypto_transfer_request);
> >
> > /**
> > - * crypto_transfer_hash_request_to_engine - transfer one request to list
> > + * crypto_transfer_request_to_engine - transfer one request to list
> > * into the engine queue
> > * @engine: the hardware engine
> > * @req: the request need to be listed into the engine queue
> > */
> > -int crypto_transfer_hash_request_to_engine(struct crypto_engine *engine,
> > - struct ahash_request *req)
> > -{
> > - return crypto_transfer_hash_request(engine, req, true);
> > -}
> > -EXPORT_SYMBOL_GPL(crypto_transfer_hash_request_to_engine);
> > -
> > -/**
> > - * crypto_finalize_cipher_request - finalize one request if the request is done
> > - * @engine: the hardware engine
> > - * @req: the request need to be finalized
> > - * @err: error number
> > - */
> > -void crypto_finalize_cipher_request(struct crypto_engine *engine,
> > - struct ablkcipher_request *req, int err)
> > +int crypto_transfer_request_to_engine(struct crypto_engine *engine,
> > + struct crypto_async_request *req)
> > {
> > - unsigned long flags;
> > - bool finalize_cur_req = false;
> > - int ret;
> > -
> > - spin_lock_irqsave(&engine->queue_lock, flags);
> > - if (engine->cur_req == &req->base)
> > - finalize_cur_req = true;
> > - spin_unlock_irqrestore(&engine->queue_lock, flags);
> > -
> > - if (finalize_cur_req) {
> > - if (engine->cur_req_prepared &&
> > - engine->unprepare_cipher_request) {
> > - ret = engine->unprepare_cipher_request(engine, req);
> > - if (ret)
> > - dev_err(engine->dev, "failed to unprepare request\n");
> > - }
> > - spin_lock_irqsave(&engine->queue_lock, flags);
> > - engine->cur_req = NULL;
> > - engine->cur_req_prepared = false;
> > - spin_unlock_irqrestore(&engine->queue_lock, flags);
> > - }
> > -
> > - req->base.complete(&req->base, err);
> > -
> > - kthread_queue_work(engine->kworker, &engine->pump_requests);
> > + return crypto_transfer_request(engine, req, true);
> > }
> > -EXPORT_SYMBOL_GPL(crypto_finalize_cipher_request);
> > +EXPORT_SYMBOL_GPL(crypto_transfer_request_to_engine);
> >
> > /**
> > - * crypto_finalize_hash_request - finalize one request if the request is done
> > + * crypto_finalize_request - finalize one request if the request is done
> > * @engine: the hardware engine
> > * @req: the request need to be finalized
> > * @err: error number
> > */
> > -void crypto_finalize_hash_request(struct crypto_engine *engine,
> > - struct ahash_request *req, int err)
> > +void crypto_finalize_request(struct crypto_engine *engine,
> > + struct crypto_async_request *req, int err)
> > {
> > unsigned long flags;
> > bool finalize_cur_req = false;
> > int ret;
> > + struct crypto_engine_reqctx *enginectx;
> >
> > spin_lock_irqsave(&engine->queue_lock, flags);
> > - if (engine->cur_req == &req->base)
> > + if (engine->cur_req == req)
> > finalize_cur_req = true;
> > spin_unlock_irqrestore(&engine->queue_lock, flags);
> >
> > if (finalize_cur_req) {
> > + enginectx = crypto_tfm_ctx(req->tfm);
> > if (engine->cur_req_prepared &&
> > - engine->unprepare_hash_request) {
> > - ret = engine->unprepare_hash_request(engine, req);
> > + enginectx->op.unprepare_request) {
> > + ret = enginectx->op.unprepare_request(engine, req);
> > if (ret)
> > dev_err(engine->dev, "failed to unprepare request\n");
> > }
> > @@ -322,11 +216,11 @@ void crypto_finalize_hash_request(struct crypto_engine *engine,
> > spin_unlock_irqrestore(&engine->queue_lock, flags);
> > }
> >
> > - req->base.complete(&req->base, err);
> > + req->complete(req, err);
> >
> > kthread_queue_work(engine->kworker, &engine->pump_requests);
> > }
> > -EXPORT_SYMBOL_GPL(crypto_finalize_hash_request);
> > +EXPORT_SYMBOL_GPL(crypto_finalize_request);
> >
> > /**
> > * crypto_engine_start - start the hardware engine
> > diff --git a/include/crypto/engine.h b/include/crypto/engine.h
> > index dd04c1699b51..2e45db45849b 100644
> > --- a/include/crypto/engine.h
> > +++ b/include/crypto/engine.h
> > @@ -17,7 +17,6 @@
> > #include <linux/kernel.h>
> > #include <linux/kthread.h>
> > #include <crypto/algapi.h>
> > -#include <crypto/hash.h>
> >
> > #define ENGINE_NAME_LEN 30
> > /*
> > @@ -65,19 +64,6 @@ struct crypto_engine {
>
> You also need to remove these 6 functions from the comment header of
> that structure
>
Thanks, fixed for next version.
Regards
^ permalink raw reply
* Re: [PATCH RFC 0/4] crypto: engine - Permit to enqueue all async requests
From: Corentin Labbe @ 2017-12-07 9:37 UTC (permalink / raw)
To: Fabien DESSENNE
Cc: Lionel DEBIEVE, herbert@gondor.apana.org.au, mst@redhat.com,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org,
linux-crypto@vger.kernel.org, mcoquelin.stm32@gmail.com,
Benjamin GAIGNARD, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org, Alexandre TORGUE
In-Reply-To: <4d58a1fc-8577-3fdb-ae6b-e1fade7c6582@st.com>
On Wed, Dec 06, 2017 at 10:59:47AM +0000, Fabien DESSENNE wrote:
> Hi Corentin,
>
>
> I am fine with this proposal: it is generic enough and I have been able
> to test and run the crypto engine with aead_request without changing any
> single line of code.
>
> This is what I need to be able to send the AEAD extension of the
> stm32-cryp driver (successfully tested with your engine upgrade proposal).
>
>
> I have also tested the stm32-hash patch.
>
> Note that stm32-cryp (new driver applied by Herbert recently
> [https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/commit/?id=9e054ec21ef8344345b28603fb272fe999f735db])
> would also need to be converted to the new crypto engine API : this is a
> trivial patch.
Yes, patch for converting it is already done.
>
> Thank you for your proposal, I hope that this proposal is fine for
> Herbert too.
>
Thanks for your test, I hope other maintainer will test it also.
Regards
Corentin Labbe
^ permalink raw reply
* Re: [PATCH v3 1/2] virtio_mmio: add cleanup for virtio_mmio_probe
From: Cornelia Huck @ 2017-12-07 11:08 UTC (permalink / raw)
To: weiping zhang; +Cc: virtualization, mst
In-Reply-To: <9fc4a1dd52c99edfccdf4c710445ed9a43aad66a.1512568542.git.zhangweiping@didichuxing.com>
On Wed, 6 Dec 2017 21:59:16 +0800
weiping zhang <zhangweiping@didichuxing.com> wrote:
> As mentioned at drivers/base/core.c:
> /*
> * NOTE: _Never_ directly free @dev after calling this function, even
> * if it returned an error! Always use put_device() to give up the
> * reference initialized in this function instead.
> */
> so we don't free vm_dev until vm_dev.dev.release be called.
>
> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
> ---
> drivers/virtio/virtio_mmio.c | 51 +++++++++++++++++++++++++++++++++++---------
> 1 file changed, 41 insertions(+), 10 deletions(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply
* Re: [PATCH v3 2/2] virtio_mmio: add cleanup for virtio_mmio_remove
From: Cornelia Huck @ 2017-12-07 11:09 UTC (permalink / raw)
To: weiping zhang; +Cc: virtualization, mst
In-Reply-To: <1a62b93b539bcde52f629314f04193928f02693e.1512568542.git.zhangweiping@didichuxing.com>
On Wed, 6 Dec 2017 21:59:32 +0800
weiping zhang <zhangweiping@didichuxing.com> wrote:
> cleanup all resource allocated by virtio_mmio_probe.
>
> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
> ---
> drivers/virtio/virtio_mmio.c | 6 ++++++
> 1 file changed, 6 insertions(+)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply
* Re: [PATCH v18 05/10] xbitmap: add more operations
From: Wei Wang @ 2017-12-07 12:01 UTC (permalink / raw)
To: Tetsuo Handa, willy
Cc: aarcange, virtio-dev, riel, nilal, kvm, mst, qemu-devel,
amit.shah, liliang.opensource, mawilcox, linux-kernel,
virtualization, linux-mm, yang.zhang.wz, quan.xu, cornelia.huck,
pbonzini, akpm, mhocko, mgorman
In-Reply-To: <201712031050.IAC64520.QVLFFOOJOSFtHM@I-love.SAKURA.ne.jp>
On 12/03/2017 09:50 AM, Tetsuo Handa wrote:
> Matthew Wilcox wrote:
>> On Fri, Dec 01, 2017 at 03:09:08PM +0000, Wang, Wei W wrote:
>>> On Friday, December 1, 2017 9:02 PM, Tetsuo Handa wrote:
>>>> If start == end is legal,
>>>>
>>>> for (; start < end; start = (start | (IDA_BITMAP_BITS - 1)) + 1) {
>>>>
>>>> makes this loop do nothing because 10 < 10 is false.
>>> How about "start <= end "?
>> Don't ask Tetsuo for his opinion, write some userspace code that uses it.
>>
> Please be sure to prepare for "end == -1UL" case, for "start < end" will become
> true when "start = (start | (IDA_BITMAP_BITS - 1)) + 1" made "start == 0" due to
> overflow.
I think there is one more corner case with this API: searching for bit
"1" from [0, ULONG_MAX] while no bit is set in the range, there appear
to be no possible value that we can return (returning "end + 1" will be
"ULONG_MAX + 1", which is 0)
I plan to make the "end" be exclusive of the searching, that is, [start,
end), and return "end" if no such bit is found.
For cases like [16, 16), returning 16 doesn't mean bit 16 is 1 or 0, it
simply means there is no bits to search in the given range, since 16 is
exclusive.
Please let me know if you have a different thought.
Best,
Wei
^ permalink raw reply
* [PATCH 0/1] suspend/resume for virtio_ccw
From: Christian Borntraeger @ 2017-12-07 14:11 UTC (permalink / raw)
To: Halil Pasic, Cornelia Huck; +Cc: linux-s390, Thomas Huth, kvm, virtualization
With this patch I can suspend/resume a KVM guest.
[root@test ~]# cd /sys/power/
[root@test power]# echo test_resume > disk
[root@test power]# lscss
Device Subchan. DevType CU Type Use PIM PAM POM CHPIDs
----------------------------------------------------------------------
0.0.0003 0.0.0000 0000/00 3832/03 yes 80 80 ff 00000000 00000000
0.0.0000 0.0.0001 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0001 0.0.0002 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0004 0.0.0003 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0005 0.0.0004 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0006 0.0.0005 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0007 0.0.0006 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0008 0.0.0007 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0009 0.0.0008 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.000a 0.0.0009 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.000b 0.0.000a 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.000c 0.0.000b 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.000e 0.0.000c 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.000f 0.0.000d 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0010 0.0.000e 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0011 0.0.000f 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0012 0.0.0010 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0013 0.0.0011 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0014 0.0.0012 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0002 0.0.0013 0000/00 3832/09 yes 80 80 ff 00000000 00000000
0.0.000d 0.0.0014 0000/00 3832/01 yes 80 80 ff 00000000 00000000
0.0.0015 0.0.0015 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0016 0.0.0016 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0017 0.0.0017 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0018 0.0.0018 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0019 0.0.0019 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.3.ffba 0.3.0000 0000/00 3832/05 yes 80 80 ff 00000000 00000000
[root@test power]# echo disk > state
[ 195.854629] PM: hibernation entry
[ 195.854780] PM: Syncing filesystems ...
[ 195.855837] PM: done.
[ 195.855839] Freezing user space processes ... (elapsed 0.001 seconds) done.
[ 195.856927] OOM killer disabled.
[ 195.856941] PM: Preallocating image memory... done (allocated 182918 pages)
[ 196.229494] PM: Allocated 731672 kbytes in 0.37 seconds (1977.49 MB/s)
[ 196.229495] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done.
[ 196.231772] Suspending console(s) (use no_console_suspend to debug)
[ 196.238620] Disabling non-boot CPUs ...
[ 196.238632] PM: Creating hibernation image:
[ 196.267113] PM: Need to copy 182169 pages
[ 196.892074] PM: Hibernation image created (182169 pages copied)
[ 196.905127] PM: Using 1 thread(s) for compression.
[ 196.905127] PM: Compressing and saving image data (182525 pages)...
[ 197.358550] PM: Image saving progress: 0%
[ 197.595283] PM: Image saving progress: 10%
[ 197.740148] PM: Image saving progress: 20%
[ 197.859172] PM: Image saving progress: 30%
[ 197.989838] PM: Image saving progress: 40%
[ 198.100692] PM: Image saving progress: 50%
[ 198.205154] PM: Image saving progress: 60%
[ 198.308987] PM: Image saving progress: 70%
[ 198.412584] PM: Image saving progress: 80%
[ 198.516285] PM: Image saving progress: 90%
[ 198.620025] PM: Image saving progress: 100%
[ 198.620176] PM: Image saving done.
[ 198.620178] PM: Wrote 730100 kbytes in 1.71 seconds (426.95 MB/s)
[ 198.620256] PM: S
[ 198.620323] |
[ 198.650677] PM: Using 1 thread(s) for decompression.
[ 198.650677] PM: Loading and decompressing image data (182525 pages)...
[ 198.663329] PM: Image loading progress: 0%
[ 198.990431] PM: Image loading progress: 10%
[ 199.155921] PM: Image loading progress: 20%
[ 199.317973] PM: Image loading progress: 30%
[ 199.457977] PM: Image loading progress: 40%
[ 199.614798] PM: Image loading progress: 50%
[ 199.843670] PM: Image loading progress: 60%
[ 200.075105] PM: Image loading progress: 70%
[ 200.307333] PM: Image loading progress: 80%
[ 200.540000] PM: Image loading progress: 90%
[ 195.856941] PM: Preallocating image memory... done (allocated 182918 pages)
[ 196.229494] PM: Allocated 731672 kbytes in 0.37 seconds (1977.49 MB/s)
[ 196.229495] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done.
[ 196.231772] Suspending console(s) (use no_console_suspend to debug)
[ 196.238620] Disabling non-boot CPUs ...
[ 196.238632] PM: Creating hibernation image:
[ 196.267113] PM: Need to copy 182169 pages
[ 201.314326] OOM killer enabled.
[ 201.314327] Restarting tasks ... done.
[ 201.315976] PM: hibernation exit
[root@test power]# lscss
Device Subchan. DevType CU Type Use PIM PAM POM CHPIDs
----------------------------------------------------------------------
0.0.0003 0.0.0000 0000/00 3832/03 yes 80 80 ff 00000000 00000000
0.0.0000 0.0.0001 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0001 0.0.0002 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0004 0.0.0003 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0005 0.0.0004 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0006 0.0.0005 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0007 0.0.0006 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0008 0.0.0007 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0009 0.0.0008 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.000a 0.0.0009 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.000b 0.0.000a 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.000c 0.0.000b 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.000e 0.0.000c 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.000f 0.0.000d 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0010 0.0.000e 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0011 0.0.000f 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0012 0.0.0010 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0013 0.0.0011 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0014 0.0.0012 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0002 0.0.0013 0000/00 3832/09 yes 80 80 ff 00000000 00000000
0.0.000d 0.0.0014 0000/00 3832/01 yes 80 80 ff 00000000 00000000
0.0.0015 0.0.0015 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0016 0.0.0016 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0017 0.0.0017 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0018 0.0.0018 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.0.0019 0.0.0019 0000/00 3832/02 yes 80 80 ff 00000000 00000000
0.3.ffba 0.3.0000 0000/00 3832/05 yes 80 80 ff 00000000 00000000
[root@test power]#
Christian Borntraeger (1):
virtio/s390: implement PM operations for virtio_ccw
drivers/s390/virtio/virtio_ccw.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
--
2.9.4
^ permalink raw reply
* [PATCH 1/1] virtio/s390: implement PM operations for virtio_ccw
From: Christian Borntraeger @ 2017-12-07 14:11 UTC (permalink / raw)
To: Halil Pasic, Cornelia Huck; +Cc: linux-s390, Thomas Huth, kvm, virtualization
In-Reply-To: <20171207141102.70190-1-borntraeger@de.ibm.com>
Suspend/Resume to/from disk currently fails. Let us wire
up the necessary callbacks. This is mostly just forwarding
the requests to the virtio drivers. The only thing that
has to be done in virtio_ccw itself is to re-set the
virtio revision.
Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
drivers/s390/virtio/virtio_ccw.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index b18fe201..330b3fa 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -1300,6 +1300,9 @@ static int virtio_ccw_cio_notify(struct ccw_device *cdev, int event)
vcdev->device_lost = true;
rc = NOTIFY_DONE;
break;
+ case CIO_OPER:
+ rc = NOTIFY_OK;
+ break;
default:
rc = NOTIFY_DONE;
break;
@@ -1312,6 +1315,25 @@ static struct ccw_device_id virtio_ids[] = {
{},
};
+static int virtio_ccw_freeze(struct ccw_device *cdev)
+{
+ struct virtio_ccw_device *vcdev = dev_get_drvdata(&cdev->dev);
+
+ return virtio_device_freeze(&vcdev->vdev);
+}
+
+static int virtio_ccw_restore(struct ccw_device *cdev)
+{
+ struct virtio_ccw_device *vcdev = dev_get_drvdata(&cdev->dev);
+ int ret;
+
+ ret = virtio_ccw_set_transport_rev(vcdev);
+ if (ret)
+ return ret;
+
+ return virtio_device_restore(&vcdev->vdev);
+}
+
static struct ccw_driver virtio_ccw_driver = {
.driver = {
.owner = THIS_MODULE,
@@ -1324,6 +1346,9 @@ static struct ccw_driver virtio_ccw_driver = {
.set_online = virtio_ccw_online,
.notify = virtio_ccw_cio_notify,
.int_class = IRQIO_VIR,
+ .freeze = virtio_ccw_freeze,
+ .thaw = virtio_ccw_restore,
+ .restore = virtio_ccw_restore,
};
static int __init pure_hex(char **cp, unsigned int *val, int min_digit,
--
2.9.4
^ permalink raw reply related
* Re: [PATCH 1/1] virtio/s390: implement PM operations for virtio_ccw
From: Cornelia Huck @ 2017-12-07 14:32 UTC (permalink / raw)
To: Christian Borntraeger; +Cc: linux-s390, Thomas Huth, kvm, virtualization
In-Reply-To: <20171207141102.70190-2-borntraeger@de.ibm.com>
On Thu, 7 Dec 2017 15:11:02 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> Suspend/Resume to/from disk currently fails. Let us wire
> up the necessary callbacks. This is mostly just forwarding
> the requests to the virtio drivers. The only thing that
> has to be done in virtio_ccw itself is to re-set the
> virtio revision.
>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> drivers/s390/virtio/virtio_ccw.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
Less work than I had expected :)
This looks complete: The only thing we do in our online function not
directly triggered by registering the virtio device is setting the
revision.
Will apply, but would not mind some R-bs/acks.
^ permalink raw reply
* Re: [PATCH v18 05/10] xbitmap: add more operations
From: Michael S. Tsirkin @ 2017-12-07 15:41 UTC (permalink / raw)
To: Wei Wang
Cc: yang.zhang.wz, kvm, Tetsuo Handa, liliang.opensource, qemu-devel,
virtualization, linux-mm, aarcange, virtio-dev, mawilcox, willy,
quan.xu, nilal, riel, cornelia.huck, mhocko, linux-kernel,
amit.shah, pbonzini, akpm, mgorman
In-Reply-To: <5A292D94.5000700@intel.com>
On Thu, Dec 07, 2017 at 08:01:24PM +0800, Wei Wang wrote:
> On 12/03/2017 09:50 AM, Tetsuo Handa wrote:
> > Matthew Wilcox wrote:
> > > On Fri, Dec 01, 2017 at 03:09:08PM +0000, Wang, Wei W wrote:
> > > > On Friday, December 1, 2017 9:02 PM, Tetsuo Handa wrote:
> > > > > If start == end is legal,
> > > > >
> > > > > for (; start < end; start = (start | (IDA_BITMAP_BITS - 1)) + 1) {
> > > > >
> > > > > makes this loop do nothing because 10 < 10 is false.
> > > > How about "start <= end "?
> > > Don't ask Tetsuo for his opinion, write some userspace code that uses it.
> > >
> > Please be sure to prepare for "end == -1UL" case, for "start < end" will become
> > true when "start = (start | (IDA_BITMAP_BITS - 1)) + 1" made "start == 0" due to
> > overflow.
>
> I think there is one more corner case with this API: searching for bit "1"
> from [0, ULONG_MAX] while no bit is set in the range, there appear to be no
> possible value that we can return (returning "end + 1" will be "ULONG_MAX +
> 1", which is 0)
> I plan to make the "end" be exclusive of the searching, that is, [start,
> end), and return "end" if no such bit is found.
>
> For cases like [16, 16), returning 16 doesn't mean bit 16 is 1 or 0, it
> simply means there is no bits to search in the given range, since 16 is
> exclusive.
>
> Please let me know if you have a different thought.
>
> Best,
> Wei
Matthew is right though - you want to include tests for all
these corner cases.
^ permalink raw reply
* Re: [PATCH 1/1] virtio/s390: implement PM operations for virtio_ccw
From: David Hildenbrand @ 2017-12-07 15:48 UTC (permalink / raw)
To: Christian Borntraeger, Halil Pasic, Cornelia Huck
Cc: linux-s390, Thomas Huth, kvm, virtualization
In-Reply-To: <20171207141102.70190-2-borntraeger@de.ibm.com>
On 07.12.2017 15:11, Christian Borntraeger wrote:
> Suspend/Resume to/from disk currently fails. Let us wire
> up the necessary callbacks. This is mostly just forwarding
> the requests to the virtio drivers. The only thing that
> has to be done in virtio_ccw itself is to re-set the
> virtio revision.
>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> drivers/s390/virtio/virtio_ccw.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> index b18fe201..330b3fa 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -1300,6 +1300,9 @@ static int virtio_ccw_cio_notify(struct ccw_device *cdev, int event)
> vcdev->device_lost = true;
> rc = NOTIFY_DONE;
> break;
> + case CIO_OPER:
> + rc = NOTIFY_OK;
> + break;
> default:
> rc = NOTIFY_DONE;
> break;
> @@ -1312,6 +1315,25 @@ static struct ccw_device_id virtio_ids[] = {
> {},
> };
>
> +static int virtio_ccw_freeze(struct ccw_device *cdev)
> +{
> + struct virtio_ccw_device *vcdev = dev_get_drvdata(&cdev->dev);
> +
> + return virtio_device_freeze(&vcdev->vdev);
> +}
> +
> +static int virtio_ccw_restore(struct ccw_device *cdev)
> +{
> + struct virtio_ccw_device *vcdev = dev_get_drvdata(&cdev->dev);
> + int ret;
> +
> + ret = virtio_ccw_set_transport_rev(vcdev);
> + if (ret)
> + return ret;
> +
> + return virtio_device_restore(&vcdev->vdev);
> +}
> +
> static struct ccw_driver virtio_ccw_driver = {
> .driver = {
> .owner = THIS_MODULE,
> @@ -1324,6 +1346,9 @@ static struct ccw_driver virtio_ccw_driver = {
> .set_online = virtio_ccw_online,
> .notify = virtio_ccw_cio_notify,
> .int_class = IRQIO_VIR,
> + .freeze = virtio_ccw_freeze,
> + .thaw = virtio_ccw_restore,
> + .restore = virtio_ccw_restore,
> };
>
> static int __init pure_hex(char **cp, unsigned int *val, int min_digit,
>
Really neat, hope we haven't forgotten to reinitialize anything. (guess
if it works, we should be 99.9% fine)
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [RFC] virtio-net: help live migrate SR-IOV devices
From: Alexander Duyck @ 2017-12-07 16:45 UTC (permalink / raw)
To: achiad shochat
Cc: Jakub Kicinski, Hannes Frederic Sowa, Sridhar Samudrala,
Michael S. Tsirkin, virtualization, Shannon Nelson, Achiad,
Peter Waskiewicz Jr, netdev, Singhai, Anjali, Andy Gospodarek,
Or Gerlitz
In-Reply-To: <CAEHy93JG-X-5qtrU2wUmxfMzKc9G3GDNL_H-FXsmAmfwvoO+vg@mail.gmail.com>
On Wed, Dec 6, 2017 at 11:28 PM, achiad shochat
<achiad.mellanox@gmail.com> wrote:
> On 5 December 2017 at 21:20, Michael S. Tsirkin <mst@redhat.com> wrote:
>> On Tue, Dec 05, 2017 at 11:59:17AM +0200, achiad shochat wrote:
>>> Then we'll have a single solution for both netvsc and virtio (and any
>>> other PV device).
>>> And we could handle the VF DMA dirt issue agnostically.
>>
>> For the record, I won't block patches adding this kist to virtio
>> on the basis that they must be generic. It's not a lot
>> of code, implementation can come first, prettify later.
>
> It's not a lot of code either way.
> So I fail to understand why not to do it right from the beginning.
> For the record...
What isn't a lot of code? If you are talking about the DMA dirtying
then I would have to disagree. The big problem with the DMA is that we
have to mark a page as dirty and non-migratable as soon as it is
mapped for Rx DMA. It isn't until the driver has either unmapped the
page or the device has been disabled that we can then allow the page
to be migrated for being dirty. That ends up being the way we have to
support this if we don't have the bonding solution.
With the bonding solution we could look at doing a lightweight DMA
dirtying which would just require flagging pages as dirty after an
unmap or sync call is performed. However it requires that we shut down
the driver/device before we can complete the migration which means we
have to have the paravirtualized fail-over approach.
As far as indicating that the interfaces are meant to be enslaved I
wonder if we couldn't look at tweaking the PCI layout of the guest and
use that to indicate that a given set of interfaces are meant to be
bonded. For example the VFs are all meant to work as a part of a
multi-function device. What if we were to make virtio-net function 0
of a PCI/PCIe device, and then place any direct assigned VFs that are
meant to be a part of the bond in functions 1-7 of the device? Then it
isn't too far off from the model we have on the host where if the VF
goes away we would expect to see the traffic on the PF that is usually
occupying function 0 of a given device.
^ permalink raw reply
* Re: [RFC] virtio-net: help live migrate SR-IOV devices
From: Michael S. Tsirkin @ 2017-12-07 16:53 UTC (permalink / raw)
To: Alexander Duyck
Cc: Jakub Kicinski, Hannes Frederic Sowa, achiad shochat,
Sridhar Samudrala, virtualization, Shannon Nelson, Achiad,
Peter Waskiewicz Jr, netdev, Singhai, Anjali, Andy Gospodarek,
Or Gerlitz
In-Reply-To: <CAKgT0UfC2o4P8DeRHnnPV9fvUzJYbkCKVHr=zHOfbEQdLn=3nA@mail.gmail.com>
On Thu, Dec 07, 2017 at 08:45:33AM -0800, Alexander Duyck wrote:
> As far as indicating that the interfaces are meant to be enslaved I
> wonder if we couldn't look at tweaking the PCI layout of the guest and
> use that to indicate that a given set of interfaces are meant to be
> bonded. For example the VFs are all meant to work as a part of a
> multi-function device. What if we were to make virtio-net function 0
> of a PCI/PCIe device, and then place any direct assigned VFs that are
> meant to be a part of the bond in functions 1-7 of the device? Then it
> isn't too far off from the model we have on the host where if the VF
> goes away we would expect to see the traffic on the PF that is usually
> occupying function 0 of a given device.
This pretty much precludes removing them with hotplug.
But as long as we are happy to limit this to pci devices,
maybe we should put them behind a pci bridge.
All devices behind a bridge would be assumed to have
the same backend.
QEMU has pci-bridge-seat which we could reuse for this -
just need to build a similar pci express bridge.
--
MST
^ permalink raw reply
* CISTI'2018 -- Doctoral Symposium -- Call for contributions
From: ML @ 2017-12-08 11:57 UTC (permalink / raw)
To: virtualization
[-- Attachment #1: Type: text/plain, Size: 2340 bytes --]
------------------------------------------------------------------
Doctoral Symposium of CISTI'2018
13th Iberian Conference on Information Systems and Technologies
13 - 16 June 2018, Caceres, Spain
http://www.cisti.eu/
---------------------------------------------------------------------------
The purpose of CISTI'2018’s Doctoral Symposium is to provide graduate students a setting where they can, informally, expose and discuss their work, collecting valuable expert opinions and sharing new ideas, methods and applications. The Doctoral Symposium is an excellent opportunity for PhD students to present and discuss their work in a Workshop format. Each presentation will be evaluated by a panel composed by at least three Information Systems and Technologies experts.
CONTRIBUTIONS AND SUBMISSION
The Doctoral Symposium is opened to PhD students whose research area includes the themes proposed for this Conference. Submissions must include an extended abstract (maximum 4 pages), following the Conference style guide. All selected contributions will be handed out along with the Conference Proceedings, in CD with an ISBN. These contributions will be available in the IEEE Xplore Digital Library and will be sent for indexing in ISI, Scopus, EI-Compendex, INSPEC and Google Scholar.
Submissions must include the field, the PhD institution and the number of months devoted to the development of the work. Additionally, they should include in a clear and succinct manner:
• The problem approached and its significance or relevance
• The research objectives and related investigation topics
• A brief display of what is already known
• A proposed solution methodology for the problem
• Expected results
IMPORTANT DATES
Paper submission: February 4, 2018
Notification of acceptance: March 18, 2018
Submission of accepted papers: March 30, 2018
Payment of registration, to ensure the inclusion of an accepted paper in the conference proceedings: April 1, 2018
Webpage of the CISTI'2018 Doctoral Symposium: http://cisti.eu/index.php?option=com_content&view=article&id=35&Itemid=119&lang=en
-
Regards,
CISTI'2018 Team
http://www.cisti.eu/
-----------------------
---
This email has been checked for viruses by AVG.
http://www.avg.com
[-- 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
* [PULL] vhost: cleanups and fixes
From: Michael S. Tsirkin @ 2017-12-08 15:47 UTC (permalink / raw)
To: Linus Torvalds
Cc: kvm, mst, netdev, wangyunjian, linux-kernel, virtualization,
zhangweiping
The following changes since commit d9e427f6ab8142d6868eb719e6a7851aafea56b6:
virtio_balloon: fix increment of vb->num_pfns in fill_balloon() (2017-12-01 16:55:45 +0200)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
for you to fetch changes up to 03e9f8a05bce7330bcd9c5cc54c8e42d0fcbf993:
virtio_net: fix return value check in receive_mergeable() (2017-12-07 18:34:52 +0200)
----------------------------------------------------------------
virtio: bugfixes
A couple of minor bugfixes.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
----------------------------------------------------------------
Yunjian Wang (1):
virtio_net: fix return value check in receive_mergeable()
weiping zhang (2):
virtio_mmio: add cleanup for virtio_mmio_probe
virtio_mmio: add cleanup for virtio_mmio_remove
drivers/net/virtio_net.c | 2 +-
drivers/virtio/virtio_mmio.c | 57 ++++++++++++++++++++++++++++++++++++--------
2 files changed, 48 insertions(+), 11 deletions(-)
^ permalink raw reply
* Re: [PATCH] ptr_ring: add barriers
From: David Miller @ 2017-12-08 18:08 UTC (permalink / raw)
To: mst; +Cc: george.cherian, netdev, linux-kernel, virtualization, edumazet
In-Reply-To: <1512501990-30029-1-git-send-email-mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Tue, 5 Dec 2017 21:29:37 +0200
> Users of ptr_ring expect that it's safe to give the
> data structure a pointer and have it be available
> to consumers, but that actually requires an smb_wmb
> or a stronger barrier.
>
> In absence of such barriers and on architectures that reorder writes,
> consumer might read an un=initialized value from an skb pointer stored
> in the skb array. This was observed causing crashes.
>
> To fix, add memory barriers. The barrier we use is a wmb, the
> assumption being that producers do not need to read the value so we do
> not need to order these reads.
>
> Reported-by: George Cherian <george.cherian@cavium.com>
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> George, could you pls report whether this patch fixes
> the issue for you?
>
> This seems to be needed in stable as well.
I really need some testing feedback for this before I apply it
and queue it up for -stable.
George?
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: Disable interrupts if napi_complete_done rescheduled napi
From: David Miller @ 2017-12-08 18:19 UTC (permalink / raw)
To: jasowang; +Cc: netdev, edumazet, virtualization, mst
In-Reply-To: <ffcf1aa3-403e-9ff0-bd51-c507d5a52991@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Thu, 7 Dec 2017 15:08:45 +0800
>
>
> On 2017年12月07日 13:09, Michael S. Tsirkin wrote:
>> On Thu, Dec 07, 2017 at 01:15:15PM +0900, Toshiaki Makita wrote:
>>> Since commit 39e6c8208d7b ("net: solve a NAPI race") napi has been
>>> able
>>> to be rescheduled within napi_complete_done() even in non-busypoll
>>> case,
>>> but virtnet_poll() always enabled interrupts before complete, and when
>>> napi was rescheduled within napi_complete_done() it did not disable
>>> interrupts.
>>> This caused more interrupts when event idx is disabled.
>>>
>>> According to commit cbdadbbf0c79 ("virtio_net: fix race in RX VQ
>>> processing") we cannot place virtqueue_enable_cb_prepare() after
>>> NAPI_STATE_SCHED is cleared, so disable interrupts again if
>>> napi_complete_done() returned false.
>>>
>>> Tested with vhost-user of OVS 2.7 on host, which does not have the
>>> event
>>> idx feature.
...
>>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>>
>> it might make sense in net and not -next since tx napi regressed
>> performance
>> in some configs, this might bring it back at least partially.
>> Jason - what do you think?
>
> No sure, the regression I saw was tested with event idx on. And
> virtqueue_disable_cb() does almost nothing for event idx (or even a
> little bit slower).
>
> The patch looks good.
>
> Acked-by: Jason Wang <jasowang@redhat.com>
I'm going to put this into net-next for now.
^ permalink raw reply
* [PATCH] virtio: make VIRTIO a menuconfig to ease disabling it all
From: Vincent Legoll @ 2017-12-09 15:26 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: Vincent Legoll
No need to get into the submenu to disable all VIRTIO-related
config entries.
This makes it easier to disable all VIRTIO config options
without entering the submenu. It will also enable one
to see that en/dis-abled state from the outside menu.
This is only intended to change menuconfig UI, not change
the config dependencies.
Signed-off-by: Vincent Legoll <vincent.legoll@gmail.com>
---
drivers/virtio/Kconfig | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index cff773f15b7e..d485a63a8233 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -5,7 +5,10 @@ config VIRTIO
bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_RPMSG
or CONFIG_S390_GUEST.
-menu "Virtio drivers"
+menuconfig VIRTIO_MENU
+ bool "Virtio drivers"
+
+if VIRTIO_MENU
config VIRTIO_PCI
tristate "PCI driver for virtio devices"
@@ -79,4 +82,4 @@ config VIRTIO_MMIO_CMDLINE_DEVICES
If unsure, say 'N'.
-endmenu
+endif # VIRTIO_MENU
--
2.14.1
^ permalink raw reply related
* Re: [PATCH v18 10/10] virtio-balloon: don't report free pages when page poisoning is enabled
From: Wei Wang @ 2017-12-11 6:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: yang.zhang.wz, kvm, penguin-kernel, liliang.opensource,
qemu-devel, virtualization, linux-mm, aarcange, virtio-dev,
mawilcox, willy, quan.xu, nilal, riel, cornelia.huck, mhocko,
linux-kernel, amit.shah, pbonzini, akpm, mgorman
In-Reply-To: <20171201173951-mutt-send-email-mst@kernel.org>
On 12/01/2017 11:49 PM, Michael S. Tsirkin wrote:
> On Wed, Nov 29, 2017 at 09:55:26PM +0800, Wei Wang wrote:
>> The guest free pages should not be discarded by the live migration thread
>> when page poisoning is enabled with PAGE_POISONING_NO_SANITY=n, because
>> skipping the transfer of such poisoned free pages will trigger false
>> positive when new pages are allocated and checked on the destination.
>> This patch skips the reporting of free pages in the above case.
>>
>> Reported-by: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>> Cc: Michal Hocko <mhocko@suse.com>
>> ---
>> drivers/virtio/virtio_balloon.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
>> index 035bd3a..6ac4cff 100644
>> --- a/drivers/virtio/virtio_balloon.c
>> +++ b/drivers/virtio/virtio_balloon.c
>> @@ -652,7 +652,9 @@ static void report_free_page(struct work_struct *work)
>> /* Start by sending the obtained cmd id to the host with an outbuf */
>> send_one_desc(vb, vb->free_page_vq, virt_to_phys(&vb->start_cmd_id),
>> sizeof(uint32_t), false, true, false);
>> - walk_free_mem_block(vb, 0, &virtio_balloon_send_free_pages);
>> + if (!(page_poisoning_enabled() &&
>> + !IS_ENABLED(CONFIG_PAGE_POISONING_NO_SANITY)))
>> + walk_free_mem_block(vb, 0, &virtio_balloon_send_free_pages);
>> /*
>> * End by sending the stop id to the host with an outbuf. Use the
>> * non-batching mode here to trigger a kick after adding the stop id.
> PAGE_POISONING_ZERO is actually OK.
>
> But I really would prefer it that we still send pages to host,
> otherwise debugging becomes much harder.
>
> And it does not have to be completely useless, even though
> you can not discard them as they would be zero-filled then.
>
> How about a config field telling host what should be there in the free
> pages? This way even though host can not discard them, host can send
> them out without reading them, still a win.
>
>
Since this poison value comes with the free page reporting feature, how
about sending the poison value via the free_page_vq, along with the cmd
id in the outbuf? That is, use the following interface:
struct virtio_balloon_free_page_vq_hdr {
bool page_poisoning;
__virtio32 poison_value;
__virtio32 cmd_id;
}
We need "bool page_poisoning" because "poison_value=0" doesn't tell
whether page poising is in use by the guest. PAGE_POISONING_ZERO sets
"page_poisoning=true, poisoning_value=0", and the host will send the
0-filled pages to the destination (if not sending 0-filled pages, the
destination host would offer non-zero pages to the guest)
The host can discard free pages only when "page_poisoning=false".
Best,
Wei
^ 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