Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH] virtio: fix oops on OOM
From: Rusty Russell @ 2010-07-26  7:25 UTC (permalink / raw)
  To: virtualization
  Cc: Linus Torvalds, linux-kernel, Chris Mason, Michael S. Tsirkin
In-Reply-To: <201007231548.38037.rusty@rustcorp.com.au>

From: "Michael S. Tsirkin" <mst@redhat.com>

virtio ring was changed to return an error code on OOM,
but one caller was missed and still checks for vq->vring.num.
The fix is just to check for <0 error code.

Long term it might make sense to change goto add_head to
just return an error on oom instead, but let's apply
a minimal fix for 2.6.35.

Reported-by: Chris Mason <chris.mason@oracle.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Tested-by: Chris Mason <chris.mason@oracle.com>
Cc: stable@kernel.org # .34.x
---
 drivers/virtio/virtio_ring.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index dd35b34..bffec32 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -164,7 +164,8 @@ int virtqueue_add_buf_gfp(struct virtqueue *_vq,
 			  gfp_t gfp)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
-	unsigned int i, avail, head, uninitialized_var(prev);
+	unsigned int i, avail, uninitialized_var(prev);
+	int head;
 
 	START_USE(vq);
 
@@ -174,8 +175,8 @@ int virtqueue_add_buf_gfp(struct virtqueue *_vq,
 	 * buffers, then go indirect. FIXME: tune this threshold */
 	if (vq->indirect && (out + in) > 1 && vq->num_free) {
 		head = vring_add_indirect(vq, sg, out, in, gfp);
-		if (head != vq->vring.num)
+		if (likely(head >= 0))
 			goto add_head;
 	}
 
 	BUG_ON(out + in > vq->vring.num);

^ permalink raw reply related

* Call for Demos and Exhibition: The 7th International Conference on Autonomic and Trusted Computing (ATC 2010)
From: Ming Zhao @ 2010-07-28 12:27 UTC (permalink / raw)
  To: tcpp-announce, performance, infodir_sigarch, vsigplan-l,
	publicity, micro_publicity, hpc-announ

The 7th International Conference on Autonomic and Trusted Computing (ATC 
2010)
                      Call for Demos and Exhibition

Highlight: ATC demos will be published on IEEE CS proceedings (indexed 
by EI)!



The ATC 2010 demo/exhibition program provides researchers and engineers 
with opportunities to show their cutting-edge work presented in an 
interactive fashion. The live demonstrations and exhibitions may include 
implementations of techniques presented in the main conference, 
workshops, other conferences and venues, as well as systems and video.

We encourage academic researchers as well as industry lab researchers 
and companies to participate in showcasing their products and research. 
Prospective applicants should submit a 4-page abstract of the 
demonstration/exhibition in LNCS-format via email (atc10demo@gmail.com). 
Video demonstrations are also welcome.

The main selection criteria will be the expected general interest of the 
demonstration to the Autonomic and Trusted Computing community.
The accepted demo/exhibition submitted before July 30 will be included 
as 4 page papers in the conference's IEEE CS proceedings (indexed by 
EI); those submitted after this date will not be included in the 
proceedings. As a condition for acceptance, at least one of the people 
must be registered for the main conference, and present the demo in the 
conference. All demos will compete for the Best Demo Award.

At the conference, you will at minimum be provided with a space for your 
demo, a poster stand, a table and chairs, and a power connection (220V). 
Any other special equipment should be provided by yourself, including 
power converters, projectors, audio amplification, wireless LAN, etc. 
Please specify this with a separate page so that we can plan the demo 
area accordingly.

Demo/Exhibition Chairs:
· Alvin Chin, Nokia Research Center, China
· Ming Zhao, Florida International University, USA

Important Dates:

For demos to be included in ATC proceedings (indexed by EI)
- Demo proposal: July 30
- Acceptance: August 3
- 4-page camera-ready version: August 8

For demos not to be included in ATC proceedings
- Demo proposal: August 15
- Acceptance: August 30

Further Information:
· atc10demo@gmail.com


Thanks,

Alvin Chin and Ming Zhao
ATC2010 Demo Co-Chairs

^ permalink raw reply

* [GIT PULL net-next-2.6] vhost-net patchset for 2.6.36
From: Michael S. Tsirkin @ 2010-07-28 13:32 UTC (permalink / raw)
  To: David Miller; +Cc: kvm, virtualization, netdev, linux-kernel, krkumar2
In-Reply-To: <20100701164127.GA3199@redhat.com>

David,
The following tree includes the current vhost-net patchset.
Please merge it for 2.6.36.
Thanks!

The following changes since commit 4cfa580e7eebb8694b875d2caff3b989ada2efac:

  r6040: Fix args to phy_mii_ioctl(). (2010-07-21 21:10:49 -0700)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net-next

David Stevens (1):
      vhost-net: mergeable buffers support

Michael S. Tsirkin (1):
      vhost: apply cgroup to vhost workers

Sridhar Samudrala (1):
      cgroups: Add an API to attach a task to current task's cgroup

Tejun Heo (1):
      vhost: replace vhost_workqueue with per-vhost kthread

 drivers/vhost/net.c    |  293 +++++++++++++++++++++++++++++++++++++++++-------
 drivers/vhost/vhost.c  |  228 ++++++++++++++++++++++++++++++++-----
 drivers/vhost/vhost.h  |   55 ++++++---
 include/linux/cgroup.h |    7 +
 kernel/cgroup.c        |   23 ++++
 5 files changed, 515 insertions(+), 91 deletions(-)


^ permalink raw reply

* [PATCH] vhost: locking/rcu cleanup
From: Michael S. Tsirkin @ 2010-07-29 12:23 UTC (permalink / raw)
  To: Michael S. Tsirkin, David S. Miller, Sridhar Samudrala, Tejun Heo,
	Jeff Dike <jd>

I saw WARN_ON(!list_empty(&dev->work_list)) trigger
so our custom flush is not as airtight as need be.

This patch switches to a simple atomic counter + srcu instead of
the custom locked queue + flush implementation.

This will slow down the setup ioctls, which should not matter -
it's slow path anyway. We use the expedited flush to at least
make sure it has a sane time bound.

Works fine for me. I got reports that with many guests,
work lock is highly contended, and this patch should in theory
fix this as well - but I haven't tested this yet.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/vhost/net.c   |   55 +++++--------------
 drivers/vhost/vhost.c |  140 ++++++++++++++++++++++---------------------------
 drivers/vhost/vhost.h |   47 +++++++++-------
 3 files changed, 103 insertions(+), 139 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index f13e56b..ee69c51 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -111,8 +111,9 @@ static void tx_poll_start(struct vhost_net *net, struct socket *sock)
 
 /* Expects to be always run from workqueue - which acts as
  * read-size critical section for our kind of RCU. */
-static void handle_tx(struct vhost_net *net)
+static void handle_tx(struct vhost_dev *dev)
 {
+	struct vhost_net *net = container_of(dev, struct vhost_net, dev);
 	struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
 	unsigned out, in, s;
 	int head;
@@ -127,7 +128,7 @@ static void handle_tx(struct vhost_net *net)
 	size_t len, total_len = 0;
 	int err, wmem;
 	size_t hdr_size;
-	struct socket *sock = rcu_dereference(vq->private_data);
+	struct socket *sock = vhost_vq_data(vq, &net->dev);
 	if (!sock)
 		return;
 
@@ -305,7 +306,7 @@ static void handle_rx_big(struct vhost_net *net)
 	size_t len, total_len = 0;
 	int err;
 	size_t hdr_size;
-	struct socket *sock = rcu_dereference(vq->private_data);
+	struct socket *sock = vhost_vq_data(vq, &net->dev);
 	if (!sock || skb_queue_empty(&sock->sk->sk_receive_queue))
 		return;
 
@@ -416,7 +417,7 @@ static void handle_rx_mergeable(struct vhost_net *net)
 	int err, headcount;
 	size_t vhost_hlen, sock_hlen;
 	size_t vhost_len, sock_len;
-	struct socket *sock = rcu_dereference(vq->private_data);
+	struct socket *sock = vhost_vq_data(vq, &net->dev);
 	if (!sock || skb_queue_empty(&sock->sk->sk_receive_queue))
 		return;
 
@@ -500,46 +501,15 @@ static void handle_rx_mergeable(struct vhost_net *net)
 	unuse_mm(net->dev.mm);
 }
 
-static void handle_rx(struct vhost_net *net)
+static void handle_rx(struct vhost_dev *dev)
 {
+	struct vhost_net *net = container_of(dev, struct vhost_net, dev);
 	if (vhost_has_feature(&net->dev, VIRTIO_NET_F_MRG_RXBUF))
 		handle_rx_mergeable(net);
 	else
 		handle_rx_big(net);
 }
 
