* Re: [PATCHv2 RFC 0/4] virtio and vhost-net capacity handling
From: Michael S. Tsirkin @ 2011-06-02 17:17 UTC (permalink / raw)
To: linux-kernel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, virtualization, steved,
Christian Borntraeger, Tom Lendacky, Martin Schwidefsky, linux390
In-Reply-To: <cover.1307029008.git.mst@redhat.com>
On Thu, Jun 02, 2011 at 06:42:35PM +0300, Michael S. Tsirkin wrote:
> OK, here's a new attempt to use the new capacity api. I also added more
> comments to clarify the logic. Hope this is more readable. Let me know
> pls.
>
> This is on top of the patches applied by Rusty.
>
> Warning: untested. Posting now to give people chance to
> comment on the API.
>
> Changes from v1:
> - fix comment in patch 2 to correct confusion noted by Rusty
> - rewrite patch 3 along the lines suggested by Rusty
> note: it's not exactly the same but I hope it's close
> enough, the main difference is that mine does limited
> polling even in the unlikely xmit failure case.
> - added a patch to not return capacity from add_buf
> it always looked like a weird hack
>
> Michael S. Tsirkin (4):
> virtio_ring: add capacity check API
> virtio_net: fix tx capacity checks using new API
> virtio_net: limit xmit polling
> Revert "virtio: make add_buf return capacity remaining:
>
> drivers/net/virtio_net.c | 111 ++++++++++++++++++++++++++----------------
> drivers/virtio/virtio_ring.c | 10 +++-
> include/linux/virtio.h | 7 ++-
> 3 files changed, 84 insertions(+), 44 deletions(-)
>
> --
> 1.7.5.53.gc233e
And just FYI, here's a patch (on top) that I considered but
decided against. This does a single get_buf before
xmit. I thought it's not really needed as the capacity
check in add_buf is relatively cheap, and we removed
the kick in xmit_skb. But the point is that the loop
will now be easy to move around if someone manages
to show this benefits speed (which I doubt).
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b25db1c..75af5be 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -590,6 +590,9 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
struct virtnet_info *vi = netdev_priv(dev);
int ret, i;
+ /* Try to pop an skb, to increase the chance we can add this one. */
+ free_old_xmit_skb(v);
+
/* We normally do have space in the ring, so try to queue the skb as
* fast as possible. */
ret = xmit_skb(vi, skb);
@@ -627,8 +630,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
* This is so that we don't hog the skb memory unnecessarily. *
* Doing this after kick means there's a chance we'll free
* the skb we have just sent, which is hot in cache. */
- for (i = 0; i < 2; i++)
- free_old_xmit_skb(v);
+ free_old_xmit_skb(v);
if (likely(free_xmit_capacity(vi)))
return NETDEV_TX_OK;
^ permalink raw reply related
* Re: [PATCHv2 RFC 3/4] virtio_net: limit xmit polling
From: Sridhar Samudrala @ 2011-06-02 18:09 UTC (permalink / raw)
To: Michael S. Tsirkin
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: <a80199422de16ae355e56ee1b2abc9b2bf91a7f6.1307029009.git.mst@redhat.com>
On Thu, 2011-06-02 at 18:43 +0300, Michael S. Tsirkin wrote:
> 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 | 106 +++++++++++++++++++++++++++++-----------------
> 1 files changed, 67 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index a0ee78d..b25db1c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -509,17 +509,33 @@ again:
> return received;
> }
>
> -static void free_old_xmit_skbs(struct virtnet_info *vi)
> +static bool free_old_xmit_skb(struct virtnet_info *vi)
> {
> struct sk_buff *skb;
> 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++;
> - dev_kfree_skb_any(skb);
> - }
> + skb = virtqueue_get_buf(vi->svq, &len);
> + if (unlikely(!skb))
> + return false;
> + 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 true;
> +}
> +
> +/* Check capacity and try to free enough pending old buffers to enable queueing
> + * new ones. Return true if we can guarantee that a following
> + * virtqueue_add_buf will succeed. */
> +static bool free_xmit_capacity(struct virtnet_info *vi)
> +{
> + struct sk_buff *skb;
> + unsigned int len;
> +
> + while (virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2)
> + if (unlikely(!free_old_xmit_skb))
> + return false;
If we are using INDIRECT descriptors, 1 descriptor entry is good enough
to guarantee that an skb can be queued unless we run out of memory.
Is it worth checking if 'indirect' is set on the svq and then only free
1 descriptor? Otherwise, we will be dropping the packet if there are
less than 18 free descriptors although we ony need 1.
> + return true;
> }
>
> static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
> @@ -572,30 +588,34 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
> 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);
> -
> - /* Try to transmit */
> - capacity = xmit_skb(vi, skb);
> -
> - /* This can happen with OOM and indirect buffers. */
> - if (unlikely(capacity < 0)) {
> - if (net_ratelimit()) {
> - if (likely(capacity == -ENOMEM)) {
> - dev_warn(&dev->dev,
> - "TX queue failure: out of memory\n");
> - } else {
> - dev->stats.tx_fifo_errors++;
> + int ret, i;
> +
> + /* We normally do have space in the ring, so try to queue the skb as
> + * fast as possible. */
> + ret = xmit_skb(vi, skb);
> + if (unlikely(ret < 0)) {
> + /* This triggers on the first xmit after ring full condition.
> + * We need to free up some skbs first. */
> + if (likely(free_xmit_capacity(vi))) {
> + ret = xmit_skb(vi, skb);
> + /* This should never fail. Check, just in case. */
> + if (unlikely(ret < 0)) {
> dev_warn(&dev->dev,
> "Unexpected TX queue failure: %d\n",
> - capacity);
> + ret);
> + dev->stats.tx_fifo_errors++;
> + dev->stats.tx_dropped++;
> + kfree_skb(skb);
> + return NETDEV_TX_OK;
> }
> + } else {
> + /* Ring full: it might happen if we get a callback while
> + * the queue is still mostly full. This should be
> + * extremely rare. */
> + dev->stats.tx_dropped++;
> + kfree_skb(skb);
> + goto stop;
> }
> - dev->stats.tx_dropped++;
> - kfree_skb(skb);
> - return NETDEV_TX_OK;
> }
> virtqueue_kick(vi->svq);
>
> @@ -603,18 +623,26 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> skb_orphan(skb);
> nf_reset(skb);
>
> - /* Apparently nice girls don't return TX_BUSY; stop the queue
> - * 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_delayed(vi->svq))) {
> - /* More just got used, free them then recheck. */
> - free_old_xmit_skbs(vi);
> - capacity = virtqueue_min_capacity(vi->svq);
> - if (capacity >= 2+MAX_SKB_FRAGS) {
> - netif_start_queue(dev);
> - virtqueue_disable_cb(vi->svq);
> - }
> + /* We transmit one skb, so try to free at least two pending skbs.
> + * This is so that we don't hog the skb memory unnecessarily. *
> + * Doing this after kick means there's a chance we'll free
> + * the skb we have just sent, which is hot in cache. */
> + for (i = 0; i < 2; i++)
> + free_old_xmit_skb(v);
> +
> + if (likely(free_xmit_capacity(vi)))
> + return NETDEV_TX_OK;
> +
> +stop:
> + /* Apparently nice girls don't return TX_BUSY; check capacity and stop
> + * the queue before it gets out of hand.
> + * Naturally, this wastes entries. */
> + netif_stop_queue(dev);
> + if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
> + /* More just got used, free them and recheck. */
> + if (free_xmit_capacity(vi)) {
> + netif_start_queue(dev);
> + virtqueue_disable_cb(vi->svq);
> }
> }
>
^ permalink raw reply
* Re: [PATCHv2 RFC 3/4] virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-06-02 19:23 UTC (permalink / raw)
To: Sridhar Samudrala
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: <1307038193.2321.38.camel@w-sridhar.beaverton.ibm.com>
On Thu, Jun 02, 2011 at 11:09:53AM -0700, Sridhar Samudrala wrote:
> On Thu, 2011-06-02 at 18:43 +0300, Michael S. Tsirkin wrote:
> > 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 | 106 +++++++++++++++++++++++++++++-----------------
> > 1 files changed, 67 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index a0ee78d..b25db1c 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -509,17 +509,33 @@ again:
> > return received;
> > }
> >
> > -static void free_old_xmit_skbs(struct virtnet_info *vi)
> > +static bool free_old_xmit_skb(struct virtnet_info *vi)
> > {
> > struct sk_buff *skb;
> > 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++;
> > - dev_kfree_skb_any(skb);
> > - }
> > + skb = virtqueue_get_buf(vi->svq, &len);
> > + if (unlikely(!skb))
> > + return false;
> > + 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 true;
> > +}
> > +
> > +/* Check capacity and try to free enough pending old buffers to enable queueing
> > + * new ones. Return true if we can guarantee that a following
> > + * virtqueue_add_buf will succeed. */
> > +static bool free_xmit_capacity(struct virtnet_info *vi)
> > +{
> > + struct sk_buff *skb;
> > + unsigned int len;
> > +
> > + while (virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2)
> > + if (unlikely(!free_old_xmit_skb))
> > + return false;
> If we are using INDIRECT descriptors, 1 descriptor entry is good enough
> to guarantee that an skb can be queued unless we run out of memory.
> Is it worth checking if 'indirect' is set on the svq and then only free
> 1 descriptor? Otherwise, we will be dropping the packet if there are
> less than 18 free descriptors although we ony need 1.
This is not introduced with this patch: the
issue is that we need to consider worst case
as indirect memory allocation can fail.
I don't think it matters much: 18 out of 256
descriptors is not too expensive.
> > + return true;
> > }
> >
> > static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
> > @@ -572,30 +588,34 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
> > 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);
> > -
> > - /* Try to transmit */
> > - capacity = xmit_skb(vi, skb);
> > -
> > - /* This can happen with OOM and indirect buffers. */
> > - if (unlikely(capacity < 0)) {
> > - if (net_ratelimit()) {
> > - if (likely(capacity == -ENOMEM)) {
> > - dev_warn(&dev->dev,
> > - "TX queue failure: out of memory\n");
> > - } else {
> > - dev->stats.tx_fifo_errors++;
> > + int ret, i;
> > +
> > + /* We normally do have space in the ring, so try to queue the skb as
> > + * fast as possible. */
> > + ret = xmit_skb(vi, skb);
> > + if (unlikely(ret < 0)) {
> > + /* This triggers on the first xmit after ring full condition.
> > + * We need to free up some skbs first. */
> > + if (likely(free_xmit_capacity(vi))) {
> > + ret = xmit_skb(vi, skb);
> > + /* This should never fail. Check, just in case. */
> > + if (unlikely(ret < 0)) {
> > dev_warn(&dev->dev,
> > "Unexpected TX queue failure: %d\n",
> > - capacity);
> > + ret);
> > + dev->stats.tx_fifo_errors++;
> > + dev->stats.tx_dropped++;
> > + kfree_skb(skb);
> > + return NETDEV_TX_OK;
> > }
> > + } else {
> > + /* Ring full: it might happen if we get a callback while
> > + * the queue is still mostly full. This should be
> > + * extremely rare. */
> > + dev->stats.tx_dropped++;
> > + kfree_skb(skb);
> > + goto stop;
> > }
> > - dev->stats.tx_dropped++;
> > - kfree_skb(skb);
> > - return NETDEV_TX_OK;
> > }
> > virtqueue_kick(vi->svq);
> >
> > @@ -603,18 +623,26 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> > skb_orphan(skb);
> > nf_reset(skb);
> >
> > - /* Apparently nice girls don't return TX_BUSY; stop the queue
> > - * 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_delayed(vi->svq))) {
> > - /* More just got used, free them then recheck. */
> > - free_old_xmit_skbs(vi);
> > - capacity = virtqueue_min_capacity(vi->svq);
> > - if (capacity >= 2+MAX_SKB_FRAGS) {
> > - netif_start_queue(dev);
> > - virtqueue_disable_cb(vi->svq);
> > - }
> > + /* We transmit one skb, so try to free at least two pending skbs.
> > + * This is so that we don't hog the skb memory unnecessarily. *
> > + * Doing this after kick means there's a chance we'll free
> > + * the skb we have just sent, which is hot in cache. */
> > + for (i = 0; i < 2; i++)
> > + free_old_xmit_skb(v);
> > +
> > + if (likely(free_xmit_capacity(vi)))
> > + return NETDEV_TX_OK;
> > +
> > +stop:
> > + /* Apparently nice girls don't return TX_BUSY; check capacity and stop
> > + * the queue before it gets out of hand.
> > + * Naturally, this wastes entries. */
> > + netif_stop_queue(dev);
> > + if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
> > + /* More just got used, free them and recheck. */
> > + if (free_xmit_capacity(vi)) {
> > + netif_start_queue(dev);
> > + virtqueue_disable_cb(vi->svq);
> > }
> > }
> >
^ permalink raw reply
* Re: [PATCH RFC 3/3] virtio_net: limit xmit polling
From: Krishna Kumar2 @ 2011-06-03 4:08 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: habanero, lguest, Shirley Ma, kvm, Carsten Otte, linux-s390,
Heiko Carstens, linux-kernel, virtualization, steved,
Christian Borntraeger, Tom Lendacky, netdev, Martin Schwidefsky,
linux390
In-Reply-To: <20110602153423.GA11300@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> wrote on 06/02/2011 09:04:23 PM:
> > > Is this where the bug was?
> >
> > Return value in free_old_xmit() was wrong. I will re-do against the
> > mainline kernel.
> >
> > Thanks,
> >
> > - KK
>
> Just noting that I'm working on that patch as well, it might
> be more efficient if we don't both of us do this in parallel :)
OK, but my intention was to work on a alternate approach, which
was the reason to base it against your patch.
I will check your latest patch.
thanks,
- KK
^ permalink raw reply
* [patch] xen: off by one errors in multicalls.c
From: Dan Carpenter @ 2011-06-03 4:45 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: open list:XEN HYPERVISOR IN..., Konrad Rzeszutek Wilk,
maintainer:X86 ARCHITECTURE..., kernel-janitors,
open list:XEN HYPERVISOR IN..., Ingo Molnar, H. Peter Anvin,
Thomas Gleixner
b->args[] has MC_ARGS elements, so the comparison here should be
">=" instead of ">". Otherwise we read past the end of the array
one space.
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
This is a static checker patch and I haven't tested it. Please
review carefully.
diff --git a/arch/x86/xen/multicalls.c b/arch/x86/xen/multicalls.c
index 8bff7e7..1b2b73f 100644
--- a/arch/x86/xen/multicalls.c
+++ b/arch/x86/xen/multicalls.c
@@ -189,10 +189,10 @@ struct multicall_space __xen_mc_entry(size_t args)
unsigned argidx = roundup(b->argidx, sizeof(u64));
BUG_ON(preemptible());
- BUG_ON(b->argidx > MC_ARGS);
+ BUG_ON(b->argidx >= MC_ARGS);
if (b->mcidx == MC_BATCH ||
- (argidx + args) > MC_ARGS) {
+ (argidx + args) >= MC_ARGS) {
mc_stats_flush(b->mcidx == MC_BATCH ? FL_SLOTS : FL_ARGS);
xen_mc_flush();
argidx = roundup(b->argidx, sizeof(u64));
@@ -206,7 +206,7 @@ struct multicall_space __xen_mc_entry(size_t args)
ret.args = &b->args[argidx];
b->argidx = argidx + args;
- BUG_ON(b->argidx > MC_ARGS);
+ BUG_ON(b->argidx >= MC_ARGS);
return ret;
}
@@ -216,7 +216,7 @@ struct multicall_space xen_mc_extend_args(unsigned long op, size_t size)
struct multicall_space ret = { NULL, NULL };
BUG_ON(preemptible());
- BUG_ON(b->argidx > MC_ARGS);
+ BUG_ON(b->argidx >= MC_ARGS);
if (b->mcidx == 0)
return ret;
@@ -224,14 +224,14 @@ struct multicall_space xen_mc_extend_args(unsigned long op, size_t size)
if (b->entries[b->mcidx - 1].op != op)
return ret;
- if ((b->argidx + size) > MC_ARGS)
+ if ((b->argidx + size) >= MC_ARGS)
return ret;
ret.mc = &b->entries[b->mcidx - 1];
ret.args = &b->args[b->argidx];
b->argidx += size;
- BUG_ON(b->argidx > MC_ARGS);
+ BUG_ON(b->argidx >= MC_ARGS);
return ret;
}
^ permalink raw reply related
* Re: [patch] xen: off by one errors in multicalls.c
From: Jeremy Fitzhardinge @ 2011-06-03 18:24 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jeremy Fitzhardinge, Konrad Rzeszutek Wilk,
maintainer:X86 ARCHITECTURE..., kernel-janitors,
open list:XEN HYPERVISOR IN..., open list:XEN HYPERVISOR IN...,
H. Peter Anvin, Thomas Gleixner, Ingo Molnar
In-Reply-To: <20110603044528.GD3661@shale.localdomain>
On 06/02/2011 09:45 PM, Dan Carpenter wrote:
> b->args[] has MC_ARGS elements, so the comparison here should be
> ">=" instead of ">". Otherwise we read past the end of the array
> one space.
Yeah, looks like a correct fix. Fortunately I don't think anything
currently hits that path in practice, though there are some pending
patches which will exercise it more.
Thanks,
J
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
> This is a static checker patch and I haven't tested it. Please
> review carefully.
>
> diff --git a/arch/x86/xen/multicalls.c b/arch/x86/xen/multicalls.c
> index 8bff7e7..1b2b73f 100644
> --- a/arch/x86/xen/multicalls.c
> +++ b/arch/x86/xen/multicalls.c
> @@ -189,10 +189,10 @@ struct multicall_space __xen_mc_entry(size_t args)
> unsigned argidx = roundup(b->argidx, sizeof(u64));
>
> BUG_ON(preemptible());
> - BUG_ON(b->argidx > MC_ARGS);
> + BUG_ON(b->argidx >= MC_ARGS);
>
> if (b->mcidx == MC_BATCH ||
> - (argidx + args) > MC_ARGS) {
> + (argidx + args) >= MC_ARGS) {
> mc_stats_flush(b->mcidx == MC_BATCH ? FL_SLOTS : FL_ARGS);
> xen_mc_flush();
> argidx = roundup(b->argidx, sizeof(u64));
> @@ -206,7 +206,7 @@ struct multicall_space __xen_mc_entry(size_t args)
> ret.args = &b->args[argidx];
> b->argidx = argidx + args;
>
> - BUG_ON(b->argidx > MC_ARGS);
> + BUG_ON(b->argidx >= MC_ARGS);
> return ret;
> }
>
> @@ -216,7 +216,7 @@ struct multicall_space xen_mc_extend_args(unsigned long op, size_t size)
> struct multicall_space ret = { NULL, NULL };
>
> BUG_ON(preemptible());
> - BUG_ON(b->argidx > MC_ARGS);
> + BUG_ON(b->argidx >= MC_ARGS);
>
> if (b->mcidx == 0)
> return ret;
> @@ -224,14 +224,14 @@ struct multicall_space xen_mc_extend_args(unsigned long op, size_t size)
> if (b->entries[b->mcidx - 1].op != op)
> return ret;
>
> - if ((b->argidx + size) > MC_ARGS)
> + if ((b->argidx + size) >= MC_ARGS)
> return ret;
>
> ret.mc = &b->entries[b->mcidx - 1];
> ret.args = &b->args[b->argidx];
> b->argidx += size;
>
> - BUG_ON(b->argidx > MC_ARGS);
> + BUG_ON(b->argidx >= MC_ARGS);
> return ret;
> }
>
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/virtualization
>
^ permalink raw reply
* Re: [Xen-devel] Re: [patch] xen: off by one errors in multicalls.c
From: Konrad Rzeszutek Wilk @ 2011-06-03 19:57 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: open list:XEN HYPERVISOR IN..., Dan Carpenter,
maintainer:X86 ARCHITECTURE..., kernel-janitors,
open list:XEN HYPERVISOR IN..., Jeremy Fitzhardinge,
H. Peter Anvin, Thomas Gleixner, Ingo Molnar
In-Reply-To: <4DE926D4.9010009@goop.org>
On Fri, Jun 03, 2011 at 11:24:20AM -0700, Jeremy Fitzhardinge wrote:
> On 06/02/2011 09:45 PM, Dan Carpenter wrote:
> > b->args[] has MC_ARGS elements, so the comparison here should be
> > ">=" instead of ">". Otherwise we read past the end of the array
> > one space.
>
> Yeah, looks like a correct fix. Fortunately I don't think anything
> currently hits that path in practice, though there are some pending
> patches which will exercise it more.
OK, queueing it for rc1.
^ permalink raw reply
* Re: [PATCHv2 RFC 0/4] virtio and vhost-net capacity handling
From: Rusty Russell @ 2011-06-06 3:39 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, virtualization, steved,
Christian Borntraeger, Tom Lendacky, Martin Schwidefsky, linux390
In-Reply-To: <20110602171721.GA13215@redhat.com>
On Thu, 2 Jun 2011 20:17:21 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Jun 02, 2011 at 06:42:35PM +0300, Michael S. Tsirkin wrote:
> > OK, here's a new attempt to use the new capacity api. I also added more
> > comments to clarify the logic. Hope this is more readable. Let me know
> > pls.
> >
> > This is on top of the patches applied by Rusty.
> >
> > Warning: untested. Posting now to give people chance to
> > comment on the API.
> >
> > Changes from v1:
> > - fix comment in patch 2 to correct confusion noted by Rusty
> > - rewrite patch 3 along the lines suggested by Rusty
> > note: it's not exactly the same but I hope it's close
> > enough, the main difference is that mine does limited
> > polling even in the unlikely xmit failure case.
> > - added a patch to not return capacity from add_buf
> > it always looked like a weird hack
> >
> > Michael S. Tsirkin (4):
> > virtio_ring: add capacity check API
> > virtio_net: fix tx capacity checks using new API
> > virtio_net: limit xmit polling
> > Revert "virtio: make add_buf return capacity remaining:
> >
> > drivers/net/virtio_net.c | 111 ++++++++++++++++++++++++++----------------
> > drivers/virtio/virtio_ring.c | 10 +++-
> > include/linux/virtio.h | 7 ++-
> > 3 files changed, 84 insertions(+), 44 deletions(-)
> >
> > --
> > 1.7.5.53.gc233e
>
>
> And just FYI, here's a patch (on top) that I considered but
> decided against. This does a single get_buf before
> xmit. I thought it's not really needed as the capacity
> check in add_buf is relatively cheap, and we removed
> the kick in xmit_skb. But the point is that the loop
> will now be easy to move around if someone manages
> to show this benefits speed (which I doubt).
Agreed. The other is clearer.
I like the approach these patches take. Testing is required, but I
think the final result is a neater driver than the current one, as well
as having nicer latency.
Thanks,
Rusty.
^ permalink raw reply
* [PATCH 00/49] Staging: hv: Driver cleanup
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization
Further cleanup of the hv drivers:
1) Continue to cleanup our drivers to conform to the Linux Driver
Model.
2) Fix some long standing bugs with regards to unloading and
reloading the drivers - block, net and stor.
3) VMBUS is an ACPI enumerated device; make VMBUS an ACPI bus driver.
4) Get rid of channel polling code; instead the channel receive paths
will be purely interrupt drive.
5) Cleanup error return values in the vmbus driver and general cleanup.
6) Fix memory barrier calls in the ring buffer code.
7) Increase the timeout values for some critical guest to host calls.
Regads,
K. Y
^ permalink raw reply
* [PATCH 01/49] Staging: hv: vmbus: In vmbus_child_driver_unregister() don't set the bus field to NULL
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400540-13193-1-git-send-email-kys@microsoft.com>
As part of conforming to the Linux Driver Model, do not set the bus field to
NULL when the driver un-registers.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index ec1d38c..51af6d8 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -631,7 +631,6 @@ void vmbus_child_driver_unregister(struct device_driver *drv)
driver_unregister(drv);
- drv->bus = NULL;
}
EXPORT_SYMBOL(vmbus_child_driver_unregister);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 02/49] Staging: hv: storvsc: Cleanup the exit function in storvsc_drv.c
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
Get rid of unnecessary layering in the module exit path.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/storvsc_drv.c | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 942cc5f..e21f7e6 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -792,7 +792,7 @@ static int storvsc_drv_init(void)
return ret;
}
-static void storvsc_drv_exit(void)
+static void __exit storvsc_drv_exit(void)
{
vmbus_child_driver_unregister(&storvsc_drv.driver);
}
@@ -806,13 +806,8 @@ static int __init storvsc_init(void)
return ret;
}
-static void __exit storvsc_exit(void)
-{
- storvsc_drv_exit();
-}
-
MODULE_LICENSE("GPL");
MODULE_VERSION(HV_DRV_VERSION);
MODULE_DESCRIPTION("Microsoft Hyper-V virtual storage driver");
module_init(storvsc_init);
-module_exit(storvsc_exit);
+module_exit(storvsc_drv_exit);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 03/49] Staging: hv: storvsc: Cleanup the module init function in storvsc_drv.c
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
Get rid of unnecessary layering in the module init path.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/storvsc_drv.c | 13 ++-----------
1 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index e21f7e6..33bce87 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -756,7 +756,7 @@ static struct hv_driver storvsc_drv = {
/*
* storvsc_drv_init - StorVsc driver initialization.
*/
-static int storvsc_drv_init(void)
+static int __init storvsc_drv_init(void)
{
int ret;
struct hv_driver *drv = &storvsc_drv;
@@ -797,17 +797,8 @@ static void __exit storvsc_drv_exit(void)
vmbus_child_driver_unregister(&storvsc_drv.driver);
}
-static int __init storvsc_init(void)
-{
- int ret;
-
- DPRINT_INFO(STORVSC_DRV, "Storvsc initializing....");
- ret = storvsc_drv_init();
- return ret;
-}
-
MODULE_LICENSE("GPL");
MODULE_VERSION(HV_DRV_VERSION);
MODULE_DESCRIPTION("Microsoft Hyper-V virtual storage driver");
-module_init(storvsc_init);
+module_init(storvsc_drv_init);
module_exit(storvsc_drv_exit);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 04/49] Staging: hv: storvsc: Fix a bug in the storvsc_remove() function
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
When the storvs driver unloads, we need to accomodate disk cache flushes.
Re-order the code to permit this.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/storvsc_drv.c | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 33bce87..fd474d6 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -317,22 +317,20 @@ static int storvsc_remove(struct hv_device *dev)
struct hv_host_device *host_dev =
(struct hv_host_device *)host->hostdata;
+ DPRINT_INFO(STORVSC, "removing host adapter (%p)...", host);
+ scsi_remove_host(host);
+
+ DPRINT_INFO(STORVSC, "releasing host adapter (%p)...", host);
+ scsi_host_put(host);
/*
* Call to the vsc driver to let it know that the device is being
* removed
*/
storvsc_dev_remove(dev);
-
if (host_dev->request_pool) {
kmem_cache_destroy(host_dev->request_pool);
host_dev->request_pool = NULL;
}
-
- DPRINT_INFO(STORVSC, "removing host adapter (%p)...", host);
- scsi_remove_host(host);
-
- DPRINT_INFO(STORVSC, "releasing host adapter (%p)...", host);
- scsi_host_put(host);
return 0;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 05/49] Staging: hv: storvsc: Cleanup some dated/unnecessary comments
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
Cleanup some dated/unnecessary comments.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/storvsc_drv.c | 14 +-------------
1 files changed, 1 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index fd474d6..499e1d7 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -308,9 +308,6 @@ static unsigned int copy_to_bounce_buffer(struct scatterlist *orig_sgl,
}
-/*
- * storvsc_remove - Callback when our device is removed
- */
static int storvsc_remove(struct hv_device *dev)
{
struct Scsi_Host *host = dev_get_drvdata(&dev->device);
@@ -322,10 +319,7 @@ static int storvsc_remove(struct hv_device *dev)
DPRINT_INFO(STORVSC, "releasing host adapter (%p)...", host);
scsi_host_put(host);
- /*
- * Call to the vsc driver to let it know that the device is being
- * removed
- */
+
storvsc_dev_remove(dev);
if (host_dev->request_pool) {
kmem_cache_destroy(host_dev->request_pool);
@@ -423,7 +417,6 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
DPRINT_INFO(STORVSC_DRV, "sdev (%p) dev obj (%p) - host resetting...",
scmnd->device, dev);
- /* Invokes the vsc to reset the host/bus */
ret = storvsc_host_reset(dev);
if (ret != 0)
return ret;
@@ -477,7 +470,6 @@ static void storvsc_commmand_completion(struct hv_storvsc_request *request)
scmnd->host_scribble = NULL;
scmnd->scsi_done = NULL;
- /* !!DO NOT MODIFY the scmnd after this call */
scsi_done_fn(scmnd);
kmem_cache_free(host_dev->request_pool, cmd_request);
@@ -750,10 +742,6 @@ static struct hv_driver storvsc_drv = {
.remove = storvsc_remove,
};
-
-/*
- * storvsc_drv_init - StorVsc driver initialization.
- */
static int __init storvsc_drv_init(void)
{
int ret;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 06/49] Staging: hv: stor: Get rid of unnecessary DPRINTs in stor vsc_drv.c
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
Get rid of unnecessary DPRINTs in stor vsc_drv.c.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/storvsc_drv.c | 19 -------------------
1 files changed, 0 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 499e1d7..73fbded 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -92,12 +92,8 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
scsi_adjust_queue_depth(sdevice, MSG_SIMPLE_TAG,
STORVSC_MAX_IO_REQUESTS);
- DPRINT_INFO(STORVSC_DRV, "sdev (%p) - setting max segment size to %ld",
- sdevice, PAGE_SIZE);
blk_queue_max_segment_size(sdevice->request_queue, PAGE_SIZE);
- DPRINT_INFO(STORVSC_DRV, "sdev (%p) - adding merge bio vec routine",
- sdevice);
blk_queue_merge_bvec(sdevice->request_queue, storvsc_merge_bvec);
blk_queue_bounce_limit(sdevice->request_queue, BLK_BOUNCE_ANY);
@@ -314,10 +310,8 @@ static int storvsc_remove(struct hv_device *dev)
struct hv_host_device *host_dev =
(struct hv_host_device *)host->hostdata;
- DPRINT_INFO(STORVSC, "removing host adapter (%p)...", host);
scsi_remove_host(host);
- DPRINT_INFO(STORVSC, "releasing host adapter (%p)...", host);
scsi_host_put(host);
storvsc_dev_remove(dev);
@@ -349,9 +343,6 @@ static int storvsc_get_chs(struct scsi_device *sdev, struct block_device * bdev,
info[1] = sectors_pt;
info[2] = (int)cylinders;
- DPRINT_INFO(STORVSC_DRV, "CHS (%d, %d, %d)", (int)cylinders, heads,
- sectors_pt);
-
return 0;
}
@@ -362,7 +353,6 @@ static int storvsc_host_reset(struct hv_device *device)
struct vstor_packet *vstor_packet;
int ret, t;
- DPRINT_INFO(STORVSC, "resetting host adapter...");
stor_device = get_stor_device(device);
if (!stor_device)
@@ -391,7 +381,6 @@ static int storvsc_host_reset(struct hv_device *device)
goto cleanup;
}
- DPRINT_INFO(STORVSC, "host adapter reset completed");
/*
* At this point, all outstanding requests in the adapter
@@ -414,16 +403,10 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
(struct hv_host_device *)scmnd->device->host->hostdata;
struct hv_device *dev = host_dev->dev;
- DPRINT_INFO(STORVSC_DRV, "sdev (%p) dev obj (%p) - host resetting...",
- scmnd->device, dev);
-
ret = storvsc_host_reset(dev);
if (ret != 0)
return ret;
- DPRINT_INFO(STORVSC_DRV, "sdev (%p) dev obj (%p) - host reseted",
- scmnd->device, dev);
-
return ret;
}
@@ -500,8 +483,6 @@ static int storvsc_queuecommand_lck(struct scsi_cmnd *scmnd,
cmd_request =
(struct storvsc_cmd_request *)scmnd->host_scribble;
- DPRINT_INFO(STORVSC_DRV, "retrying scmnd %p cmd_request %p",
- scmnd, cmd_request);
goto retry_request;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 07/49] Staging: hv: stor: Rename the vriable gStorVscDeviceType in storvsc_drv.c
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
Rename the vriable gStorVscDeviceType in storvsc_drv.c.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@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 73fbded..9e51356 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -44,7 +44,7 @@ MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
static const char *driver_name = "storvsc";
/* {ba6163d9-04a1-4d29-b605-72e2ffb1dc7f} */
-static const struct hv_guid gStorVscDeviceType = {
+static const struct hv_guid stor_vsci_device_type = {
.data = {
0xd9, 0x63, 0x61, 0xba, 0xa1, 0x04, 0x29, 0x4d,
0xb6, 0x05, 0x72, 0xe2, 0xff, 0xb1, 0xdc, 0x7f
@@ -742,7 +742,7 @@ static int __init storvsc_drv_init(void)
sizeof(struct vstor_packet) + sizeof(u64),
sizeof(u64)));
- memcpy(&drv->dev_type, &gStorVscDeviceType,
+ memcpy(&drv->dev_type, &stor_vsci_device_type,
sizeof(struct hv_guid));
if (max_outstanding_req_per_channel <
--
1.7.4.1
^ permalink raw reply related
* [PATCH 08/49] Staging: hv: stor: Get rid of the unused initialization of the name field
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
The name field of hv_driver is unused in storvsc_drv.c; get rid of it.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/storvsc_drv.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 9e51356..53e9ebd 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -749,7 +749,6 @@ static int __init storvsc_drv_init(void)
STORVSC_MAX_IO_REQUESTS)
return -1;
- drv->name = driver_name;
drv->driver.name = driver_name;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 09/49] Staging: hv: blk: Get rid of the unused initialization of the name field
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization
Cc: K. Y. Srinivasan, Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
The name field of hv_driver is unused in blkvsc_drv.c; get rid of it.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/blkvsc_drv.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index 46daade..bcf562f 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -824,7 +824,6 @@ static int blkvsc_drv_init(void)
BUILD_BUG_ON(sizeof(sector_t) != 8);
memcpy(&drv->dev_type, &dev_type, sizeof(struct hv_guid));
- drv->name = drv_name;
drv->driver.name = drv_name;
/* The driver belongs to vmbus */
--
1.7.4.1
^ permalink raw reply related
* [PATCH 10/49] Staging: hv: mouse: Get rid of the unused initialization of the name field
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
The name field of hv_driver is unused in hv_mouse.c; get rid of it.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/hv_mouse.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
index 359e737..b191810 100644
--- a/drivers/staging/hv/hv_mouse.c
+++ b/drivers/staging/hv/hv_mouse.c
@@ -936,7 +936,6 @@ static int __init mousevsc_init(void)
sizeof(struct hv_guid));
drv->driver.name = driver_name;
- drv->name = driver_name;
/* The driver belongs to vmbus */
vmbus_child_driver_register(&drv->driver);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 11/49] Staging: hv: vmbus: Don't free the channel when the channel is closed
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
When the driver unloads, the device must persist. A channel represents the
device and so we should not free the channel when the channel is closed as
part of the driver unloading.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/channel.c | 14 --------------
1 files changed, 0 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/hv/channel.c b/drivers/staging/hv/channel.c
index f655e59..aca9ac8 100644
--- a/drivers/staging/hv/channel.c
+++ b/drivers/staging/hv/channel.c
@@ -552,7 +552,6 @@ void vmbus_close(struct vmbus_channel *channel)
{
struct vmbus_channel_close_channel *msg;
struct vmbus_channel_msginfo *info;
- unsigned long flags;
int ret;
/* Stop callback and cancel the timer asap */
@@ -591,19 +590,6 @@ void vmbus_close(struct vmbus_channel *channel)
kfree(info);
- /*
- * If we are closing the channel during an error path in
- * opening the channel, don't free the channel since the
- * caller will free the channel
- */
-
- if (channel->state == CHANNEL_OPEN_STATE) {
- spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
- list_del(&channel->listentry);
- spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
-
- free_channel(channel);
- }
}
EXPORT_SYMBOL_GPL(vmbus_close);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 12/49] Staging: hv: storvsc: Add a DMI signature to support auto-loading
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
To support auto-loading the storvsc driver, add a DMI signature.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/storvsc_drv.c | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 53e9ebd..2c6d2f2 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -24,6 +24,7 @@
#include <linux/module.h>
#include <linux/device.h>
#include <linux/blkdev.h>
+#include <linux/dmi.h>
#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
#include <scsi/scsi_host.h>
@@ -723,6 +724,27 @@ static struct hv_driver storvsc_drv = {
.remove = storvsc_remove,
};
+/*
+ * We use a DMI table to determine if we should autoload this driver This is
+ * needed by distro tools to determine if the hyperv drivers should be
+ * installed and/or configured. We don't do anything else with the table, but
+ * it needs to be present.
+ */
+
+static const struct dmi_system_id __initconst
+hv_stor_dmi_table[] __maybe_unused = {
+ {
+ .ident = "Hyper-V",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
+ DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
+ },
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(dmi, hv_stor_dmi_table);
+
static int __init storvsc_drv_init(void)
{
int ret;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 13/49] Staging: hv: vmbus: Change the signature of vmbus_bus_init()
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
In preparation for making the vmbus driver an ACPI bus driver, change the
signature of vmbus_bus_init() to accept the irq value.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 51af6d8..1b69339 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -528,7 +528,7 @@ static irqreturn_t vmbus_isr(int irq, void *dev_id)
* - get the irq resource
* - retrieve the channel offers
*/
-static int vmbus_bus_init(struct pci_dev *pdev)
+static int vmbus_bus_init(int irq)
{
int ret;
unsigned int vector;
@@ -552,13 +552,13 @@ static int vmbus_bus_init(struct pci_dev *pdev)
}
/* Get the interrupt resource */
- ret = request_irq(pdev->irq, vmbus_isr,
+ ret = request_irq(irq, vmbus_isr,
IRQF_SHARED | IRQF_SAMPLE_RANDOM,
- driver_name, pdev);
+ driver_name, hv_pci_dev);
if (ret != 0) {
pr_err("Unable to request IRQ %d\n",
- pdev->irq);
+ irq);
bus_unregister(&hv_bus);
@@ -566,7 +566,7 @@ static int vmbus_bus_init(struct pci_dev *pdev)
goto cleanup;
}
- vector = IRQ0_VECTOR + pdev->irq;
+ vector = IRQ0_VECTOR + irq;
/*
* Notify the hypervisor of our irq and
@@ -575,7 +575,7 @@ static int vmbus_bus_init(struct pci_dev *pdev)
on_each_cpu(hv_synic_init, (void *)&vector, 1);
ret = vmbus_connect();
if (ret) {
- free_irq(pdev->irq, pdev);
+ free_irq(irq, hv_pci_dev);
bus_unregister(&hv_bus);
goto cleanup;
}
@@ -795,7 +795,7 @@ static int __devinit hv_pci_probe(struct pci_dev *pdev,
if (pdev->irq == 0)
pdev->irq = irq;
- pci_probe_error = vmbus_bus_init(pdev);
+ pci_probe_error = vmbus_bus_init(pdev->irq);
if (pci_probe_error)
pci_disable_device(pdev);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 14/49] Staging: hv: vmbus: Use the DSDT specified irq for vmbus
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
DSDT specifies the irq value for the vmbus driver; use it unconditionally.
This is an exclusive interrupt line dedicated for the vmbus driver.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 15 +++------------
1 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 1b69339..5d7ecfd 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -552,9 +552,8 @@ static int vmbus_bus_init(int irq)
}
/* Get the interrupt resource */
- ret = request_irq(irq, vmbus_isr,
- IRQF_SHARED | IRQF_SAMPLE_RANDOM,
- driver_name, hv_pci_dev);
+ ret = request_irq(irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
+ driver_name, hv_pci_dev);
if (ret != 0) {
pr_err("Unable to request IRQ %d\n",
@@ -787,15 +786,7 @@ static int __devinit hv_pci_probe(struct pci_dev *pdev,
if (pci_probe_error)
goto probe_cleanup;
- /*
- * If the PCI sub-sytem did not assign us an
- * irq, use the bios provided one.
- */
-
- if (pdev->irq == 0)
- pdev->irq = irq;
-
- pci_probe_error = vmbus_bus_init(pdev->irq);
+ pci_probe_error = vmbus_bus_init(irq);
if (pci_probe_error)
pci_disable_device(pdev);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 15/49] Staging: hv: vmbus: Make vmbus an acpi bus driver
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
Now, make the vmbus driver an ACPI bus driver.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 56 ++++++---------------------------------
1 files changed, 9 insertions(+), 47 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 5d7ecfd..176a8cd 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -39,7 +39,7 @@
#include "hyperv_vmbus.h"
-static struct pci_dev *hv_pci_dev;
+static struct acpi_device *hv_acpi_dev;
static struct tasklet_struct msg_dpc;
static struct tasklet_struct event_dpc;
@@ -49,7 +49,6 @@ EXPORT_SYMBOL(vmbus_loglevel);
/* (ALL_MODULES << 16 | DEBUG_LVL_ENTEREXIT); */
/* (((VMBUS | VMBUS_DRV)<<16) | DEBUG_LVL_ENTEREXIT); */
-static int pci_probe_error;
static struct completion probe_event;
static int irq;
@@ -553,7 +552,7 @@ static int vmbus_bus_init(int irq)
/* Get the interrupt resource */
ret = request_irq(irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
- driver_name, hv_pci_dev);
+ driver_name, hv_acpi_dev);
if (ret != 0) {
pr_err("Unable to request IRQ %d\n",
@@ -574,7 +573,7 @@ static int vmbus_bus_init(int irq)
on_each_cpu(hv_synic_init, (void *)&vector, 1);
ret = vmbus_connect();
if (ret) {
- free_irq(irq, hv_pci_dev);
+ free_irq(irq, hv_acpi_dev);
bus_unregister(&hv_bus);
goto cleanup;
}
@@ -674,7 +673,7 @@ int vmbus_child_device_register(struct hv_device *child_device_obj)
/* The new device belongs to this bus */
child_device_obj->device.bus = &hv_bus; /* device->dev.bus; */
- child_device_obj->device.parent = &hv_pci_dev->dev;
+ child_device_obj->device.parent = &hv_acpi_dev->dev;
child_device_obj->device.release = vmbus_device_release;
/*
@@ -731,6 +730,8 @@ static int vmbus_acpi_add(struct acpi_device *device)
{
acpi_status result;
+ hv_acpi_dev = device;
+
result =
acpi_walk_resources(device->handle, METHOD_NAME__CRS,
vmbus_walk_resources, &irq);
@@ -777,25 +778,6 @@ static void vmbus_acpi_exit(void)
}
-static int __devinit hv_pci_probe(struct pci_dev *pdev,
- const struct pci_device_id *ent)
-{
- hv_pci_dev = pdev;
-
- pci_probe_error = pci_enable_device(pdev);
- if (pci_probe_error)
- goto probe_cleanup;
-
- pci_probe_error = vmbus_bus_init(irq);
-
- if (pci_probe_error)
- pci_disable_device(pdev);
-
-probe_cleanup:
- complete(&probe_event);
- return pci_probe_error;
-}
-
/*
* We use a PCI table to determine if we should autoload this driver This is
* needed by distro tools to determine if the hyperv drivers should be
@@ -808,13 +790,7 @@ static const struct pci_device_id microsoft_hv_pci_table[] = {
};
MODULE_DEVICE_TABLE(pci, microsoft_hv_pci_table);
-static struct pci_driver hv_bus_driver = {
- .name = "hv_bus",
- .probe = hv_pci_probe,
- .id_table = microsoft_hv_pci_table,
-};
-
-static int __init hv_pci_init(void)
+static int __init hv_acpi_init(void)
{
int ret;
@@ -835,21 +811,7 @@ static int __init hv_pci_init(void)
return -ENODEV;
}
- vmbus_acpi_exit();
- init_completion(&probe_event);
- ret = pci_register_driver(&hv_bus_driver);
- if (ret)
- return ret;
- /*
- * All the vmbus initialization occurs within the
- * hv_pci_probe() function. Wait for hv_pci_probe()
- * to complete.
- */
- wait_for_completion(&probe_event);
-
- if (pci_probe_error)
- pci_unregister_driver(&hv_bus_driver);
- return pci_probe_error;
+ return vmbus_bus_init(irq);
}
@@ -857,4 +819,4 @@ MODULE_LICENSE("GPL");
MODULE_VERSION(HV_DRV_VERSION);
module_param(vmbus_loglevel, int, S_IRUGO|S_IWUSR);
-module_init(hv_pci_init);
+module_init(hv_acpi_init);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 16/49] Staging: hv: vmbus: Get rid of vmbus_acpi_init() by inlining the code
From: K. Y. Srinivasan @ 2011-06-06 22:49 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang, Abhishek Kane
In-Reply-To: <1307400613-13234-1-git-send-email-kys@microsoft.com>
Staging: hv: vmbus: Get rid of vmbus_acpi_init() by inlining the code.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 176a8cd..d799f42 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -758,18 +758,6 @@ static struct acpi_driver vmbus_acpi_driver = {
},
};
-static int vmbus_acpi_init(void)
-{
- int result;
-
-
- result = acpi_bus_register_driver(&vmbus_acpi_driver);
- if (result < 0)
- return result;
-
- return 0;
-}
-
static void vmbus_acpi_exit(void)
{
acpi_bus_unregister_driver(&vmbus_acpi_driver);
@@ -800,7 +788,8 @@ static int __init hv_acpi_init(void)
* Get irq resources first.
*/
- ret = vmbus_acpi_init();
+ ret = acpi_bus_register_driver(&vmbus_acpi_driver);
+
if (ret)
return ret;
--
1.7.4.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox