* [PATCH 08/18] virtio_ring: support for used_event idx feature
From: Michael S. Tsirkin @ 2011-05-04 20:51 UTC (permalink / raw)
To: linux-kernel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>
Add support for the used_event idx feature: when enabling
interrupts, publish the current avail index value to
the host so that we get interrupts on the next update.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/virtio/virtio_ring.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 507d6eb..3a3ed75 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -320,6 +320,14 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
ret = vq->data[i];
detach_buf(vq, i);
vq->last_used_idx++;
+ /* 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->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) {
+ vring_used_event(&vq->vring) = vq->last_used_idx;
+ virtio_mb();
+ }
+
END_USE(vq);
return ret;
}
@@ -341,7 +349,11 @@ bool virtqueue_enable_cb(struct virtqueue *_vq)
/* We optimistically turn back on interrupts, then check if there was
* more to do. */
+ /* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
+ * either clear the flags bit or point the event index at the next
+ * entry. Always do both to keep code simple. */
vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT;
+ vring_used_event(&vq->vring) = vq->last_used_idx;
virtio_mb();
if (unlikely(more_used(vq))) {
END_USE(vq);
@@ -468,6 +480,8 @@ void vring_transport_features(struct virtio_device *vdev)
break;
case VIRTIO_RING_F_INDIRECT_DESC:
break;
+ case VIRTIO_RING_F_USED_EVENT_IDX:
+ break;
default:
/* We don't understand this bit. */
clear_bit(i, vdev->features);
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCH 09/18] virtio: use avail_event index
From: Michael S. Tsirkin @ 2011-05-04 20:51 UTC (permalink / raw)
To: linux-kernel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>
Use the new avail_event feature to reduce the number
of exits from the guest.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/virtio/virtio_ring.c | 39 ++++++++++++++++++++++++++++++++++++++-
1 files changed, 38 insertions(+), 1 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 3a3ed75..262dfe6 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -82,6 +82,15 @@ struct vring_virtqueue
/* Host supports indirect buffers */
bool indirect;
+ /* Host publishes avail event idx */
+ bool event;
+
+ /* Is kicked_avail below valid? */
+ bool kicked_avail_valid;
+
+ /* avail idx value we already kicked. */
+ u16 kicked_avail;
+
/* Number of free buffers */
unsigned int num_free;
/* Head of free buffer list. */
@@ -228,6 +237,12 @@ add_head:
* new available array entries. */
virtio_wmb();
vq->vring.avail->idx++;
+ /* If the driver never bothers to kick in a very long while,
+ * avail index might wrap around. If that happens, invalidate
+ * kicked_avail index we stored. TODO: make sure all drivers
+ * kick at least once in 2^16 and remove this. */
+ if (unlikely(vq->vring.avail->idx == vq->kicked_avail))
+ vq->kicked_avail_valid = true;
pr_debug("Added buffer head %i to %p\n", head, vq);
END_USE(vq);
@@ -236,6 +251,23 @@ add_head:
}
EXPORT_SYMBOL_GPL(virtqueue_add_buf_gfp);
+
+static bool vring_notify(struct vring_virtqueue *vq)
+{
+ u16 old, new;
+ bool v;
+ if (!vq->event)
+ return !(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY);
+
+ v = vq->kicked_avail_valid;
+ old = vq->kicked_avail;
+ new = vq->kicked_avail = vq->vring.avail->idx;
+ vq->kicked_avail_valid = true;
+ if (unlikely(!v))
+ return true;
+ return vring_need_event(vring_avail_event(&vq->vring), new, old);
+}
+
void virtqueue_kick(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
@@ -244,7 +276,7 @@ void virtqueue_kick(struct virtqueue *_vq)
/* Need to update avail index before checking if we should notify */
virtio_mb();
- if (!(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY))
+ if (vring_notify(vq))
/* Prod other side to tell it about changes. */
vq->notify(&vq->vq);
@@ -437,6 +469,8 @@ struct virtqueue *vring_new_virtqueue(unsigned int num,
vq->vq.name = name;
vq->notify = notify;
vq->broken = false;
+ vq->kicked_avail_valid = false;
+ vq->kicked_avail = 0;
vq->last_used_idx = 0;
list_add_tail(&vq->vq.list, &vdev->vqs);
#ifdef DEBUG
@@ -444,6 +478,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int num,
#endif
vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC);
+ vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_AVAIL_EVENT_IDX);
/* No callback? Tell other side not to bother us. */
if (!callback)
@@ -482,6 +517,8 @@ void vring_transport_features(struct virtio_device *vdev)
break;
case VIRTIO_RING_F_USED_EVENT_IDX:
break;
+ case VIRTIO_RING_F_AVAIL_EVENT_IDX:
+ break;
default:
/* We don't understand this bit. */
clear_bit(i, vdev->features);
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCH 10/18] vhost: utilize used_event index
From: Michael S. Tsirkin @ 2011-05-04 20:51 UTC (permalink / raw)
To: linux-kernel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>
Support the new used_event index. When acked,
utilize it to reduce the # of interrupts sent to the guest.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 74 +++++++++++++++++++++++++++++++++++++------------
drivers/vhost/vhost.h | 7 ++++
2 files changed, 63 insertions(+), 18 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 2ab2912..e33d5a3 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -37,6 +37,8 @@ enum {
VHOST_MEMORY_F_LOG = 0x1,
};
+#define vhost_used_event(vq) ((u16 __user *)&vq->avail->ring[vq->num])
+
static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
poll_table *pt)
{
@@ -161,6 +163,8 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->last_avail_idx = 0;
vq->avail_idx = 0;
vq->last_used_idx = 0;
+ vq->signalled_used = 0;
+ vq->signalled_used_valid = false;
vq->used_flags = 0;
vq->log_used = false;
vq->log_addr = -1ull;
@@ -489,14 +493,15 @@ static int memory_access_ok(struct vhost_dev *d, struct vhost_memory *mem,
return 1;
}
-static int vq_access_ok(unsigned int num,
+static int vq_access_ok(struct vhost_dev *d, unsigned int num,
struct vring_desc __user *desc,
struct vring_avail __user *avail,
struct vring_used __user *used)
{
+ size_t sa = vhost_has_feature(d, VIRTIO_RING_F_USED_EVENT_IDX) ? 2 : 0;
return access_ok(VERIFY_READ, desc, num * sizeof *desc) &&
access_ok(VERIFY_READ, avail,
- sizeof *avail + num * sizeof *avail->ring) &&
+ sizeof *avail + num * sizeof *avail->ring + sa) &&
access_ok(VERIFY_WRITE, used,
sizeof *used + num * sizeof *used->ring);
}
@@ -531,7 +536,7 @@ static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
/* Caller should have vq mutex and device mutex */
int vhost_vq_access_ok(struct vhost_virtqueue *vq)
{
- return vq_access_ok(vq->num, vq->desc, vq->avail, vq->used) &&
+ return vq_access_ok(vq->dev, vq->num, vq->desc, vq->avail, vq->used) &&
vq_log_access_ok(vq, vq->log_base);
}
@@ -577,6 +582,7 @@ static int init_used(struct vhost_virtqueue *vq,
if (r)
return r;
+ vq->signalled_used_valid = false;
return get_user(vq->last_used_idx, &used->idx);
}
@@ -674,7 +680,7 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
* If it is not, we don't as size might not have been setup.
* We will verify when backend is configured. */
if (vq->private_data) {
- if (!vq_access_ok(vq->num,
+ if (!vq_access_ok(d, vq->num,
(void __user *)(unsigned long)a.desc_user_addr,
(void __user *)(unsigned long)a.avail_user_addr,
(void __user *)(unsigned long)a.used_user_addr)) {
@@ -1267,6 +1273,12 @@ int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
eventfd_signal(vq->log_ctx, 1);
}
vq->last_used_idx++;
+ /* If the driver never bothers to signal in a very long while,
+ * used index might wrap around. If that happens, invalidate
+ * signalled_used index we stored. TODO: make sure driver
+ * signals at least once in 2^16 and remove this. */
+ if (unlikely(vq->last_used_idx == vq->signalled_used))
+ vq->signalled_used_valid = false;
return 0;
}
@@ -1275,6 +1287,7 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
unsigned count)
{
struct vring_used_elem __user *used;
+ u16 old, new;
int start;
start = vq->last_used_idx % vq->num;
@@ -1292,7 +1305,14 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
((void __user *)used - (void __user *)vq->used),
count * sizeof *used);
}
- vq->last_used_idx += count;
+ old = vq->last_used_idx;
+ new = (vq->last_used_idx += count);
+ /* If the driver never bothers to signal in a very long while,
+ * used index might wrap around. If that happens, invalidate
+ * signalled_used index we stored. TODO: make sure driver
+ * signals at least once in 2^16 and remove this. */
+ if (unlikely((u16)(new - vq->signalled_used) < (u16)(new - old)))
+ vq->signalled_used_valid = false;
return 0;
}
@@ -1331,29 +1351,47 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
return r;
}
-/* This actually signals the guest, using eventfd. */
-void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
{
- __u16 flags;
-
+ __u16 old, new, event;
+ bool v;
/* Flush out used index updates. This is paired
* with the barrier that the Guest executes when enabling
* interrupts. */
smp_mb();
- if (__get_user(flags, &vq->avail->flags)) {
- vq_err(vq, "Failed to get flags");
- return;
+ if (vhost_has_feature(dev, VIRTIO_F_NOTIFY_ON_EMPTY) &&
+ unlikely(vq->avail_idx == vq->last_avail_idx))
+ return true;
+
+ if (!vhost_has_feature(dev, VIRTIO_RING_F_USED_EVENT_IDX)) {
+ __u16 flags;
+ if (__get_user(flags, &vq->avail->flags)) {
+ vq_err(vq, "Failed to get flags");
+ return true;
+ }
+ return !(flags & VRING_AVAIL_F_NO_INTERRUPT);
}
+ old = vq->signalled_used;
+ v = vq->signalled_used_valid;
+ new = vq->signalled_used = vq->last_used_idx;
+ vq->signalled_used_valid = true;
- /* If they don't want an interrupt, don't signal, unless empty. */
- if ((flags & VRING_AVAIL_F_NO_INTERRUPT) &&
- (vq->avail_idx != vq->last_avail_idx ||
- !vhost_has_feature(dev, VIRTIO_F_NOTIFY_ON_EMPTY)))
- return;
+ if (unlikely(!v))
+ return true;
+
+ if (get_user(event, vhost_used_event(vq))) {
+ vq_err(vq, "Failed to get used event idx");
+ return true;
+ }
+ return vring_need_event(event, new, old);
+}
+/* This actually signals the guest, using eventfd. */
+void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+{
/* Signal the Guest tell them we used something up. */
- if (vq->call_ctx)
+ if (vq->call_ctx && vhost_notify(dev, vq))
eventfd_signal(vq->call_ctx, 1);
}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 0f1bf33..5825ac6 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -84,6 +84,12 @@ struct vhost_virtqueue {
/* Used flags */
u16 used_flags;
+ /* Last used index value we have signalled on */
+ u16 signalled_used;
+
+ /* Last used index value we have signalled on */
+ bool signalled_used_valid;
+
/* Log writes to used structure. */
bool log_used;
u64 log_addr;
@@ -164,6 +170,7 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
enum {
VHOST_FEATURES = (1 << VIRTIO_F_NOTIFY_ON_EMPTY) |
(1 << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1 << VIRTIO_RING_F_USED_EVENT_IDX) |
(1 << VHOST_F_LOG_ALL) |
(1 << VHOST_NET_F_VIRTIO_NET_HDR) |
(1 << VIRTIO_NET_F_MRG_RXBUF),
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCH 11/18] vhost: support avail_event idx
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>
Add support for the new avail_event feature in vhost_net
and vhost test modules.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/net.c | 12 ++++----
drivers/vhost/test.c | 6 ++--
drivers/vhost/vhost.c | 65 +++++++++++++++++++++++++++++++++++++------------
drivers/vhost/vhost.h | 17 +++++++------
4 files changed, 67 insertions(+), 33 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 2f7c76a..e224a92 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -144,7 +144,7 @@ static void handle_tx(struct vhost_net *net)
}
mutex_lock(&vq->mutex);
- vhost_disable_notify(vq);
+ vhost_disable_notify(&net->dev, vq);
if (wmem < sock->sk->sk_sndbuf / 2)
tx_poll_stop(net);
@@ -166,8 +166,8 @@ static void handle_tx(struct vhost_net *net)
set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
break;
}
- if (unlikely(vhost_enable_notify(vq))) {
- vhost_disable_notify(vq);
+ if (unlikely(vhost_enable_notify(&net->dev, vq))) {
+ vhost_disable_notify(&net->dev, vq);
continue;
}
break;
@@ -315,7 +315,7 @@ static void handle_rx(struct vhost_net *net)
return;
mutex_lock(&vq->mutex);
- vhost_disable_notify(vq);
+ vhost_disable_notify(&net->dev, vq);
vhost_hlen = vq->vhost_hlen;
sock_hlen = vq->sock_hlen;
@@ -334,10 +334,10 @@ static void handle_rx(struct vhost_net *net)
break;
/* OK, now we need to know about added descriptors. */
if (!headcount) {
- if (unlikely(vhost_enable_notify(vq))) {
+ if (unlikely(vhost_enable_notify(&net->dev, vq))) {
/* They have slipped one in as we were
* doing that: check again. */
- vhost_disable_notify(vq);
+ vhost_disable_notify(&net->dev, vq);
continue;
}
/* Nothing new? Wait for eventfd to tell us
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 099f302..734e1d7 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -49,7 +49,7 @@ static void handle_vq(struct vhost_test *n)
return;
mutex_lock(&vq->mutex);
- vhost_disable_notify(vq);
+ vhost_disable_notify(&n->dev, vq);
for (;;) {
head = vhost_get_vq_desc(&n->dev, vq, vq->iov,
@@ -61,8 +61,8 @@ static void handle_vq(struct vhost_test *n)
break;
/* Nothing new? Wait for eventfd to tell us they refilled. */
if (head == vq->num) {
- if (unlikely(vhost_enable_notify(vq))) {
- vhost_disable_notify(vq);
+ if (unlikely(vhost_enable_notify(&n->dev, vq))) {
+ vhost_disable_notify(&n->dev, vq);
continue;
}
break;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index e33d5a3..2aea4cb 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -38,6 +38,7 @@ enum {
};
#define vhost_used_event(vq) ((u16 __user *)&vq->avail->ring[vq->num])
+#define vhost_avail_event(vq) ((u16 __user *)&vq->used->ring[vq->num])
static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
poll_table *pt)
@@ -499,11 +500,12 @@ static int vq_access_ok(struct vhost_dev *d, unsigned int num,
struct vring_used __user *used)
{
size_t sa = vhost_has_feature(d, VIRTIO_RING_F_USED_EVENT_IDX) ? 2 : 0;
+ size_t su = vhost_has_feature(d, VIRTIO_RING_F_AVAIL_EVENT_IDX) ? 2 : 0;
return access_ok(VERIFY_READ, desc, num * sizeof *desc) &&
access_ok(VERIFY_READ, avail,
sizeof *avail + num * sizeof *avail->ring + sa) &&
access_ok(VERIFY_WRITE, used,
- sizeof *used + num * sizeof *used->ring);
+ sizeof *used + num * sizeof *used->ring + su);
}
/* Can we log writes? */
@@ -519,9 +521,11 @@ int vhost_log_access_ok(struct vhost_dev *dev)
/* Verify access for write logging. */
/* Caller should have vq mutex and device mutex */
-static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
+static int vq_log_access_ok(struct vhost_dev *d, struct vhost_virtqueue *vq,
+ void __user *log_base)
{
struct vhost_memory *mp;
+ size_t s = vhost_has_feature(d, VIRTIO_RING_F_AVAIL_EVENT_IDX) ? 2 : 0;
mp = rcu_dereference_protected(vq->dev->memory,
lockdep_is_held(&vq->mutex));
@@ -529,7 +533,7 @@ static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) &&
(!vq->log_used || log_access_ok(log_base, vq->log_addr,
sizeof *vq->used +
- vq->num * sizeof *vq->used->ring));
+ vq->num * sizeof *vq->used->ring + s));
}
/* Can we start vq? */
@@ -537,7 +541,7 @@ static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
int vhost_vq_access_ok(struct vhost_virtqueue *vq)
{
return vq_access_ok(vq->dev, vq->num, vq->desc, vq->avail, vq->used) &&
- vq_log_access_ok(vq, vq->log_base);
+ vq_log_access_ok(vq->dev, vq, vq->log_base);
}
static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
@@ -824,7 +828,7 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, unsigned long arg)
vq = d->vqs + i;
mutex_lock(&vq->mutex);
/* If ring is inactive, will check when it's enabled. */
- if (vq->private_data && !vq_log_access_ok(vq, base))
+ if (vq->private_data && !vq_log_access_ok(d, vq, base))
r = -EFAULT;
else
vq->log_base = base;
@@ -1225,6 +1229,10 @@ int vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq,
/* On success, increment avail index. */
vq->last_avail_idx++;
+
+ /* Assume notifications from guest are disabled at this point,
+ * if they aren't we would need to update avail_event index. */
+ BUG_ON(!(vq->used_flags & VRING_USED_F_NO_NOTIFY));
return head;
}
@@ -1414,7 +1422,7 @@ void vhost_add_used_and_signal_n(struct vhost_dev *dev,
}
/* OK, now we need to know about added descriptors. */
-bool vhost_enable_notify(struct vhost_virtqueue *vq)
+bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
{
u16 avail_idx;
int r;
@@ -1422,11 +1430,34 @@ bool vhost_enable_notify(struct vhost_virtqueue *vq)
if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
return false;
vq->used_flags &= ~VRING_USED_F_NO_NOTIFY;
- r = put_user(vq->used_flags, &vq->used->flags);
- if (r) {
- vq_err(vq, "Failed to enable notification at %p: %d\n",
- &vq->used->flags, r);
- return false;
+ if (!vhost_has_feature(dev, VIRTIO_RING_F_AVAIL_EVENT_IDX)) {
+ r = put_user(vq->used_flags, &vq->used->flags);
+ if (r) {
+ vq_err(vq, "Failed to enable notification at %p: %d\n",
+ &vq->used->flags, r);
+ return false;
+ }
+ } else {
+ r = put_user(vq->last_avail_idx, vhost_avail_event(vq));
+ if (r) {
+ vq_err(vq, "Failed to update avail event index at %p: %d\n",
+ vhost_avail_event(vq), r);
+ return false;
+ }
+ }
+ if (unlikely(vq->log_used)) {
+ void __user *used;
+ /* Make sure data is seen before log. */
+ smp_wmb();
+ used = vhost_has_feature(dev, VIRTIO_RING_F_AVAIL_EVENT_IDX) ?
+ &vq->used->flags : vhost_avail_event(vq);
+ /* Log used flags or event index entry write. Both are 16 bit
+ * fields. */
+ log_write(vq->log_base, vq->log_addr +
+ (used - (void __user *)vq->used),
+ sizeof(u16));
+ if (vq->log_ctx)
+ eventfd_signal(vq->log_ctx, 1);
}
/* They could have slipped one in as we were doing that: make
* sure it's written, then check again. */
@@ -1442,15 +1473,17 @@ bool vhost_enable_notify(struct vhost_virtqueue *vq)
}
/* We don't need to be notified again. */
-void vhost_disable_notify(struct vhost_virtqueue *vq)
+void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
{
int r;
if (vq->used_flags & VRING_USED_F_NO_NOTIFY)
return;
vq->used_flags |= VRING_USED_F_NO_NOTIFY;
- r = put_user(vq->used_flags, &vq->used->flags);
- if (r)
- vq_err(vq, "Failed to enable notification at %p: %d\n",
- &vq->used->flags, r);
+ if (!vhost_has_feature(dev, VIRTIO_RING_F_AVAIL_EVENT_IDX)) {
+ r = put_user(vq->used_flags, &vq->used->flags);
+ if (r)
+ vq_err(vq, "Failed to enable notification at %p: %d\n",
+ &vq->used->flags, r);
+ }
}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 5825ac6..edf84be 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -155,8 +155,8 @@ void vhost_add_used_and_signal(struct vhost_dev *, struct vhost_virtqueue *,
void vhost_add_used_and_signal_n(struct vhost_dev *, struct vhost_virtqueue *,
struct vring_used_elem *heads, unsigned count);
void vhost_signal(struct vhost_dev *, struct vhost_virtqueue *);
-void vhost_disable_notify(struct vhost_virtqueue *);
-bool vhost_enable_notify(struct vhost_virtqueue *);
+void vhost_disable_notify(struct vhost_dev *, struct vhost_virtqueue *);
+bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
@@ -168,12 +168,13 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
} while (0)
enum {
- VHOST_FEATURES = (1 << VIRTIO_F_NOTIFY_ON_EMPTY) |
- (1 << VIRTIO_RING_F_INDIRECT_DESC) |
- (1 << VIRTIO_RING_F_USED_EVENT_IDX) |
- (1 << VHOST_F_LOG_ALL) |
- (1 << VHOST_NET_F_VIRTIO_NET_HDR) |
- (1 << VIRTIO_NET_F_MRG_RXBUF),
+ VHOST_FEATURES = (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |
+ (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1ULL << VIRTIO_RING_F_USED_EVENT_IDX) |
+ (1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX) |
+ (1ULL << VHOST_F_LOG_ALL) |
+ (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
+ (1ULL << VIRTIO_NET_F_MRG_RXBUF),
};
static inline bool vhost_has_feature(struct vhost_dev *dev, int bit)
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCH 12/18] virtio_test: support used_event index
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>
Add ability to test the new used_event feature,
enable by default.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
tools/virtio/virtio_test.c | 18 ++++++++++++++++--
1 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 9e65e6d..157ec68 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -210,18 +210,29 @@ const struct option longopts[] = {
.val = 'i',
},
{
+ .name = "used-event-idx",
+ .val = 'U',
+ },
+ {
+ .name = "no-used-event-idx",
+ .val = 'u',
+ },
+ {
}
};
static void help()
{
- fprintf(stderr, "Usage: virtio_test [--help] [--no-indirect]\n");
+ fprintf(stderr, "Usage: virtio_test [--help]"
+ " [--no-indirect] "
+ " [--no-used-event-idx]\n");
}
int main(int argc, char **argv)
{
struct vdev_info dev;
- unsigned long long features = 1ULL << VIRTIO_RING_F_INDIRECT_DESC;
+ unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1ULL << VIRTIO_RING_F_USED_EVENT_IDX);
int o;
for (;;) {
@@ -238,6 +249,9 @@ int main(int argc, char **argv)
case 'i':
features &= ~(1ULL << VIRTIO_RING_F_INDIRECT_DESC);
break;
+ case 'u':
+ features &= ~(1ULL << VIRTIO_RING_F_USED_EVENT_IDX);
+ break;
default:
assert(0);
break;
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCH 13/18] virtio_test: avail_event index support
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Rusty Russell, Carsten Otte, Christian Borntraeger, linux390,
Martin Schwidefsky, Heiko Carstens, Shirley Ma, lguest,
linux-kernel, virtualization, netdev, linux-s390, kvm,
Krishna Kumar, Tom Lendacky, steved, habanero
In-Reply-To: <cover.1304541918.git.mst@redhat.com>
Add ability to test the new avail_event feature,
enable by default.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
tools/virtio/virtio_test.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 157ec68..8adf55d 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -202,6 +202,14 @@ const struct option longopts[] = {
.val = 'h',
},
{
+ .name = "avail-event-idx",
+ .val = 'A',
+ },
+ {
+ .name = "no-avail-event-idx",
+ .val = 'a',
+ },
+ {
.name = "indirect",
.val = 'I',
},
@@ -224,7 +232,8 @@ const struct option longopts[] = {
static void help()
{
fprintf(stderr, "Usage: virtio_test [--help]"
- " [--no-indirect] "
+ " [--no-indirect]"
+ " [--no-avail-event-idx]"
" [--no-used-event-idx]\n");
}
@@ -232,7 +241,8 @@ int main(int argc, char **argv)
{
struct vdev_info dev;
unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
- (1ULL << VIRTIO_RING_F_USED_EVENT_IDX);
+ (1ULL << VIRTIO_RING_F_USED_EVENT_IDX) |
+ (1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX);
int o;
for (;;) {
@@ -243,6 +253,9 @@ int main(int argc, char **argv)
case '?':
help();
exit(2);
+ case 'a':
+ features &= ~(1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX);
+ break;
case 'h':
help();
goto done;
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCH 14/18] virtio: add api for delayed callbacks
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>
Add an API that tells the other side that callbacks
should be delayed until a lot of work has been done.
Implement using the new used_event feature.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/virtio/virtio_ring.c | 27 +++++++++++++++++++++++++++
include/linux/virtio.h | 9 +++++++++
2 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 262dfe6..3a70d70 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -397,6 +397,33 @@ bool virtqueue_enable_cb(struct virtqueue *_vq)
}
EXPORT_SYMBOL_GPL(virtqueue_enable_cb);
+bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ int bufs;
+
+ START_USE(vq);
+
+ /* We optimistically turn back on interrupts, then check if there was
+ * more to do. */
+ /* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
+ * either clear the flags bit or point the event index at the next
+ * entry. Always do both to keep code simple. */
+ vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT;
+ /* TODO: tune this threshold */
+ bufs = (vq->vring.avail->idx - vq->last_used_idx) * 3 / 4;
+ vring_used_event(&vq->vring) = vq->last_used_idx + bufs;
+ virtio_mb();
+ if (unlikely(vq->vring.used->idx - vq->last_used_idx > bufs)) {
+ END_USE(vq);
+ return false;
+ }
+
+ END_USE(vq);
+ return true;
+}
+EXPORT_SYMBOL_GPL(virtqueue_enable_cb_delayed);
+
void *virtqueue_detach_unused_buf(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 718336b..5151fd1 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -51,6 +51,13 @@ struct virtqueue {
* This re-enables callbacks; it returns "false" if there are pending
* buffers in the queue, to detect a possible race between the driver
* checking for more work, and enabling callbacks.
+ * virtqueue_enable_cb_delayed: restart callbacks after disable_cb.
+ * vq: the struct virtqueue we're talking about.
+ * This re-enables callbacks but hints to the other side to delay
+ * interrupts until most of the available buffers have been processed;
+ * it returns "false" if there are many pending buffers in the queue,
+ * to detect a possible race between the driver checking for more work,
+ * and enabling callbacks.
* virtqueue_detach_unused_buf: detach first unused buffer
* vq: the struct virtqueue we're talking about.
* Returns NULL or the "data" token handed to add_buf
@@ -86,6 +93,8 @@ void virtqueue_disable_cb(struct virtqueue *vq);
bool virtqueue_enable_cb(struct virtqueue *vq);
+bool virtqueue_enable_cb_delayed(struct virtqueue *vq);
+
void *virtqueue_detach_unused_buf(struct virtqueue *vq);
/**
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCH 16/18] virtio_ring: Add capacity check API
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>
>From: Shirley Ma <mashirle@us.ibm.com>
Signed-off-by: Shirley Ma <xma@us.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
I'm not sure who wrote this first anymore :)
But it's a simple patch.
drivers/virtio/virtio_ring.c | 8 ++++++++
include/linux/virtio.h | 5 +++++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 3a70d70..57bf9d5 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -365,6 +365,14 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
}
EXPORT_SYMBOL_GPL(virtqueue_get_buf);
+int virtqueue_get_capacity(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ return vq->num_free;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_capacity);
+
void virtqueue_disable_cb(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 5151fd1..944ebcd 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -42,6 +42,9 @@ struct virtqueue {
* vq: the struct virtqueue we're talking about.
* len: the length written into the buffer
* Returns NULL or the "data" token handed to add_buf.
+ * virtqueue_get_capacity: get the current capacity of the queue
+ * vq: the struct virtqueue we're talking about.
+ * Returns remaining capacity of the queue.
* virtqueue_disable_cb: disable callbacks
* vq: the struct virtqueue we're talking about.
* Note that this is not necessarily synchronous, hence unreliable and only
@@ -89,6 +92,8 @@ void virtqueue_kick(struct virtqueue *vq);
void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len);
+int virtqueue_get_capacity(struct virtqueue *vq);
+
void virtqueue_disable_cb(struct virtqueue *vq);
bool virtqueue_enable_cb(struct virtqueue *vq);
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCH 17/18] virtio_net: fix TX capacity checks using new API
From: Michael S. Tsirkin @ 2011-05-04 20:53 UTC (permalink / raw)
To: linux-kernel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>
virtio net uses the number of sg entries to
check for TX ring capacity freed. But this
gives incorrect results when indirect buffers
are used. Use the new capacity API instead.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f685324..f33c92b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -509,19 +509,17 @@ again:
return received;
}
-static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
+static void free_old_xmit_skbs(struct virtnet_info *vi)
{
struct sk_buff *skb;
- unsigned int len, tot_sgs = 0;
+ unsigned int len;
while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
pr_debug("Sent skb %p\n", skb);
vi->dev->stats.tx_bytes += skb->len;
vi->dev->stats.tx_packets++;
- tot_sgs += skb_vnet_hdr(skb)->num_sg;
dev_kfree_skb_any(skb);
}
- return tot_sgs;
}
static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -611,7 +609,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
netif_stop_queue(dev);
if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
/* More just got used, free them then recheck. */
- capacity += free_old_xmit_skbs(vi);
+ free_old_xmit_skbs(vi);
+ capacity = virtqueue_get_capacity(vi->svq);
if (capacity >= 2+MAX_SKB_FRAGS) {
netif_start_queue(dev);
virtqueue_disable_cb(vi->svq);
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCH 18/18] virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-05-04 20:53 UTC (permalink / raw)
To: linux-kernel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>
Current code might introduce a lot of latency variation
if there are many pending bufs at the time we
attempt to transmit a new one. This is bad for
real-time applications and can't be good for TCP either.
Free up just enough to both clean up all buffers
eventually and to be able to xmit the next packet.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 18 +++++++++++-------
1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f33c92b..9982bd7 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -509,17 +509,23 @@ again:
return received;
}
-static void free_old_xmit_skbs(struct virtnet_info *vi)
+static bool free_old_xmit_skbs(struct virtnet_info *vi, int capacity)
{
struct sk_buff *skb;
unsigned int len;
+ bool c;
+ /* We try to free up at least 2 skbs per one sent, so that we'll get
+ * all of the memory back if they are used fast enough. */
+ int n = 2;
- while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
+ while ((c = virtqueue_get_capacity(vi->svq) >= capacity) && --n > 0 &&
+ (skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
pr_debug("Sent skb %p\n", skb);
vi->dev->stats.tx_bytes += skb->len;
vi->dev->stats.tx_packets++;
dev_kfree_skb_any(skb);
}
+ return c;
}
static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -574,8 +580,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
struct virtnet_info *vi = netdev_priv(dev);
int capacity;
- /* Free up any pending old buffers before queueing new ones. */
- free_old_xmit_skbs(vi);
+ /* Free enough pending old buffers to enable queueing new ones. */
+ free_old_xmit_skbs(vi, 2+MAX_SKB_FRAGS);
/* Try to transmit */
capacity = xmit_skb(vi, skb);
@@ -609,9 +615,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
netif_stop_queue(dev);
if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
/* More just got used, free them then recheck. */
- free_old_xmit_skbs(vi);
- capacity = virtqueue_get_capacity(vi->svq);
- if (capacity >= 2+MAX_SKB_FRAGS) {
+ if (!likely(free_old_xmit_skbs(vi, 2+MAX_SKB_FRAGS))) {
netif_start_queue(dev);
virtqueue_disable_cb(vi->svq);
}
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCH 15/18] virtio_net: delay TX callbacks
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Rusty Russell, Carsten Otte, Christian Borntraeger, linux390,
Martin Schwidefsky, Heiko Carstens, Shirley Ma, lguest,
linux-kernel, virtualization, netdev, linux-s390, kvm,
Krishna Kumar, Tom Lendacky, steved, habanero
In-Reply-To: <cover.1304541918.git.mst@redhat.com>
Ask for delayed callbacks on TX ring full, to give the
other side more of a chance to make progress.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0cb0b06..f685324 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -609,7 +609,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
* before it gets out of hand. Naturally, this wastes entries. */
if (capacity < 2+MAX_SKB_FRAGS) {
netif_stop_queue(dev);
- if (unlikely(!virtqueue_enable_cb(vi->svq))) {
+ if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
/* More just got used, free them then recheck. */
capacity += free_old_xmit_skbs(vi);
if (capacity >= 2+MAX_SKB_FRAGS) {
--
1.7.5.53.gc233e
^ permalink raw reply related
* Re: [PATCH] tcp_cubic: limit delayed_ack ratio to prevent divide error
From: Brandeburg, Jesse @ 2011-05-04 20:53 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David Miller, Sangtae Ha, Injong Rhee, Valdis.Kletnieks@vt.edu,
rdunlap@xenotime.net, lkml@techboom.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20110504130456.425dee68@nehalam>
On Wed, 4 May 2011, Stephen Hemminger wrote:
> TCP Cubic keeps a metric that estimates the amount of delayed
> acknowledgements to use in adjusting the window. If an abnormally
> large number of packets are acknowledged at once, then the update
> could wrap and reach zero. This kind of ACK could only
> happen when there was a large window and huge number of
> ACK's were lost.
>
> This patch limits the value of delayed ack ratio. The choice of 32
> is just a conservative value since normally it should be range of
> 1 to 4 packets.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
patch seems fine, but please credit the reporter (lkml@techboom.com) with
reporting the issue with logs, maybe even with Reported-by: and some kind
of reference to the panic message or the email thread in the text or
header?
^ permalink raw reply
* Re: [PATCH v4 1/1] can: add pruss CAN driver.
From: Oliver Hartkopp @ 2011-05-04 20:55 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: sachi-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Arnd Bergmann, Subhasish Ghosh, nsekhar-l0cyMroinI0, open list,
CAN NETWORK DRIVERS, Marc Kleine-Budde,
Netdev-u79uwXL29TY76Z2rM5mHXA, m-watkins-l0cyMroinI0,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <4DC17A31.8070409-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
On 04.05.2011 18:09, Wolfgang Grandegger wrote:
> On 05/04/2011 05:57 PM, Kurt Van Dijck wrote:
>> When doing so, I'd vote for an unlimited(by software) list of hardware filters (id/mask).
>> The hardware must abort when no more filters are available.
>
> Sounds good and not even to complicated. For the SJA1000 we would just
> allow to set the global mask.
Yes. "unlimited(by software)" was a bit misleading at first reading, as we
should not filter IDs by software in the irq handler. But to create a API that
supports as much HW filters as the hardware provides is a good idea.
>> I think that when using hardware filters, knowing the actual device
>> with it's amount of hardware filters is the least of your problems.
>> Userspace applications that suddenly stop working properly due to
>> hw filters (i.e. some traffic not coming in anymore) will be a major
>> source of bugreports.
>
> Well, hardware filtering will be off by default and must explicitly be
> set by the user, like for the bitrate setting.
To be correct: By the admin.
The setting of CAN HW filters has a system-wide effect to all users on the
local host. The same effect as we have for the setting of the bitrate. This is
the major difference to the user-configurable per-socket CAN-ID filters that
are provided e.g. by the CAN_RAW socket.
As the current netlink configuration interface for CAN interfaces is not
accessible for standard users also this would be the right place to extend the
netlink interface.
Regards,
Oliver
^ permalink raw reply
* Re: [net-next-2.6 0/9][pull request] Intel Wired LAN Driver Update
From: David Miller @ 2011-05-04 20:56 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <1304537441-2056-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 4 May 2011 12:30:32 -0700
> The following series contains updates to e100, igb, igbvf, ixgb and ixgbe.
>
> The following are changes since commit e67f88dd12f610da98ca838822f2c9b4e7c6100e:
> net: dont hold rtnl mutex during netlink dump callbacks
> and are available in the git repository at:
> master.kernel.org:/pub/scm/linux/kernel/git/jkirsher/net-next-2.6 master
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [net-next-2.6 0/9][pull request] Intel Wired LAN Driver Update
From: David Miller @ 2011-05-04 20:59 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <20110504.135621.232749554.davem@davemloft.net>
BTW, Jeff, can I get some kind of time frame on when the ethtool_ops->phys_id
conversions/removals for e100, e1000, and igb will be coming my way?
It's starting to be on the order of weeks that I've been waiting for
this so that I can apply Stephen Hemminger's patch which gets rid of
the old ->phys_id interfaces entirely.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] libertas: Convert lbs_pr_<level> to pr_<level>
From: Joe Perches @ 2011-05-04 21:01 UTC (permalink / raw)
To: Dan Williams
Cc: John W. Linville, libertas-dev, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1304537949.1097.8.camel@dcbw.foobar.com>
On Wed, 2011-05-04 at 14:39 -0500, Dan Williams wrote:
> On Mon, 2011-05-02 at 16:49 -0700, Joe Perches wrote:
> > Use the standard pr_<level> functions eases grep a bit.
> > Added a few missing terminating newlines to messages.
> > Coalesced long formats.
> Is there any reason to not put the pr_fmt() definition into 'defs.h'
> instead of C&P at the top of every file? I don't really care either way
> but that seems cleaner since almost all the libertas files are going to
> use logging.
It has to be before any #include <linux/kernel.h>
or any other #include that uses it.
At some point, all of the uses of
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
should be removed as I intend to have that
or an equivalent become the default.
^ permalink raw reply
* Re: 2.6.38.2, kernel panic, probably related to framentation handling
From: David Miller @ 2011-05-04 21:05 UTC (permalink / raw)
To: eric.dumazet; +Cc: denys, netdev
In-Reply-To: <1304539346.32152.81.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 04 May 2011 22:02:26 +0200
> [PATCH] net: ip_expire() must revalidate route
>
> Commit 4a94445c9a5c (net: Use ip_route_input_noref() in input path)
> added a bug in IP defragmentation handling, in case timeout is fired.
>
> When a frame is defragmented, we use last skb dst field when building
> final skb. Its dst is valid, since we are in rcu read section.
>
> But if a timeout occurs, we take first queued fragment to build one ICMP
> TIME EXCEEDED message. Problem is all queued skb have weak dst pointers,
> since we escaped RCU critical section after their queueing. icmp_send()
> might dereference a now freed (and possibly reused) part of memory.
>
> Calling skb_dst_drop() and ip_route_input_noref() to revalidate route is
> the only possible choice.
>
> Reported-by: Denys Fedoryshchenko <denys@visp.net.lb>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
Applied to net-2.6 and queued up for -stable, thanks Eric!
^ permalink raw reply
* Re: [ethtool PATCH 4/4] v5 Add RX packet classification interface
From: Alexander Duyck @ 2011-05-04 21:07 UTC (permalink / raw)
To: Ben Hutchings
Cc: Dimitris Michailidis, davem@davemloft.net, Kirsher, Jeffrey T,
netdev@vger.kernel.org
In-Reply-To: <1304534738.2926.74.camel@bwh-desktop>
On 5/4/2011 11:45 AM, Ben Hutchings wrote:
> On Wed, 2011-05-04 at 11:21 -0700, Alexander Duyck wrote:
> [...]
>> Honestly what I would prefer to see is a seperate call added such as an
>> ETHTOOL_GRSCLSRLLOC that we could pass the flow specifier to and perhaps
>> include first/last location call in that and then let the driver return
>> where it wants to drop the rule.
>
> This must not be done as a separate operation because it's racy (in fact
> that's an inherent problem with the rule manager). In the sfc driver
> (and probably others in future) filters could be inserted for RFS at any
> time.
>
>> That way we can avoid having to create
>> an overly complicated rule manager that can handle all the bizarre rule
>> ordering options that I am sure all the different network devices support.
>
> Right, the rule manager can't implement that.
>
>> The only reason I am not implementing this now is because there aren't
>> any drivers in place that would currently use it. I figure once cxgb
>> has a means in place of supporting flow classifier rules then Dimitris
>> can add the necessary code to ethtool and the kernel to allow the driver
>> to specify rule locations. I would prefer to avoid adding features
>> based on speculation of what will be needed and would like to be able to
>> actually see how the features will be used.
>
> If you are going to implement the same interface in ixgbe as in niu
> (modulo bugs), then I have no objection to going ahead with this and
> then adding the option for driver-assigned locations later.
>
> Please can you confirm that the location specified for
> ETHTOOL_SRXCLSRLINS will indeed be used as a priority in case of
> overlapping filters?
>
> Ben.
>
The ixgbe approach should be nearly identical in terms of how the
priorities are based on the location of the filters. The original
version from Santwona had the rule manager breaking the rules up into 7
sections so that rules that specified fewer fields would be near the end
of the list. I'm pretty sure that was all due to priorities from what I
could see in the niu driver since the filters that covered wider ranges
were being made lower priority to be matched last.
In terms of overloading the get count call, that probably would be the
best route in terms of changing rule manager behavior. The only thing I
am having a hard time seeing is how the rule manager would be able to
distinguish between low priority and high priority filter rules, or is
this something that new keywords would be added to the parser for?
I just put out version 6 of the patches. Essentially I have reduced the
size of the rule manager to being used only on insertion without any
rule location specified. The one thing to keep in mind with this rule
manager is that the rule at table size - 1 is always going to be the
lowest priority rule. So if it was reserved for unspecified rules it
would be easy to use something like that to achieve an "auto-select"
location that the driver could then reassign as rules were added to it.
Thanks,
Alex
^ permalink raw reply
* Re: [net-next-2.6 PATCH] can: make struct can_proto const
From: David Miller @ 2011-05-04 21:08 UTC (permalink / raw)
To: socketcan-fJ+pQTUTwRTk1uMJSBkQmQ
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4DC1B86A.40308-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
From: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
Date: Wed, 04 May 2011 22:34:50 +0200
> On 04.05.2011 06:40, Kurt Van Dijck wrote:
>> commit 53914b67993c724cec585863755c9ebc8446e83b had the
>> same message. That commit did put everything in place but
>> did not make can_proto const itself.
>>
>> Signed-off-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
>>
>
> Acked-by: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
Applied.
^ permalink raw reply
* Re: [net-next-2.6 PATCH] can: rename can_try_module_get to can_get_proto
From: David Miller @ 2011-05-04 21:08 UTC (permalink / raw)
To: socketcan-fJ+pQTUTwRTk1uMJSBkQmQ
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4DC1B89A.3080904-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
From: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
Date: Wed, 04 May 2011 22:35:38 +0200
> On 04.05.2011 06:42, Kurt Van Dijck wrote:
>> can: rename can_try_module_get to can_get_proto
>>
>> can_try_module_get does return a struct can_proto.
>> The name explains what is done in so much detail that a caller
>> may not notice that a struct can_proto is locked/unlocked.
>>
>> Signed-off-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
>
> Acked-by: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
Applied.
^ permalink raw reply
* Re: [net-next-2.6 0/9][pull request] Intel Wired LAN Driver Update
From: Jeff Kirsher @ 2011-05-04 21:14 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, gospo@redhat.com, bphilips@novell.com
In-Reply-To: <20110504.135904.229767743.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 550 bytes --]
On Wed, 2011-05-04 at 13:59 -0700, David Miller wrote:
> BTW, Jeff, can I get some kind of time frame on when the ethtool_ops->phys_id
> conversions/removals for e100, e1000, and igb will be coming my way?
>
> It's starting to be on the order of weeks that I've been waiting for
> this so that I can apply Stephen Hemminger's patch which gets rid of
> the old ->phys_id interfaces entirely.
>
> Thanks.
I have the remaining patches in testing currently. Should have the
remaining three patches to you by this weekend (if not sooner).
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [net-next-2.6 0/9][pull request] Intel Wired LAN Driver Update
From: David Miller @ 2011-05-04 21:16 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <1304543684.2998.6.camel@jtkirshe-MOBL1>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 04 May 2011 14:14:44 -0700
> On Wed, 2011-05-04 at 13:59 -0700, David Miller wrote:
>> BTW, Jeff, can I get some kind of time frame on when the ethtool_ops->phys_id
>> conversions/removals for e100, e1000, and igb will be coming my way?
>>
>> It's starting to be on the order of weeks that I've been waiting for
>> this so that I can apply Stephen Hemminger's patch which gets rid of
>> the old ->phys_id interfaces entirely.
>>
>> Thanks.
>
> I have the remaining patches in testing currently. Should have the
> remaining three patches to you by this weekend (if not sooner).
Thanks!
^ permalink raw reply
* [PDFv2] virtio-spec: 64 bit features, used/avail event
From: Michael S. Tsirkin @ 2011-05-04 21:17 UTC (permalink / raw)
To: linux-kernel, qemu-devel, kvm
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, linux-s390,
netdev, habanero, Rusty Russell, Heiko Carstens, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <20110504203256.GA20819@redhat.com>
People asked for a pdf for a new spec, so here it is:
http://userweb.kernel.org/~mst/virtio-spec-event-idx-v2.pdf
Guest and host implementation can be found here:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net-next-event-idx-v1
git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu-kvm.git virtio-net-event-idx-v1
Description reposted below:
I'm working on a patchset (to follow shortly)
that modified the notificatin hand-off in virtio to be basically
like Xen: each side published an index, the other side only triggers
an event when it crosses that index value
(Xen event indexes start at 1, ours start at 0 for
backward-compatiblity, but that's minor).
Especially for testing, it is very convenient to have
separate feature bits for this change in used and available
ring; since we've run out of bits in the 32 bit field,
I added another 32 bit and bit 31 enables that.
I started with using both flags and indexes in parallel,
but switched to doing either-or: this means we do
not need to tweak memory access ordering as index access just
replaces flags access.
A note on naming: the index replacing avail->flags is named
used_event, the index replacing used->flags is named
avail_event to stress the fact that these actually
point into the other side of the ring:
event is triggered when avail->idx == used->avail_event + 1
and when used->idx == avail->used_event + 1, respectively.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
--
MST
^ permalink raw reply
* Re: [PATCH 0/4] [RFC] virtio-net: Improve small packet performance
From: Michael S. Tsirkin @ 2011-05-04 21:23 UTC (permalink / raw)
To: Krishna Kumar2; +Cc: davem, eric.dumazet, kvm, netdev, rusty
In-Reply-To: <OFAEA8C363.C13C507B-ON65257886.0051F71F-65257886.0052217F@in.ibm.com>
On Wed, May 04, 2011 at 08:29:44PM +0530, Krishna Kumar2 wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote on 05/04/2011 08:16:22 PM:
>
> > > A. virtio:
> > > - Provide a API to get available number of slots.
> > >
> > > B. virtio-net:
> > > - Remove stop/start txq's and associated callback.
> > > - Pre-calculate the number of slots needed to transmit
> > > the skb in xmit_skb and bail out early if enough space
> > > is not available. My testing shows that 2.5-3% of
> > > packets are benefited by using this API.
> > > - Do not drop skbs but instead return TX_BUSY like other
> > > drivers.
> > > - When returning EBUSY, set a per-txq variable to indicate
> > > to dev_queue_xmit() whether to restart xmits on this txq.
> > >
> > > C. net/sched/sch_generic.c:
> > > Since virtio-net now returns EBUSY, the skb is requeued to
> > > gso_skb. This allows adding the addional check for restart
> > > xmits in just the slow-path (the first re-queued packet
> > > case of dequeue_skb, where it checks for gso_skb) before
> > > deciding whether to call the driver or not.
> > >
> > > Patch was also tested between two servers with Emulex OneConnect
> > > 10G cards to confirm there is no regression. Though the patch is
> > > an attempt to improve only small packet performance, there was
> > > improvement for 1K, 2K and also 16K both in BW and SD. Results
> > > from Guest -> Remote Host (BW in Mbps) for 1K and 16K I/O sizes:
> > >
> > > ________________________________________________________
> > > I/O Size: 1K
> > > # BW1 BW2 (%) SD1 SD2 (%)
> > > ________________________________________________________
> > > 1 1226 3313 (170.2) 6.6 1.9 (-71.2)
> > > 2 3223 7705 (139.0) 18.0 7.1 (-60.5)
> > > 4 7223 8716 (20.6) 36.5 29.7 (-18.6)
> > > 8 8689 8693 (0) 131.5 123.0 (-6.4)
> > > 16 8059 8285 (2.8) 578.3 506.2 (-12.4)
> > > 32 7758 7955 (2.5) 2281.4 2244.2 (-1.6)
> > > 64 7503 7895 (5.2) 9734.0 9424.4 (-3.1)
> > > 96 7496 7751 (3.4) 21980.9 20169.3 (-8.2)
> > > 128 7389 7741 (4.7) 40467.5 34995.5 (-13.5)
> > > ________________________________________________________
> > > Summary: BW: 16.2% SD: -10.2%
> > >
> > > ________________________________________________________
> > > I/O Size: 16K
> > > # BW1 BW2 (%) SD1 SD2 (%)
> > > ________________________________________________________
> > > 1 6684 7019 (5.0) 1.1 1.1 (0)
> > > 2 7674 7196 (-6.2) 5.0 4.8 (-4.0)
> > > 4 7358 8032 (9.1) 21.3 20.4 (-4.2)
> > > 8 7393 8015 (8.4) 82.7 82.0 (-.8)
> > > 16 7958 8366 (5.1) 283.2 310.7 (9.7)
> > > 32 7792 8113 (4.1) 1257.5 1363.0 (8.3)
> > > 64 7673 8040 (4.7) 5723.1 5812.4 (1.5)
> > > 96 7462 7883 (5.6) 12731.8 12119.8 (-4.8)
> > > 128 7338 7800 (6.2) 21331.7 21094.7 (-1.1)
> > > ________________________________________________________
> > > Summary: BW: 4.6% SD: -1.5%
> > >
> > > Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> > > ---
> >
> > So IIUC, we delay transmit by an arbitrary value and hope
> > that the host is done with the packets by then?
>
> Not "hope" exactly. If the device is not ready, then
> the packet is requeued. The main idea is to avoid
> drops/stop/starts, etc.
Yes, I see that, definitely. I guess it's a win if the
interrupt takes at least a jiffy to arrive anyway,
and a loss if not. Is there some reason interrupts
might be delayed until the next jiffy?
> > Interesting.
> >
> > I am currently testing an approach where
> > we tell the host explicitly to interrupt us only after
> > a large part of the queue is empty.
> > With 256 entries in a queue, we should get 1 interrupt per
> > on the order of 100 packets which does not seem like a lot.
> >
> > I can post it, mind testing this?
>
> Sure.
>
> - KK
Just posted. Would appreciate feedback.
--
MST
^ permalink raw reply
* Re: [ethtool PATCH 4/4] v5 Add RX packet classification interface
From: Ben Hutchings @ 2011-05-04 21:54 UTC (permalink / raw)
To: Alexander Duyck
Cc: Dimitris Michailidis, davem@davemloft.net, Kirsher, Jeffrey T,
netdev@vger.kernel.org
In-Reply-To: <4DC1C018.5090709@intel.com>
On Wed, 2011-05-04 at 14:07 -0700, Alexander Duyck wrote:
> On 5/4/2011 11:45 AM, Ben Hutchings wrote:
[...]
> > Please can you confirm that the location specified for
> > ETHTOOL_SRXCLSRLINS will indeed be used as a priority in case of
> > overlapping filters?
> >
> > Ben.
> >
>
> The ixgbe approach should be nearly identical in terms of how the
> priorities are based on the location of the filters.
OK, good.
> The original
> version from Santwona had the rule manager breaking the rules up into 7
> sections so that rules that specified fewer fields would be near the end
> of the list. I'm pretty sure that was all due to priorities from what I
> could see in the niu driver since the filters that covered wider ranges
> were being made lower priority to be matched last.
That would make sense.
> In terms of overloading the get count call, that probably would be the
> best route in terms of changing rule manager behavior. The only thing I
> am having a hard time seeing is how the rule manager would be able to
> distinguish between low priority and high priority filter rules, or is
> this something that new keywords would be added to the parser for?
Right, there would have to be keywords to specify that.
> I just put out version 6 of the patches. Essentially I have reduced the
> size of the rule manager to being used only on insertion without any
> rule location specified. The one thing to keep in mind with this rule
> manager is that the rule at table size - 1 is always going to be the
> lowest priority rule. So if it was reserved for unspecified rules it
> would be easy to use something like that to achieve an "auto-select"
> location that the driver could then reassign as rules were added to it.
I don't think any location value within the physical table size should
select this special behaviour. The special location values for
auto-select (with whatever priority) should be distinct from all the
physical location values.
I still need to review your patches but it sounds like they will be
ready to apply.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ 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