-static void handle_tx_kick(struct vhost_work *work)
-{
-	struct vhost_virtqueue *vq = container_of(work, struct vhost_virtqueue,
-						  poll.work);
-	struct vhost_net *net = container_of(vq->dev, struct vhost_net, dev);
-
-	handle_tx(net);
-}
-
-static void handle_rx_kick(struct vhost_work *work)
-{
-	struct vhost_virtqueue *vq = container_of(work, struct vhost_virtqueue,
-						  poll.work);
-	struct vhost_net *net = container_of(vq->dev, struct vhost_net, dev);
-
-	handle_rx(net);
-}
-
-static void handle_tx_net(struct vhost_work *work)
-{
-	struct vhost_net *net = container_of(work, struct vhost_net,
-					     poll[VHOST_NET_VQ_TX].work);
-	handle_tx(net);
-}
-
-static void handle_rx_net(struct vhost_work *work)
-{
-	struct vhost_net *net = container_of(work, struct vhost_net,
-					     poll[VHOST_NET_VQ_RX].work);
-	handle_rx(net);
-}
-
 static int vhost_net_open(struct inode *inode, struct file *f)
 {
 	struct vhost_net *n = kmalloc(sizeof *n, GFP_KERNEL);
@@ -550,16 +520,18 @@ static int vhost_net_open(struct inode *inode, struct file *f)
 		return -ENOMEM;
 
 	dev = &n->dev;
-	n->vqs[VHOST_NET_VQ_TX].handle_kick = handle_tx_kick;
-	n->vqs[VHOST_NET_VQ_RX].handle_kick = handle_rx_kick;
+	vhost_work_set_fn(&n->vqs[VHOST_NET_VQ_TX].work, handle_tx);
+	vhost_work_set_fn(&n->vqs[VHOST_NET_VQ_RX].work, handle_rx);
 	r = vhost_dev_init(dev, n->vqs, VHOST_NET_VQ_MAX);
 	if (r < 0) {
 		kfree(n);
 		return r;
 	}
 
-	vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
-	vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);
+	vhost_poll_init(n->poll + VHOST_NET_VQ_TX,
+			&n->vqs[VHOST_NET_VQ_TX].work, POLLOUT, dev);
+	vhost_poll_init(n->poll + VHOST_NET_VQ_RX,
+			&n->vqs[VHOST_NET_VQ_RX].work, POLLIN, dev);
 	n->tx_poll_state = VHOST_NET_POLL_DISABLED;
 
 	f->private_data = n;
@@ -640,6 +612,7 @@ static int vhost_net_release(struct inode *inode, struct file *f)
 	/* We do an extra flush before freeing memory,
 	 * since jobs can re-queue themselves. */
 	vhost_net_flush(n);
+	vhost_dev_free(&n->dev);
 	kfree(n);
 	return 0;
 }
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index e05557d..daa95c8 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -60,22 +60,27 @@ static int vhost_poll_wakeup(wait_queue_t *wait, unsigned mode, int sync,
 	return 0;
 }
 
+/* Must be called for each vq before vhost_dev_init. */
+void vhost_work_set_fn(struct vhost_work *work, vhost_work_fn_t fn)
+{
+	work->fn = fn;
+}
+
+static void vhost_work_init(struct vhost_work *work)
+{
+	atomic_set(&work->queue_seq, 0);
+	work->done_seq = 0;
+}
+
 /* Init poll structure */
