similar to: [PATCH 0/6] virtio: virtqueue ops cleanup

skb is only NULL the first time around: it's more correct to test for being under-budget. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> diff -r 2a94425ac7d5 drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c Thu Nov 15 13:47:28 2007 +1100 +++ b/drivers/net/virtio_net.c Thu Nov 15 23:10:44 2007 +1100 @@ -198,8 +198,8 @@ again: if (vi->num < vi->max / 2)

skb is only NULL the first time around: it's more correct to test for being under-budget. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> diff -r 2a94425ac7d5 drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c Thu Nov 15 13:47:28 2007 +1100 +++ b/drivers/net/virtio_net.c Thu Nov 15 23:10:44 2007 +1100 @@ -198,8 +198,8 @@ again: if (vi->num < vi->max / 2)

This helper returns 1 if a call to add_buf will not fail with -ENOSPC. This will help callers that do while(1) { alloc() if (add_buf()) { free(); break; } } This will result in one less alloc/free exercise. Signed-off-by: Amit Shah <amit.shah at redhat.com> --- drivers/virtio/virtio_ring.c | 8 ++++++++ include/linux/virtio.h | 5 +++++ 2 files changed, 13

This helper returns 1 if a call to add_buf will not fail with -ENOSPC. This will help callers that do while(1) { alloc() if (add_buf()) { free(); break; } } This will result in one less alloc/free exercise. Signed-off-by: Amit Shah <amit.shah at redhat.com> --- drivers/virtio/virtio_ring.c | 8 ++++++++ include/linux/virtio.h | 5 +++++ 2 files changed, 13

From: Mark McLoughlin <markmc at redhat.com> hdr->csum_start is the offset from the start of the ethernet header to the transport layer checksum field. skb->csum_start is the offset from skb->head. skb_partial_csum_set() assumes that skb->data points to the ethernet header - i.e. it computes skb->csum_start by adding the headroom to hdr->csum_start. Since

Rusty, Anthony, Dor, I need your brain power :-) On smp guests I have seen a problem with virtio (the version in curent Avi's git) which do not occur on single processor guests: kernel BUG at /space/kvm/drivers/virtio/virtio_ring.c:228! illegal operation: 0001 [#1] Modules linked in: ipv6 CPU: 2 Not tainted Process swapper (pid: 0, task: 000000000f83e038, ksp: 000000000f877d70) Krnl

Rusty, Anthony, Dor, I need your brain power :-) On smp guests I have seen a problem with virtio (the version in curent Avi's git) which do not occur on single processor guests: kernel BUG at /space/kvm/drivers/virtio/virtio_ring.c:228! illegal operation: 0001 [#1] Modules linked in: ipv6 CPU: 2 Not tainted Process swapper (pid: 0, task: 000000000f83e038, ksp: 000000000f877d70) Krnl

OK, here's a new attempt to use the new capacity api. I also added more comments to clarify the logic. Hope this is more readable. Let me know pls. This is on top of the patches applied by Rusty. Warning: untested. Posting now to give people chance to comment on the API. Changes from v1: - fix comment in patch 2 to correct confusion noted by Rusty - rewrite patch 3 along the lines

OK, here's a new attempt to use the new capacity api. I also added more comments to clarify the logic. Hope this is more readable. Let me know pls. This is on top of the patches applied by Rusty. Warning: untested. Posting now to give people chance to comment on the API. Changes from v1: - fix comment in patch 2 to correct confusion noted by Rusty - rewrite patch 3 along the lines

The virtio block device holds a lock during I/O request processing. Kicking the virtqueue while the lock is held results in long lock hold times and increases contention for the lock. This patch modifies virtqueue_kick() to optionally release a lock while notifying the host. Virtio block is modified to pass in its lock. This allows other vcpus to queue I/O requests during the time spent

The virtio block device holds a lock during I/O request processing. Kicking the virtqueue while the lock is held results in long lock hold times and increases contention for the lock. This patch modifies virtqueue_kick() to optionally release a lock while notifying the host. Virtio block is modified to pass in its lock. This allows other vcpus to queue I/O requests during the time spent

Since we want to reset the device to remove them, this is simpler (device is reset for us on driver remove). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- drivers/net/virtio_net.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff -r 7e5b3ff06f60 drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c Wed Jan 23 22:53:14

Since we want to reset the device to remove them, this is simpler (device is reset for us on driver remove). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- drivers/net/virtio_net.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff -r 7e5b3ff06f60 drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c Wed Jan 23 22:53:14

virtio net used to unlink skbs from send queues on error, but ever since 48925e372f04f5e35fec6269127c62b2c71ab794 we do not do this. This causes guest data corruption and crashes with vhost since net core can requeue the skb or free it without it being taken off the list. This patch fixes this by queueing the skb after successfull transmit. Signed-off-by: Michael S. Tsirkin <mst at

virtio net used to unlink skbs from send queues on error, but ever since 48925e372f04f5e35fec6269127c62b2c71ab794 we do not do this. This causes guest data corruption and crashes with vhost since net core can requeue the skb or free it without it being taken off the list. This patch fixes this by queueing the skb after successfull transmit. Signed-off-by: Michael S. Tsirkin <mst at

This helper returns 1 if a call to add_buf will not fail with -ENOSPC. This will help callers that do while(1) { alloc() if (add_buf()) { free(); break; } } This will result in one less alloc/free exercise. Signed-off-by: Amit Shah <amit.shah at redhat.com> --- v2: return true/false instead of 1/0 drivers/virtio/virtio_ring.c | 8 ++++++++ include/linux/virtio.h |

This helper returns 1 if a call to add_buf will not fail with -ENOSPC. This will help callers that do while(1) { alloc() if (add_buf()) { free(); break; } } This will result in one less alloc/free exercise. Signed-off-by: Amit Shah <amit.shah at redhat.com> --- v2: return true/false instead of 1/0 drivers/virtio/virtio_ring.c | 8 ++++++++ include/linux/virtio.h |

If we fail to transmit a packet, we assume the queue is full and put the skb into last_xmit_skb. However, if more space frees up before we xmit it, we loop, and the result can be transmitting the same skb twice. Fix is simple: set skb to NULL if we've used it in some way, and check before sending. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c |

If we fail to transmit a packet, we assume the queue is full and put the skb into last_xmit_skb. However, if more space frees up before we xmit it, we loop, and the result can be transmitting the same skb twice. Fix is simple: set skb to NULL if we've used it in some way, and check before sending. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c |

On Thursday 01 May 2008 00:31:46 Mark McLoughlin wrote: > virtio_net currently only frees old transmit skbs just > before queueing new ones. If the queue is full, it then > enables interrupts and waits for notification that more > work has been performed. Hi Mark, This patch is fine, but it's better to do it from skb_xmit_done(). Of course, this is usually called from an