-void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
+void vhost_poll_init(struct vhost_poll *poll, struct vhost_work *work,
 		     unsigned long mask, struct vhost_dev *dev)
 {
-	struct vhost_work *work = &poll->work;
-
+	poll->work = work;
 	init_waitqueue_func_entry(&poll->wait, vhost_poll_wakeup);
 	init_poll_funcptr(&poll->table, vhost_poll_func);
 	poll->mask = mask;
 	poll->dev = dev;
-
-	INIT_LIST_HEAD(&work->node);
-	work->fn = fn;
-	init_waitqueue_head(&work->done);
-	work->flushing = 0;
-	work->queue_seq = work->done_seq = 0;
 }
 
 /* Start polling a file. We add ourselves to file's wait queue. The caller must
@@ -99,40 +104,16 @@ void vhost_poll_stop(struct vhost_poll *poll)
  * locks that are also used by the callback. */
 void vhost_poll_flush(struct vhost_poll *poll)
 {
-	struct vhost_work *work = &poll->work;
-	unsigned seq;
-	int left;
-	int flushing;
-
-	spin_lock_irq(&poll->dev->work_lock);
-	seq = work->queue_seq;
-	work->flushing++;
-	spin_unlock_irq(&poll->dev->work_lock);
-	wait_event(work->done, ({
-		   spin_lock_irq(&poll->dev->work_lock);
-		   left = seq - work->done_seq <= 0;
-		   spin_unlock_irq(&poll->dev->work_lock);
-		   left;
-	}));
-	spin_lock_irq(&poll->dev->work_lock);
-	flushing = --work->flushing;
-	spin_unlock_irq(&poll->dev->work_lock);
-	BUG_ON(flushing < 0);
+	synchronize_srcu_expedited(&poll->dev->worker_srcu);
 }
 
 void vhost_poll_queue(struct vhost_poll *poll)
 {
 	struct vhost_dev *dev = poll->dev;
-	struct vhost_work *work = &poll->work;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->work_lock, flags);
-	if (list_empty(&work->node)) {
-		list_add_tail(&work->node, &dev->work_list);
-		work->queue_seq++;
-		wake_up_process(dev->worker);
-	}
-	spin_unlock_irqrestore(&dev->work_lock, flags);
+	struct vhost_work *work = poll->work;
+
+	atomic_inc(&work->queue_seq);
+	wake_up_process(dev->worker);
 }
 
 static void vhost_vq_reset(struct vhost_dev *dev,
@@ -164,41 +145,39 @@ static void vhost_vq_reset(struct vhost_dev *dev,
 static int vhost_worker(void *data)
 {
 	struct vhost_dev *dev = data;
-	struct vhost_work *work = NULL;
-	unsigned uninitialized_var(seq);
+	struct vhost_work *uninitialized_var(work);
+	unsigned n, i, vq = 0;
+	int seq;
 
-	for (;;) {
-		/* mb paired w/ kthread_stop */
-		set_current_state(TASK_INTERRUPTIBLE);
+	n = dev->nvqs;
+repeat:
+	set_current_state(TASK_INTERRUPTIBLE);	/* mb paired w/ kthread_stop */
 
-		spin_lock_irq(&dev->work_lock);
-		if (work) {
-			work->done_seq = seq;
-			if (work->flushing)
-				wake_up_all(&work->done);
-		}
+	if (kthread_should_stop()) {
+		__set_current_state(TASK_RUNNING);
+		return 0;
+	}
 
-		if (kthread_should_stop()) {
-			spin_unlock_irq(&dev->work_lock);
-			__set_current_state(TASK_RUNNING);
-			return 0;
+	for (i = 0; i < n; ++i) {
+		work = &dev->vqs[(vq + i) % n].work;
+		seq = atomic_read(&work->queue_seq);
+		if (seq != work->done_seq) {
+			work->done_seq = seq;
+			break;
 		}
-		if (!list_empty(&dev->work_list)) {
-			work = list_first_entry(&dev->work_list,
-						struct vhost_work, node);
-			list_del_init(&work->node);
-			seq = work->queue_seq;
-		} else
-			work = NULL;
-		spin_unlock_irq(&dev->work_lock);
+		work = NULL;
+	}
 
-		if (work) {
-			__set_current_state(TASK_RUNNING);
-			work->fn(work);
-		} else
-			schedule();
+	if (work) {
+		int idx;
+		__set_current_state(TASK_RUNNING);
+		idx = srcu_read_lock(&dev->worker_srcu);
+		work->fn(dev);
+		srcu_read_unlock(&dev->worker_srcu, idx);
+	} else
+		schedule();
 
-	}
+	goto repeat;
 }
 
 long vhost_dev_init(struct vhost_dev *dev,
@@ -213,20 +192,22 @@ long vhost_dev_init(struct vhost_dev *dev,
 	dev->log_file = NULL;
 	dev->memory = NULL;
 	dev->mm = NULL;
-	spin_lock_init(&dev->work_lock);
-	INIT_LIST_HEAD(&dev->work_list);
 	dev->worker = NULL;
 
 	for (i = 0; i < dev->nvqs; ++i) {
 		dev->vqs[i].dev = dev;
 		mutex_init(&dev->vqs[i].mutex);
 		vhost_vq_reset(dev, dev->vqs + i);
-		if (dev->vqs[i].handle_kick)
+		if (dev->vqs[i].work.fn)
 			vhost_poll_init(&dev->vqs[i].poll,
-					dev->vqs[i].handle_kick, POLLIN, dev);
+					&dev->vqs[i].work, POLLIN, dev);
 	}
+	return init_srcu_struct(&dev->worker_srcu);
+}
 
-	return 0;
+void vhost_dev_free(struct vhost_dev *dev)
+{
+	cleanup_srcu_struct(&dev->worker_srcu);
 }
 
 /* Caller should have device mutex */
@@ -240,7 +221,7 @@ long vhost_dev_check_owner(struct vhost_dev *dev)
 static long vhost_dev_set_owner(struct vhost_dev *dev)
 {
 	struct task_struct *worker;
-	int err;
+	int i, err;
 	/* Is there an owner already? */
 	if (dev->mm) {
 		err = -EBUSY;
@@ -258,6 +239,10 @@ static long vhost_dev_set_owner(struct vhost_dev *dev)
 	err = cgroup_attach_task_current_cg(worker);
 	if (err)
 		goto err_cgroup;
+
+	for (i = 0; i < dev->nvqs; ++i) {
+		vhost_work_init(&dev->vqs[i].work);
+	}
 	wake_up_process(worker);	/* avoid contributing to loadavg */
 
 	return 0;
@@ -293,7 +278,7 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
 {
 	int i;
 	for (i = 0; i < dev->nvqs; ++i) {
-		if (dev->vqs[i].kick && dev->vqs[i].handle_kick) {
+		if (dev->vqs[i].kick && dev->vqs[i].work.fn) {
 			vhost_poll_stop(&dev->vqs[i].poll);
 			vhost_poll_flush(&dev->vqs[i].poll);
 		}
@@ -322,7 +307,6 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
 		mmput(dev->mm);
 	dev->mm = NULL;
 
-	WARN_ON(!list_empty(&dev->work_list));
 	kthread_stop(dev->worker);
 }
 
@@ -644,7 +628,7 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
 		r = -ENOIOCTLCMD;
 	}
 
-	if (pollstop && vq->handle_kick)
+	if (pollstop && vq->work.fn)
 		vhost_poll_stop(&vq->poll);
 
 	if (ctx)
@@ -652,12 +636,12 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
 	if (filep)
 		fput(filep);
 
-	if (pollstart && vq->handle_kick)
+	if (pollstart && vq->work.fn)
 		vhost_poll_start(&vq->poll, vq->kick);
 
 	mutex_unlock(&vq->mutex);
 
-	if (pollstop && vq->handle_kick)
+	if (pollstop && vq->work.fn)
 		vhost_poll_flush(&vq->poll);
 	return r;
 }
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index afd7729..9c990ea 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -11,9 +11,10 @@
 #include <linux/uio.h>
 #include <linux/virtio_config.h>
 #include <linux/virtio_ring.h>
+#include <linux/srcu.h>
 #include <asm/atomic.h>
 
-struct vhost_device;
+struct vhost_dev;
 
 enum {
 	/* Enough place for all fragments, head, and virtio net header. */
@@ -21,29 +22,33 @@ enum {
 };
 
 struct vhost_work;
-typedef void (*vhost_work_fn_t)(struct vhost_work *work);
+typedef void (*vhost_work_fn_t)(struct vhost_dev *dev);
 
 struct vhost_work {
-	struct list_head	  node;
+	/* Callback function to execute. */
 	vhost_work_fn_t		  fn;
-	wait_queue_head_t	  done;
-	int			  flushing;
-	unsigned		  queue_seq;
-	unsigned		  done_seq;
+	/* Incremented to request callback execution.
+	 * Atomic to allow multiple writers. */
+	atomic_t		  queue_seq;
+	/* Used by worker to track execution requests.
+	 * Used from a single thread so no locking. */
+	int			  done_seq;
 };
 
+void vhost_work_set_fn(struct vhost_work *work, vhost_work_fn_t fn);
+
 /* Poll a file (eventfd or socket) */
 /* Note: there's nothing vhost specific about this structure. */
 struct vhost_poll {
 	poll_table                table;
 	wait_queue_head_t        *wqh;
 	wait_queue_t              wait;
-	struct vhost_work	  work;
+	struct vhost_work	 *work;
 	unsigned long		  mask;
 	struct vhost_dev	 *dev;
 };
 
-void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
+void vhost_poll_init(struct vhost_poll *poll, struct vhost_work* work,
 		     unsigned long mask, struct vhost_dev *dev);
 void vhost_poll_start(struct vhost_poll *poll, struct file *file);
 void vhost_poll_stop(struct vhost_poll *poll);
@@ -72,11 +77,12 @@ struct vhost_virtqueue {
 	struct eventfd_ctx *error_ctx;
 	struct eventfd_ctx *log_ctx;
 
+	/* The work to execute when the Guest kicks us,
+	 * on Host activity, or timeout. */
+	struct vhost_work work;
+	/* Poll Guest for kicks */
 	struct vhost_poll poll;
 
-	/* The routine to call when the Guest pings us, or timeout. */
-	vhost_work_fn_t handle_kick;
-
 	/* Last available index we saw. */
 	u16 last_avail_idx;
 
@@ -99,12 +105,7 @@ struct vhost_virtqueue {
 	size_t vhost_hlen;
 	size_t sock_hlen;
 	struct vring_used_elem heads[VHOST_NET_MAX_SG];
-	/* We use a kind of RCU to access private pointer.
-	 * All readers access it from worker, which makes it possible to
-	 * flush the vhost_work instead of synchronize_rcu. Therefore readers do
-	 * not need to call rcu_read_lock/rcu_read_unlock: the beginning of
-	 * vhost_work execution acts instead of rcu_read_lock() and the end of
-	 * vhost_work execution acts instead of rcu_read_lock().
+	/* Readers use worker_srcu in device to access private pointer.
 	 * Writers use virtqueue mutex. */
 	void *private_data;
 	/* Log write descriptors */
@@ -112,6 +113,12 @@ struct vhost_virtqueue {
 	struct vhost_log log[VHOST_NET_MAX_SG];
 };
 
+static inline void *vhost_vq_data(struct vhost_virtqueue *vq,
+				  struct vhost_dev *dev)
+{
+	return srcu_dereference(vq->private_data, &dev->worker_srcu);
+}
+
 struct vhost_dev {
 	/* Readers use RCU to access memory table pointer
 	 * log base pointer and features.
@@ -124,12 +131,12 @@ struct vhost_dev {
 	int nvqs;
 	struct file *log_file;
 	struct eventfd_ctx *log_ctx;
-	spinlock_t work_lock;
-	struct list_head work_list;
 	struct task_struct *worker;
+	struct srcu_struct worker_srcu;
 };
 
 long vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue *vqs, int nvqs);
+void vhost_dev_free(struct vhost_dev *);
 long vhost_dev_check_owner(struct vhost_dev *);
 long vhost_dev_reset_owner(struct vhost_dev *);
 void vhost_dev_cleanup(struct vhost_dev *);
-- 
1.7.2.rc0.14.g41c1c

^ permalink raw reply related

* Re: [PATCH] vhost: locking/rcu cleanup
From: Tejun Heo @ 2010-07-30 14:49 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: David S. Miller, Sridhar Samudrala, Jeff Dike, Juan Quintela,
	Rusty Russell, Takuya Yoshikawa, David Stevens, Paul E. McKenney,
	kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20100729122325.GA24337@redhat.com>

Hello,

On 07/29/2010 02:23 PM, Michael S. Tsirkin wrote:
> I saw WARN_ON(!list_empty(&dev->work_list)) trigger
> so our custom flush is not as airtight as need be.

Could be but it's also possible that something has queued something
after the last flush?  Is the problem reproducible?

> This patch switches to a simple atomic counter + srcu instead of
> the custom locked queue + flush implementation.
> 
> This will slow down the setup ioctls, which should not matter -
> it's slow path anyway. We use the expedited flush to at least
> make sure it has a sane time bound.
> 
> Works fine for me. I got reports that with many guests,
> work lock is highly contended, and this patch should in theory
> fix this as well - but I haven't tested this yet.

Hmmm... vhost_poll_flush() becomes synchronize_srcu_expedited().  Can
you please explain how it works?  synchronize_srcu_expedited() is an
extremely heavy operation involving scheduling the cpu_stop task on
all cpus.  I'm not quite sure whether doing it from every flush is a
good idea.  Is flush supposed to be a very rare operation?

Having custom implementation is fine too but let's try to implement
something generic if at all possible.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] vhost: locking/rcu cleanup
From: Michael S. Tsirkin @ 2010-08-01  8:41 UTC (permalink / raw)
  To: Tejun Heo
  Cc: David S. Miller, Sridhar Samudrala, Jeff Dike, Juan Quintela,
	Rusty Russell, Takuya Yoshikawa, David Stevens, Paul E. McKenney,
	kvm, virtualization, netdev, linux-kernel
In-Reply-To: <4C52E692.3070405@kernel.org>

On Fri, Jul 30, 2010 at 04:49:54PM +0200, Tejun Heo wrote:
> Hello,
> 
> On 07/29/2010 02:23 PM, Michael S. Tsirkin wrote:
> > I saw WARN_ON(!list_empty(&dev->work_list)) trigger
> > so our custom flush is not as airtight as need be.
> 
> Could be but it's also possible that something has queued something
> after the last flush?
> Is the problem reproducible?

Well, We do requeue from the job itself. So need to be careful with what
we do with indexes here. Bug seemed to happen all the time when qemu was
killed under stress but now I can't reproduce anymore :(
Will try again later.

> > This patch switches to a simple atomic counter + srcu instead of
> > the custom locked queue + flush implementation.
> > 
> > This will slow down the setup ioctls, which should not matter -
> > it's slow path anyway. We use the expedited flush to at least
> > make sure it has a sane time bound.
> > 
> > Works fine for me. I got reports that with many guests,
> > work lock is highly contended, and this patch should in theory
> > fix this as well - but I haven't tested this yet.
> 
> Hmmm... vhost_poll_flush() becomes synchronize_srcu_expedited().  Can
> you please explain how it works?  synchronize_srcu_expedited() is an
> extremely heavy operation involving scheduling the cpu_stop task on
> all cpus.  I'm not quite sure whether doing it from every flush is a
> good idea.  Is flush supposed to be a very rare operation?

It is rare - happens on guest reboot typically. I guess I will
switch to regular synchronize_srcu.

> Having custom implementation is fine too but let's try to implement
> something generic if at all possible.
> 
> Thanks.

Sure. It does seem that avoiding list lock would be pretty hard
in generic code though.

> -- 
> tejun

^ permalink raw reply

* [PATCH 1/1] staging: hv: Fix missing functions for net_device_ops
From: Haiyang Zhang @ 2010-08-02 17:00 UTC (permalink / raw)
  To: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	"'virtualization@lists.osdl.org'" <virtualiz>

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

From: Haiyang Zhang <haiyangz@microsoft.com>

Subject: staging: hv: Fix missing functions for net_device_ops
  Unlike in older kernels, ether_setup() no longer sets default
functions. We need to put them into net_device_ops. For example,
eth_change_mtu prevents setting MTU beyond 1500. This is necessary
because we don't support jumbo frame at this time.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>

---
 drivers/staging/hv/netvsc_drv.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 56e1157..64a0114 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -327,6 +327,9 @@ static const struct net_device_ops device_ops = {
 	.ndo_stop =			netvsc_close,
 	.ndo_start_xmit =		netvsc_start_xmit,
 	.ndo_set_multicast_list =	netvsc_set_multicast_list,
+	.ndo_change_mtu =		eth_change_mtu,
+	.ndo_validate_addr =		eth_validate_addr,
+	.ndo_set_mac_address =		eth_mac_addr,
 };
 
 static int netvsc_probe(struct device *device)
-- 
1.6.3.2


[-- Attachment #2: 0729-fix-missing-functions-for-net_device_ops.patch --]
[-- Type: application/octet-stream, Size: 1131 bytes --]

From: Haiyang Zhang <haiyangz@microsoft.com>

Subject: staging: hv: Fix missing functions for net_device_ops
  Unlike in older kernels, ether_setup() no longer sets default
functions. We need to put them into net_device_ops. For example,
eth_change_mtu prevents setting MTU beyond 1500. This is necessary
because we don't support jumbo frame at this time.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>

---
 drivers/staging/hv/netvsc_drv.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 56e1157..64a0114 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -327,6 +327,9 @@ static const struct net_device_ops device_ops = {
 	.ndo_stop =			netvsc_close,
 	.ndo_start_xmit =		netvsc_start_xmit,
 	.ndo_set_multicast_list =	netvsc_set_multicast_list,
+	.ndo_change_mtu =		eth_change_mtu,
+	.ndo_validate_addr =		eth_validate_addr,
+	.ndo_set_mac_address =		eth_mac_addr,
 };
 
 static int netvsc_probe(struct device *device)
-- 
1.6.3.2


[-- Attachment #3: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

^ permalink raw reply related

* Re: [PATCH 1/1] staging: hv: Fix missing functions for net_device_ops
From: Greg KH @ 2010-08-02 17:17 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org', Hank Janssen
In-Reply-To: <1FB5E1D5CA062146B38059374562DF7280E8547C@TK5EX14MBXC121.redmond.corp.microsoft.com>

On Mon, Aug 02, 2010 at 05:00:33PM +0000, Haiyang Zhang wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> Subject: staging: hv: Fix missing functions for net_device_ops
>   Unlike in older kernels, ether_setup() no longer sets default
> functions.

So what kernel versions are affected by this?

Please don't attach a patch and put it inline, it forces me to edit it
by hand.

Care to provide the above information and resend it so I can apply it?

thanks,

greg k-h

^ permalink raw reply

* RE: [PATCH 1/1] staging: hv: Fix missing functions for net_device_ops
From: Haiyang Zhang @ 2010-08-02 17:41 UTC (permalink / raw)
  To: Greg KH
  Cc: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org', Hank Janssen
In-Reply-To: <20100802171754.GA8213@suse.de>

> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Monday, August 02, 2010 1:18 PM
>
> So what kernel versions are affected by this?
> 
> Please don't attach a patch and put it inline, it forces me to edit it
> by hand.
> 
> Care to provide the above information and resend it so I can apply it?

It affects kernel 2.6.32 and later, which are all versions contain Hyper-V drivers.
I will send out an email with the inline patch only.

Thanks,

- Haiyang

^ permalink raw reply

* [PATCH 1/1] staging: hv: Fix missing functions for net_device_ops (resubmit)
From: Haiyang Zhang @ 2010-08-02 17:46 UTC (permalink / raw)
  To: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	"'virtualization@lists.osdl.org'" <virtualiz>

From: Haiyang Zhang <haiyangz@microsoft.com>

Subject: staging: hv: Fix missing functions for net_device_ops
  Unlike in older kernels, ether_setup() no longer sets default
functions. We need to put them into net_device_ops. For example,
eth_change_mtu prevents setting MTU beyond 1500. This is necessary
because we don't support jumbo frame at this time.
  It affects kernel 2.6.32 and later, which are all versions
containing Hyper-V drivers.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>

---
 drivers/staging/hv/netvsc_drv.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 56e1157..64a0114 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -327,6 +327,9 @@ static const struct net_device_ops device_ops = {
 	.ndo_stop =			netvsc_close,
 	.ndo_start_xmit =		netvsc_start_xmit,
 	.ndo_set_multicast_list =	netvsc_set_multicast_list,
+	.ndo_change_mtu =		eth_change_mtu,
+	.ndo_validate_addr =		eth_validate_addr,
+	.ndo_set_mac_address =		eth_mac_addr,
 };
 
 static int netvsc_probe(struct device *device)
-- 
1.6.3.2

^ permalink raw reply related

* Re: [GIT PULL net-next-2.6] vhost-net patchset for 2.6.36
From: David Miller @ 2010-08-02 22:54 UTC (permalink / raw)
  To: mst; +Cc: kvm, virtualization, netdev, linux-kernel, krkumar2
In-Reply-To: <20100728133231.GA1932@redhat.com>

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Wed, 28 Jul 2010 16:32:31 +0300

> The following changes since commit 4cfa580e7eebb8694b875d2caff3b989ada2efac:
> 
>   r6040: Fix args to phy_mii_ioctl(). (2010-07-21 21:10:49 -0700)
> 
> are available in the git repository at:
>   git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net-next
> 

Pulled, thanks Michael.

^ permalink raw reply

* Re: [PATCH 1/1] staging: hv: Fix missing functions for net_device_ops (resubmit)
From: Greg KH @ 2010-08-03  1:30 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org', 'gregkh@suse.de',
	Hank Janssen
In-Reply-To: <1FB5E1D5CA062146B38059374562DF7280E8556B@TK5EX14MBXC121.redmond.corp.microsoft.com>

On Mon, Aug 02, 2010 at 05:46:01PM +0000, Haiyang Zhang wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> Subject: staging: hv: Fix missing functions for net_device_ops
>   Unlike in older kernels, ether_setup() no longer sets default

Why put the subject in the body of the email?  This forces me to edit it
again :(

> functions. We need to put them into net_device_ops. For example,
> eth_change_mtu prevents setting MTU beyond 1500. This is necessary
> because we don't support jumbo frame at this time.

What does this mean?  Is it a bug?  Just not going as fast as possible?
Something else?

>   It affects kernel 2.6.32 and later, which are all versions
> containing Hyper-V drivers.

So is this fixing a bug that needs to be backported to these older
kernels (i.e. the staging tree?)  Is this something that is causing
people problems?

Third time's a charm?

thanks,

greg k-h

^ permalink raw reply

* Re: [E1000-eedc] [PATCH 11/17] ECP implementation
From: John Fastabend @ 2010-08-03  6:36 UTC (permalink / raw)
  To: Jens Osterkamp
  Cc: chrisw@redhat.com, evb@yahoogroups.com,
	e1000-eedc@lists.sourceforge.net,
	virtualization@lists.linux-foundation.org
In-Reply-To: <1279881273-10261-12-git-send-email-jens@linux.vnet.ibm.com>

Jens Osterkamp wrote:
> This is the implementation of the edge control protocol (ECP) as specified
> in IEEE 802.1Qbg.
> 
> For this it extends the infrastructure defined lldpad to send and receive
> ECP frames with a new (yet to be defined) ethertype.
> Received frames are validated and analyzed before the content is handed to the
> upper layer protocol (ULP, VDP in this case) for further processing. Frames
> to be transmitted are compiled from VSI (guest interface) profiles registered
> on a interface.
> Reception and transmission of ECP frames is controlled by RX and TX state
> machines, timeouts are handled timeout functions.
> The patch still contains a lot of debug code to allow low-level protocol
> analysis.
> 
> Signed-off-by: Jens Osterkamp <jens@linux.vnet.ibm.com>
> ---
>  Makefile.am        |    2 +
>  ecp/ecp.c          |   77 +++++++
>  ecp/ecp.h          |   92 ++++++++
>  ecp/ecp_rx.c       |  597 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  ecp/ecp_tx.c       |  467 ++++++++++++++++++++++++++++++++++++++++
>  include/lldp_evb.h |    6 +
>  include/lldp_vdp.h |  157 ++++++++++++++
>  lldp/l2_packet.h   |    2 +
>  lldp/ports.h       |   25 ++-
>  lldp_evb.c         |    2 +
>  10 files changed, 1424 insertions(+), 3 deletions(-)
>  create mode 100644 ecp/ecp.c
>  create mode 100644 ecp/ecp.h
>  create mode 100644 ecp/ecp_rx.c
>  create mode 100644 ecp/ecp_tx.c
>  create mode 100644 include/lldp_vdp.h
> 

snip

> +
> +struct vdp_user_data {
> +       LIST_HEAD(vdp_head, vdp_data) head;
> +};
> +
> +struct lldp_module *vdp_register(void);
> +void vdp_unregister(struct lldp_module *mod);
> +struct vdp_data *vdp_data(char *ifname);
> +struct packed_tlv *vdp_gettlv(struct port *port, struct vsi_profile *profile);
> +void vdp_vsi_sm_station(struct vsi_profile *profile);
> +struct vsi_profile *vdp_add_profile(struct vsi_profile *profile);
> +
> +#define MAC_ADDR_STRLEN                18
> +#define INSTANCE_STRLEN                32
> +
> +#define PRINT_PROFILE(s, p)    \
> +{ int c; \
> +  c = sprintf(s, "\nmode: %i\n", p->mode); s += c; \
> +  c = sprintf(s, "response: %i\n", p->response); s += c; \
> +  c = sprintf(s, "state: %i\n", p->state); s += c; \
> +  c = sprintf(s, "mgrid: %i\n", p->mgrid); s += c; \
> +  c = sprintf(s, "id: %x%x%x\n", p->id[2], p->id[1], p->id[0]); \
> +  s += c; \
> +  c = sprintf(s, "version: %i\n", p->version); s += c; \
> +  char instance[INSTANCE_STRLEN+2]; \
> +  instance2str(p->instance, instance, sizeof(instance)); \
> +  c = sprintf(s, "instance: %s\n", &instance); s += c; \
> +  char macbuf[MAC_ADDR_STRLEN+1]; \
> +  mac2str(p->mac, macbuf, MAC_ADDR_STRLEN); \
> +  c = sprintf(s, "mac: %s\n", macbuf); s += c; \
> +  c = sprintf(s, "vlan: %i\n\n", p->vlan); s += c; \
> +}
> +
> +#endif /* _LLDP_VDP_H */
> diff --git a/lldp/l2_packet.h b/lldp/l2_packet.h
> index 16f3683..0962429 100644
> --- a/lldp/l2_packet.h
> +++ b/lldp/l2_packet.h
> @@ -36,6 +36,8 @@
> 
>  #define ETH_P_LLDP 0x88cc
> 
> +/* TODO: use extended ethertype until final ethertype is available */
> +#define ETH_P_ECP 0x88b7
> 
>  #define ETH_FRAME_LEN   1514
> 
> diff --git a/lldp/ports.h b/lldp/ports.h
> index 0138efe..c2e18ec 100644
> --- a/lldp/ports.h
> +++ b/lldp/ports.h
> @@ -136,21 +136,40 @@ struct porttlvs{
>         struct unpacked_tlv *last_peer;
>  };
> 
> +struct ecp {
> +       struct l2_packet_data *l2;
> +       int sequence;
> +       int retries;
> +       int ackReceived;
> +       int ackTimerExpired;
> +       u16 lastSequence;
> +       u16 seqECPDU;
> +       struct portrx rx;
> +       struct porttx tx;
> +       struct portstats stats;
> +};
> +


This structure is ecp specific and should be part of the ecp module.  Not in 
ports.h.

>  struct port {
>         char *ifname;
>         u8 hw_resetting;
>         u8 portEnabled;
>         u8 prevPortEnabled;
>         u8 adminStatus;
> -       u8 rxChanges;
> -       u16   lldpdu;
> +
> +       /* protocol specific */
>         struct l2_packet_data *l2;
>         struct portrx rx;
>         struct porttx tx;
> -       struct porttlvs tlvs;
>         struct portstats stats;
>         struct porttimers timers;
> +       u8 rxChanges;
> +       u16   lldpdu;
>         struct msap msap;
> +
> +       /* not sure */
> +       struct porttlvs tlvs;
> +
> +       struct ecp ecp;
>         struct port *next;
>  };
> 

This adds ecp knowledge into the generic port structure I want to keep the port 
structure module agnostic.  dcbx addresses the same issue by using the module 
data space.  evb should use a similar scheme and create this structure at 
ifup(). Any reason why this can't work?

> diff --git a/lldp_evb.c b/lldp_evb.c
> index bce01b6..b1b7edc 100644
> --- a/lldp_evb.c
> +++ b/lldp_evb.c
> @@ -354,6 +354,8 @@ static void evb_statemachine(struct evb_data *ed, struct tlv_info_evb *tie)
>                  * different parameters ? Check parameters. switch state back to
>                  * EVB_CONFIGURE ? */
>                 printf("%s: state -> EVB_CONFIRMATION\n", __func__);
> +               if (ed->tie->scap & LLDP_EVB_CAPABILITY_PROTOCOL_ECP)
> +                       ecp_init(ed->ifname);
>                 break;
>         default:
>                 fprintf(stderr, "EVB statemachine reached invalid state !\n");
> --
> 1.7.1
> 
> 
> ------------------------------------------------------------------------------
> The Palm PDK Hot Apps Program offers developers who use the
> Plug-In Development Kit to bring their C/C++ apps to Palm for a share
> of $1 Million in cash or HP Products. Visit us here for more details:
> http://ad.doubleclick.net/clk;226879339;13503038;l?
> http://clk.atdmt.com/CRS/go/247765532/direct/01/
> _______________________________________________
> E1000-eedc mailing list
> E1000-eedc@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/e1000-eedc

^ permalink raw reply

* Re: [E1000-eedc] [PATCH 16/17] lldpad support for libvirt netlink message
From: John Fastabend @ 2010-08-03  7:52 UTC (permalink / raw)
  To: Jens Osterkamp
  Cc: chrisw@redhat.com, Stefan Berger,
	virtualization@lists.linux-foundation.org, Gerhard Stenzel,
	evb@yahoogroups.com, e1000-eedc@lists.sourceforge.net
In-Reply-To: <1279881273-10261-17-git-send-email-jens@linux.vnet.ibm.com>

Jens Osterkamp wrote:
> This code receives a IEEE 802.1Qbg virtual station instance from libvirt in
> a SETLINK message. The parsed VSI is then handed over to VDP for processing.
> The VDP state machine processes the VSI while libvirt polls the result using
> GETLINK.
> 
> Requires at least Linux kernel 2.6.35-rc1.
> 
> Signed-off-by: Jens Osterkamp <jens@linux.vnet.ibm.com>
> Signed-off-by: Gerhard Stenzel <gstenzel@linux.vnet.ibm.com>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  event_iface.c |  617 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  lldp_vdp.c    |    2 +-
>  lldpad.c      |   10 +
>  3 files changed, 622 insertions(+), 7 deletions(-)
> 

Jens,

Can you explain why this interface is needed?  There is already a kernel 
interface for this how come libvirt and lldpad can not use the kernel interface?

Thanks,
John.

^ permalink raw reply

* Re: [E1000-eedc] [PATCH 16/17] lldpad support for libvirt netlink message
From: Jens Osterkamp @ 2010-08-03 11:59 UTC (permalink / raw)
  To: John Fastabend
  Cc: chrisw@redhat.com, arnd, Stefan Berger,
	virtualization@lists.linux-foundation.org, Gerhard Stenzel,
	evb@yahoogroups.com, e1000-eedc@lists.sourceforge.net
In-Reply-To: <4C57CACE.2070202@intel.com>

On Tuesday 03 August 2010, John Fastabend wrote:
> Jens Osterkamp wrote:
> > This code receives a IEEE 802.1Qbg virtual station instance from libvirt in
> > a SETLINK message. The parsed VSI is then handed over to VDP for processing.
> > The VDP state machine processes the VSI while libvirt polls the result using
> > GETLINK.
> > 
> > Requires at least Linux kernel 2.6.35-rc1.
> > 
> > Signed-off-by: Jens Osterkamp <jens@linux.vnet.ibm.com>
> > Signed-off-by: Gerhard Stenzel <gstenzel@linux.vnet.ibm.com>
> > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > ---
> >  event_iface.c |  617 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  lldp_vdp.c    |    2 +-
> >  lldpad.c      |   10 +
> >  3 files changed, 622 insertions(+), 7 deletions(-)
> > 
> 
> Jens,
> 
> Can you explain why this interface is needed?  There is already a kernel 

We needed a way in libvirt to associate the virtual network interface(s) of a
guest with an adjacent switch. The protocol to do this could be implemented
both in userspace (e.g. in lldpad) or in the adapter firmware. You could have
different drivers for both ways in libvirt but to keep it simple we decided
to use netlink for both.

The original mailthread on netdev started here

http://marc.info/?l=linux-netdev&m=127242976205746&w=2

> interface for this how come libvirt and lldpad can not use the kernel interface?

In fact we are using the kernel interface for this, e.g. the definitions of the
netlink attributes are in the kernel header files.
The only difference is that in the userspace case, the netlink messages
are addressed to the pid of lldpad whereas in the firmware case, the netlink
messages go to pid 0 (kernel).

Best regards, 

Jens Osterkamp
--------------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

^ permalink raw reply

* Re: [E1000-eedc] [PATCH 11/17] ECP implementation
From: Jens Osterkamp @ 2010-08-03 12:02 UTC (permalink / raw)
  To: John Fastabend
  Cc: chrisw@redhat.com, evb@yahoogroups.com,
	e1000-eedc@lists.sourceforge.net,
	virtualization@lists.linux-foundation.org
In-Reply-To: <4C57B8EC.4000604@intel.com>

On Tuesday 03 August 2010, John Fastabend wrote:
> Jens Osterkamp wrote:
> > This is the implementation of the edge control protocol (ECP) as specified
> > in IEEE 802.1Qbg.
> > 
> > For this it extends the infrastructure defined lldpad to send and receive
> > ECP frames with a new (yet to be defined) ethertype.
> > Received frames are validated and analyzed before the content is handed to the
> > upper layer protocol (ULP, VDP in this case) for further processing. Frames
> > to be transmitted are compiled from VSI (guest interface) profiles registered
> > on a interface.
> > Reception and transmission of ECP frames is controlled by RX and TX state
> > machines, timeouts are handled timeout functions.
> > The patch still contains a lot of debug code to allow low-level protocol
> > analysis.
> > 
> > Signed-off-by: Jens Osterkamp <jens@linux.vnet.ibm.com>
> > ---
> >  Makefile.am        |    2 +
> >  ecp/ecp.c          |   77 +++++++
> >  ecp/ecp.h          |   92 ++++++++
> >  ecp/ecp_rx.c       |  597 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  ecp/ecp_tx.c       |  467 ++++++++++++++++++++++++++++++++++++++++
> >  include/lldp_evb.h |    6 +
> >  include/lldp_vdp.h |  157 ++++++++++++++
> >  lldp/l2_packet.h   |    2 +
> >  lldp/ports.h       |   25 ++-
> >  lldp_evb.c         |    2 +
> >  10 files changed, 1424 insertions(+), 3 deletions(-)
> >  create mode 100644 ecp/ecp.c
> >  create mode 100644 ecp/ecp.h
> >  create mode 100644 ecp/ecp_rx.c
> >  create mode 100644 ecp/ecp_tx.c
> >  create mode 100644 include/lldp_vdp.h
> > 
> 
> snip
> 
> > +
> > +struct vdp_user_data {
> > +       LIST_HEAD(vdp_head, vdp_data) head;
> > +};
> > +
> > +struct lldp_module *vdp_register(void);
> > +void vdp_unregister(struct lldp_module *mod);
> > +struct vdp_data *vdp_data(char *ifname);
> > +struct packed_tlv *vdp_gettlv(struct port *port, struct vsi_profile *profile);
> > +void vdp_vsi_sm_station(struct vsi_profile *profile);
> > +struct vsi_profile *vdp_add_profile(struct vsi_profile *profile);
> > +
> > +#define MAC_ADDR_STRLEN                18
> > +#define INSTANCE_STRLEN                32
> > +
> > +#define PRINT_PROFILE(s, p)    \
> > +{ int c; \
> > +  c = sprintf(s, "\nmode: %i\n", p->mode); s += c; \
> > +  c = sprintf(s, "response: %i\n", p->response); s += c; \
> > +  c = sprintf(s, "state: %i\n", p->state); s += c; \
> > +  c = sprintf(s, "mgrid: %i\n", p->mgrid); s += c; \
> > +  c = sprintf(s, "id: %x%x%x\n", p->id[2], p->id[1], p->id[0]); \
> > +  s += c; \
> > +  c = sprintf(s, "version: %i\n", p->version); s += c; \
> > +  char instance[INSTANCE_STRLEN+2]; \
> > +  instance2str(p->instance, instance, sizeof(instance)); \
> > +  c = sprintf(s, "instance: %s\n", &instance); s += c; \
> > +  char macbuf[MAC_ADDR_STRLEN+1]; \
> > +  mac2str(p->mac, macbuf, MAC_ADDR_STRLEN); \
> > +  c = sprintf(s, "mac: %s\n", macbuf); s += c; \
> > +  c = sprintf(s, "vlan: %i\n\n", p->vlan); s += c; \
> > +}
> > +
> > +#endif /* _LLDP_VDP_H */
> > diff --git a/lldp/l2_packet.h b/lldp/l2_packet.h
> > index 16f3683..0962429 100644
> > --- a/lldp/l2_packet.h
> > +++ b/lldp/l2_packet.h
> > @@ -36,6 +36,8 @@
> > 
> >  #define ETH_P_LLDP 0x88cc
> > 
> > +/* TODO: use extended ethertype until final ethertype is available */
> > +#define ETH_P_ECP 0x88b7
> > 
> >  #define ETH_FRAME_LEN   1514
> > 
> > diff --git a/lldp/ports.h b/lldp/ports.h
> > index 0138efe..c2e18ec 100644
> > --- a/lldp/ports.h
> > +++ b/lldp/ports.h
> > @@ -136,21 +136,40 @@ struct porttlvs{
> >         struct unpacked_tlv *last_peer;
> >  };
> > 
> > +struct ecp {
> > +       struct l2_packet_data *l2;
> > +       int sequence;
> > +       int retries;
> > +       int ackReceived;
> > +       int ackTimerExpired;
> > +       u16 lastSequence;
> > +       u16 seqECPDU;
> > +       struct portrx rx;
> > +       struct porttx tx;
> > +       struct portstats stats;
> > +};
> > +
> 
> 
> This structure is ecp specific and should be part of the ecp module.  Not in 
> ports.h.

Right, I will move it.

> 
> >  struct port {
> >         char *ifname;
> >         u8 hw_resetting;
> >         u8 portEnabled;
> >         u8 prevPortEnabled;
> >         u8 adminStatus;
> > -       u8 rxChanges;
> > -       u16   lldpdu;
> > +
> > +       /* protocol specific */
> >         struct l2_packet_data *l2;
> >         struct portrx rx;
> >         struct porttx tx;
> > -       struct porttlvs tlvs;
> >         struct portstats stats;
> >         struct porttimers timers;
> > +       u8 rxChanges;
> > +       u16   lldpdu;
> >         struct msap msap;
> > +
> > +       /* not sure */
> > +       struct porttlvs tlvs;
> > +
> > +       struct ecp ecp;
> >         struct port *next;
> >  };
> > 
> 
> This adds ecp knowledge into the generic port structure I want to keep the port 
> structure module agnostic.  dcbx addresses the same issue by using the module 
> data space.  evb should use a similar scheme and create this structure at 
> ifup(). Any reason why this can't work?

I will look into it and check if I can move it as well.

> 
> > diff --git a/lldp_evb.c b/lldp_evb.c
> > index bce01b6..b1b7edc 100644
> > --- a/lldp_evb.c
> > +++ b/lldp_evb.c
> > @@ -354,6 +354,8 @@ static void evb_statemachine(struct evb_data *ed, struct tlv_info_evb *tie)
> >                  * different parameters ? Check parameters. switch state back to
> >                  * EVB_CONFIGURE ? */
> >                 printf("%s: state -> EVB_CONFIRMATION\n", __func__);
> > +               if (ed->tie->scap & LLDP_EVB_CAPABILITY_PROTOCOL_ECP)
> > +                       ecp_init(ed->ifname);
> >                 break;
> >         default:
> >                 fprintf(stderr, "EVB statemachine reached invalid state !\n");
> > --
> > 1.7.1
> > 
> > 
> > ------------------------------------------------------------------------------
> > The Palm PDK Hot Apps Program offers developers who use the
> > Plug-In Development Kit to bring their C/C++ apps to Palm for a share
> > of $1 Million in cash or HP Products. Visit us here for more details:
> > http://ad.doubleclick.net/clk;226879339;13503038;l?
> > http://clk.atdmt.com/CRS/go/247765532/direct/01/
> > _______________________________________________
> > E1000-eedc mailing list
> > E1000-eedc@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/e1000-eedc
> 
> 


-- 
Best regards, 

Jens Osterkamp
--------------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

^ permalink raw reply

* [PATCH 1/6] staging: hv: Fixed bounce kmap problem by using correct index
From: Hank Janssen @ 2010-08-03 17:31 UTC (permalink / raw)
  To: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org'
  Cc: Haiyang Zhang, 'gregkh@suse.de'

From: Hank Janssen <hjanssen@microsoft.com> 

Fixed bounce offset kmap problem by using correct index

Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com>

---
 drivers/staging/hv/storvsc_drv.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 075b61b..3b9ccb0 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -495,7 +495,7 @@ static unsigned int copy_to_bounce_buffer(struct scatterlist *orig_sgl,
 
 		/* ASSERT(orig_sgl[i].offset + orig_sgl[i].length <= PAGE_SIZE); */
 
-		if (j == 0)
+		if (bounce_addr == 0)
 			bounce_addr = (unsigned long)kmap_atomic(sg_page((&bounce_sgl[j])), KM_IRQ0);
 
 		while (srclen) {
@@ -556,7 +556,7 @@ static unsigned int copy_from_bounce_buffer(struct scatterlist *orig_sgl,
 		destlen = orig_sgl[i].length;
 		/* ASSERT(orig_sgl[i].offset + orig_sgl[i].length <= PAGE_SIZE); */
 
-		if (j == 0)
+		if (bounce_addr == 0)
 			bounce_addr = (unsigned long)kmap_atomic(sg_page((&bounce_sgl[j])), KM_IRQ0);
 
 		while (destlen) {
--
1.6.0.2

^ permalink raw reply related

* [PATCH 2/6] staging: hv: Fixed lockup problem with bounce_buffer scatter list
From: Hank Janssen @ 2010-08-03 17:31 UTC (permalink / raw)
  To: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org'
  Cc: Haiyang Zhang, 'gregkh@suse.de'

From: Hank Janssen <hjanssen@microsoft.com>

Fixed lockup problem with bounce_buffer scatter list which caused crashes in heavy loads.

Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com> 

---
 drivers/staging/hv/storvsc_drv.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 3b9ccb0..169d701 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -615,6 +615,7 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
 	unsigned int request_size = 0;
 	int i;
 	struct scatterlist *sgl;
+	unsigned int sg_count = 0;
 
 	DPRINT_DBG(STORVSC_DRV, "scmnd %p dir %d, use_sg %d buf %p len %d "
 		   "queue depth %d tagged %d", scmnd, scmnd->sc_data_direction, @@ -697,6 +698,7 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
 	request->DataBuffer.Length = scsi_bufflen(scmnd);
 	if (scsi_sg_count(scmnd)) {
 		sgl = (struct scatterlist *)scsi_sglist(scmnd);
+		sg_count = scsi_sg_count(scmnd);
 
 		/* check if we need to bounce the sgl */
 		if (do_bounce_buffer(sgl, scsi_sg_count(scmnd)) != -1) { @@ -731,15 +733,16 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
 					      scsi_sg_count(scmnd));
 
 			sgl = cmd_request->bounce_sgl;
+			sg_count = cmd_request->bounce_sgl_count;
 		}
 
 		request->DataBuffer.Offset = sgl[0].offset;
 
-		for (i = 0; i < scsi_sg_count(scmnd); i++) {
+		for (i = 0; i < sg_count; i++) {
 			DPRINT_DBG(STORVSC_DRV, "sgl[%d] len %d offset %d\n",
 				   i, sgl[i].length, sgl[i].offset);
-			request->DataBuffer.PfnArray[i] =
-					page_to_pfn(sg_page((&sgl[i])));
+			request->DataBuffer.PfnArray[i] = 
+				page_to_pfn(sg_page((&sgl[i])));
 		}
 	} else if (scsi_sglist(scmnd)) {
 		/* ASSERT(scsi_bufflen(scmnd) <= PAGE_SIZE); */
--
1.6.0.2

^ permalink raw reply related

* [PATCH 4/6] staging: hv: Removed TODO for VMBUS, it mirrors HYPER-V behavior to closely
From: Hank Janssen @ 2010-08-03 17:31 UTC (permalink / raw)
  To: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org'
  Cc: Haiyang Zhang, 'gregkh@suse.de'

From: Hank Janssen <hjanssen@microsoft.com>

Removed TODO for VMBUS, it mirrors HYPER-V behaviour to closely and does not really share any 
usable functionality of the Linux kernel facilities. There is not really an easy way to combine functionality.

Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com>

---
 drivers/staging/hv/TODO |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/hv/TODO b/drivers/staging/hv/TODO index 66a89c8..8ca2947 100644
--- a/drivers/staging/hv/TODO
+++ b/drivers/staging/hv/TODO
@@ -4,8 +4,6 @@ TODO:
 	  driver model
 	- convert vmbus driver interface function pointer tables
           to constant, a.k.a vmbus_ops
-	- see if the vmbus can be merged with the other virtual busses
-	  in the kernel
 	- audit the network driver
 	  - checking for carrier inside open is wrong, network device API
             confusion??
--
1.6.0.2

^ permalink raw reply

* [PATCH 5/6] staging: hv: Increased storvsc ringbuffer and max_io_requests
From: Hank Janssen @ 2010-08-03 17:31 UTC (permalink / raw)
  To: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org'
  Cc: Haiyang Zhang, 'gregkh@suse.de'

From: Hank Janssen <hjanssen@microsoft.com>

Increased storvsc ringbuffer and max_io_requests. This now more closely mimics 
the numbers on Hyper-V. And will allow more IO requests to take place for the SCSI driver.

Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com> 

---
 drivers/staging/hv/storvsc_api.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/hv/storvsc_api.h b/drivers/staging/hv/storvsc_api.h
index 0063bde..8505a1c 100644
--- a/drivers/staging/hv/storvsc_api.h
+++ b/drivers/staging/hv/storvsc_api.h
@@ -28,10 +28,10 @@
 #include "vmbus_api.h"
 
 /* Defines */
-#define STORVSC_RING_BUFFER_SIZE			(10*PAGE_SIZE)
+#define STORVSC_RING_BUFFER_SIZE			(20*PAGE_SIZE)
 #define BLKVSC_RING_BUFFER_SIZE				(20*PAGE_SIZE)
 
-#define STORVSC_MAX_IO_REQUESTS				64
+#define STORVSC_MAX_IO_REQUESTS				128
 
 /*
  * In Hyper-V, each port/path/target maps to 1 scsi host adapter.  In
--
1.6.0.2

^ permalink raw reply related

* [PATCH 6/6] staging: hv: Gracefully handle SCSI resets
From: Hank Janssen @ 2010-08-03 17:31 UTC (permalink / raw)
  To: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org'
  Cc: Haiyang Zhang, 'gregkh@suse.de'

From: Hank Janssen <hjanssen@microsoft.com>

If we get a SCSI host bus reset we now gracefully handle it, and we take the device offline. 
This before sometimes caused hangs.

Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com>


---
 drivers/staging/hv/storvsc.c |   36 +++++++++++++++++++++++++++++++++++-
 1 files changed, 35 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c index 6bd2ff1..5f222cf 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -48,7 +48,9 @@ struct storvsc_device {
 
 	/* 0 indicates the device is being destroyed */
 	atomic_t RefCount;
-
+	
+	int reset;
+	spinlock_t lock;
 	atomic_t NumOutstandingRequests;
 
 	/*
@@ -93,6 +95,9 @@ static inline struct storvsc_device *AllocStorDevice(struct hv_device *Device)
 	atomic_cmpxchg(&storDevice->RefCount, 0, 2);
 
 	storDevice->Device = Device;
+	storDevice->reset  = 0;
+	spin_lock_init(&storDevice->lock);
+
 	Device->Extension = storDevice;
 
 	return storDevice;
@@ -101,6 +106,7 @@ static inline struct storvsc_device *AllocStorDevice(struct hv_device *Device)  static inline void FreeStorDevice(struct storvsc_device *Device)  {
 	/* ASSERT(atomic_read(&Device->RefCount) == 0); */
+	/*kfree(Device->lock);*/
 	kfree(Device);
 }
 
@@ -108,13 +114,24 @@ static inline void FreeStorDevice(struct storvsc_device *Device)  static inline struct storvsc_device *GetStorDevice(struct hv_device *Device)  {
 	struct storvsc_device *storDevice;
+	unsigned long flags;
 
 	storDevice = (struct storvsc_device *)Device->Extension;
+
+	spin_lock_irqsave(&storDevice->lock, flags);
+
+	if (storDevice->reset == 1) {
+		spin_unlock_irqrestore(&storDevice->lock, flags);
+		return NULL;
+	}
+
 	if (storDevice && atomic_read(&storDevice->RefCount) > 1)
 		atomic_inc(&storDevice->RefCount);
 	else
 		storDevice = NULL;
 
+	spin_unlock_irqrestore(&storDevice->lock, flags);
+
 	return storDevice;
 }
 
@@ -122,13 +139,19 @@ static inline struct storvsc_device *GetStorDevice(struct hv_device *Device)  static inline struct storvsc_device *MustGetStorDevice(struct hv_device *Device)  {
 	struct storvsc_device *storDevice;
+	unsigned long flags;
 
 	storDevice = (struct storvsc_device *)Device->Extension;
+
+	spin_lock_irqsave(&storDevice->lock, flags);
+
 	if (storDevice && atomic_read(&storDevice->RefCount))
 		atomic_inc(&storDevice->RefCount);
 	else
 		storDevice = NULL;
 
+	spin_unlock_irqrestore(&storDevice->lock, flags);
+
 	return storDevice;
 }
 
@@ -614,6 +637,7 @@ int StorVscOnHostReset(struct hv_device *Device)
 	struct storvsc_device *storDevice;
 	struct storvsc_request_extension *request;
 	struct vstor_packet *vstorPacket;
+	unsigned long flags;
 	int ret;
 
 	DPRINT_INFO(STORVSC, "resetting host adapter..."); @@ -625,6 +649,16 @@ int StorVscOnHostReset(struct hv_device *Device)
 		return -1;
 	}
 
+	spin_lock_irqsave(&storDevice->lock, flags);
+	storDevice->reset = 1;
+	spin_unlock_irqrestore(&storDevice->lock, flags);
+
+	/*
+	 * Wait for traffic in transit to complete
+	 */
+	while (atomic_read(&storDevice->NumOutstandingRequests))
+		udelay(1000);
+
 	request = &storDevice->ResetRequest;
 	vstorPacket = &request->VStorPacket;
 
--
1.6.0.2

^ permalink raw reply

* [PATCH 3/6] staging: hv: Fixed the value of the 64bit-hole inside ring buffer
From: Hank Janssen @ 2010-08-03 17:31 UTC (permalink / raw)
  To: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org'
  Cc: Haiyang Zhang, 'gregkh@suse.de'

From: Haiyang Zhang <haiyangz@microsoft.com>

Fixed the value of the 64bit-hole inside ring buffer, this caused a problem on Hyper-V when running checked Windows builds.

Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com>

---
 drivers/staging/hv/ring_buffer.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/hv/ring_buffer.c b/drivers/staging/hv/ring_buffer.c
index 66a4974..133f909 100644
--- a/drivers/staging/hv/ring_buffer.c
+++ b/drivers/staging/hv/ring_buffer.c
@@ -192,8 +192,7 @@ Description:
 static inline u64
 GetRingBufferIndices(RING_BUFFER_INFO *RingInfo)  {
-	return ((u64)RingInfo->RingBuffer->WriteIndex << 32)
-	|| RingInfo->RingBuffer->ReadIndex;
+	return (u64)RingInfo->RingBuffer->WriteIndex << 32;
 }
 
 
--
1.6.0.2

^ permalink raw reply related

* Re: [PATCH 1/6] staging: hv: Fixed bounce kmap problem by using correct index
From: Greg KH @ 2010-08-03 17:37 UTC (permalink / raw)
  To: Hank Janssen
  Cc: 'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org', Haiyang Zhang,
	'linux-kernel@vger.kernel.org'
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FD0D21@TK5EX14MBXC114.redmond.corp.microsoft.com>

On Tue, Aug 03, 2010 at 05:31:51PM +0000, Hank Janssen wrote:
> From: Hank Janssen <hjanssen@microsoft.com> 
> 
> Fixed bounce offset kmap problem by using correct index

Is this a bugfix that needs to go to previous kernel releases?  If so,
which ones?  What is the symptom of the problem here?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 2/6] staging: hv: Fixed lockup problem with bounce_buffer scatter list
From: Greg KH @ 2010-08-03 17:38 UTC (permalink / raw)
  To: Hank Janssen
  Cc: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FD1D29@TK5EX14MBXC114.redmond.corp.microsoft.com>

On Tue, Aug 03, 2010 at 05:31:53PM +0000, Hank Janssen wrote:
> From: Hank Janssen <hjanssen@microsoft.com>
> 
> Fixed lockup problem with bounce_buffer scatter list which caused crashes in heavy loads.

Same questions for you as patch 1/6.

Also, please wrap your changelog comments at 72 columns.

> 
> Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
> Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com> 

What's with the trailing space?

> 
> ---
>  drivers/staging/hv/storvsc_drv.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
> index 3b9ccb0..169d701 100644
> --- a/drivers/staging/hv/storvsc_drv.c
> +++ b/drivers/staging/hv/storvsc_drv.c
> @@ -615,6 +615,7 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
>  	unsigned int request_size = 0;
>  	int i;
>  	struct scatterlist *sgl;
> +	unsigned int sg_count = 0;
>  
>  	DPRINT_DBG(STORVSC_DRV, "scmnd %p dir %d, use_sg %d buf %p len %d "
>  		   "queue depth %d tagged %d", scmnd, scmnd->sc_data_direction, @@ -697,6 +698,7 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
>  	request->DataBuffer.Length = scsi_bufflen(scmnd);
>  	if (scsi_sg_count(scmnd)) {
>  		sgl = (struct scatterlist *)scsi_sglist(scmnd);
> +		sg_count = scsi_sg_count(scmnd);
>  
>  		/* check if we need to bounce the sgl */
>  		if (do_bounce_buffer(sgl, scsi_sg_count(scmnd)) != -1) { @@ -731,15 +733,16 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
>  					      scsi_sg_count(scmnd));
>  
>  			sgl = cmd_request->bounce_sgl;
> +			sg_count = cmd_request->bounce_sgl_count;
>  		}
>  
>  		request->DataBuffer.Offset = sgl[0].offset;
>  
> -		for (i = 0; i < scsi_sg_count(scmnd); i++) {
> +		for (i = 0; i < sg_count; i++) {
>  			DPRINT_DBG(STORVSC_DRV, "sgl[%d] len %d offset %d\n",
>  				   i, sgl[i].length, sgl[i].offset);
> -			request->DataBuffer.PfnArray[i] =
> -					page_to_pfn(sg_page((&sgl[i])));
> +			request->DataBuffer.PfnArray[i] = 

Trailing space here?

You didn't run 'checkpatch.pl' now did you :)

You should set your editor to show up trailing spaces in bright red to
keep this from happening.  I can tell you how to do this in vim, all
other editors, you are on your own.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 5/6] staging: hv: Increased storvsc ringbuffer and max_io_requests
From: Greg KH @ 2010-08-03 17:40 UTC (permalink / raw)
  To: Hank Janssen
  Cc: 'linux-kernel@vger.kernel.org',
	'devel@driverdev.osuosl.org',
	'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FD1D36@TK5EX14MBXC114.redmond.corp.microsoft.com>

On Tue, Aug 03, 2010 at 05:31:55PM +0000, Hank Janssen wrote:
> From: Hank Janssen <hjanssen@microsoft.com>
> 
> Increased storvsc ringbuffer and max_io_requests. This now more closely mimics 
> the numbers on Hyper-V.

What do you mean by this?  The max provided by the hypervisor?

> And will allow more IO requests to take place for the SCSI driver.

Which means what, better throughput?  Or just more outstanding requests
and the same speed overall?

thanks,

greg k-h

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox