Re: [PATCH net-next RFC 3/3] virtio-net: conditionally enable tx interrupt

2014-10-13 Thread Jason Wang
On 10/11/2014 10:48 PM, Eric Dumazet wrote:
 On Sat, 2014-10-11 at 15:16 +0800, Jason Wang wrote:
 We free transmitted packets in ndo_start_xmit() in the past to get better
 performance in the past. One side effect is that skb_orphan() needs to be
 called in ndo_start_xmit() which makes sk_wmem_alloc not accurate in
 fact. For TCP protocol, this means several optimization could not work well
 such as TCP small queue and auto corking. This can lead extra low
 throughput of small packets stream.

 Thanks to the urgent descriptor support. This patch tries to solve this
 issue by enable the tx interrupt selectively for stream packets. This means
 we don't need to orphan TCP stream packets in ndo_start_xmit() but enable
 tx interrupt for those packets. After we get tx interrupt, a tx napi was
 scheduled to free those packets.

 With this method, sk_wmem_alloc of TCP socket were more accurate than in
 the past which let TCP can batch more through TSQ and auto corking.

 Signed-off-by: Jason Wang jasow...@redhat.com
 ---
  drivers/net/virtio_net.c | 164 
 ---
  1 file changed, 128 insertions(+), 36 deletions(-)

 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index 5810841..b450fc4 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
 @@ -72,6 +72,8 @@ struct send_queue {
  
  /* Name of the send queue: output.$index */
  char name[40];
 +
 +struct napi_struct napi;
  };
  
  /* Internal representation of a receive virtqueue */
 @@ -217,15 +219,40 @@ static struct page *get_a_page(struct receive_queue 
 *rq, gfp_t gfp_mask)
  return p;
  }
  
 +static int free_old_xmit_skbs(struct send_queue *sq, int budget)
 +{
 +struct sk_buff *skb;
 +unsigned int len;
 +struct virtnet_info *vi = sq-vq-vdev-priv;
 +struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
 +int sent = 0;
 +
 +while (sent  budget 
 +   (skb = virtqueue_get_buf(sq-vq, len)) != NULL) {
 +pr_debug(Sent skb %p\n, skb);
 +
 +u64_stats_update_begin(stats-tx_syncp);
 +stats-tx_bytes += skb-len;
 +stats-tx_packets++;
 +u64_stats_update_end(stats-tx_syncp);
 +
 +dev_kfree_skb_any(skb);
 +sent++;
 +}
 +
 You could accumulate skb-len in a totlen var, and perform a single

   u64_stats_update_begin(stats-tx_syncp);
   stats-tx_bytes += totlen;
   stats-tx_packets += sent;
   u64_stats_update_end(stats-tx_syncp);

 after the loop.


Yes, will do this in a separated patch.
 +return sent;
 +}
 +
 ...

 +
 +static bool virtnet_skb_needs_intr(struct sk_buff *skb)
 +{
 +union {
 +unsigned char *network;
 +struct iphdr *ipv4;
 +struct ipv6hdr *ipv6;
 +} hdr;
 +struct tcphdr *th = tcp_hdr(skb);
 +u16 payload_len;
 +
 +hdr.network = skb_network_header(skb);
 +
 +/* Only IPv4/IPv6 with TCP is supported */
   Oh well, yet another packet flow dissector :)

   If most packets were caught by your implementation, you could use it
 for fast patj and fallback to skb_flow_dissect() for encapsulated
 traffic.

   struct flow_keys keys;   

   if (!skb_flow_dissect(skb, keys)) 
   return false;

   if (keys.ip_proto != IPPROTO_TCP)
   return false;

   then check __skb_get_poff() how to get th, and check if there is some
 payload...



Yes, but we don't know if most of packets were TCP or encapsulated TCP,
it depends on userspace application. If not, looks like
skb_flow_dissect() can bring some overhead, or it could be ignored?

skb_flow_dissect

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


Re: [PATCH net-next RFC 1/3] virtio: support for urgent descriptors

2014-10-13 Thread Jason Wang
On 10/12/2014 05:27 PM, Michael S. Tsirkin wrote:
 On Sat, Oct 11, 2014 at 03:16:44PM +0800, Jason Wang wrote:
 Below should be useful for some experiments Jason is doing.
 I thought I'd send it out for early review/feedback.

 event idx feature allows us to defer interrupts until
 a specific # of descriptors were used.
 Sometimes it might be useful to get an interrupt after
 a specific descriptor, regardless.
 This adds a descriptor flag for this, and an API
 to create an urgent output descriptor.
 This is still an RFC:
 we'll need a feature bit for drivers to detect this,
 but we've run out of feature bits for virtio 0.X.
 For experimentation purposes, drivers can assume
 this is set, or add a driver-specific feature bit.

 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 Signed-off-by: Jason Wang jasow...@redhat.com
 I see that as compared to my original patch, you have
 added a new flag: VRING_AVAIL_F_NO_URGENT_INTERRUPT
 I don't think it's necessary, see below.

 As such, I think this patch should be split:
 - original patch adding support for urgent descriptors
 - a patch adding virtqueue_enable/disable_cb_urgent(_prepare)?

Not sure this is a good idea, since the api of first patch is in-completed.

 ---
  drivers/virtio/virtio_ring.c | 75 
 +---
  include/linux/virtio.h   | 14 
  include/uapi/linux/virtio_ring.h |  5 ++-
  3 files changed, 89 insertions(+), 5 deletions(-)

[...]
  
 +unsigned virtqueue_enable_cb_prepare_urgent(struct virtqueue *_vq)
 +{
 +struct vring_virtqueue *vq = to_vvq(_vq);
 +u16 last_used_idx;
 +
 +START_USE(vq);
 +vq-vring.avail-flags = ~VRING_AVAIL_F_NO_URGENT_INTERRUPT;
 +last_used_idx = vq-last_used_idx;
 +END_USE(vq);
 +return last_used_idx;
 +}
 +EXPORT_SYMBOL_GPL(virtqueue_enable_cb_prepare_urgent);
 +
 You can implement virtqueue_enable_cb_prepare_urgent
 simply by clearing ~VRING_AVAIL_F_NO_INTERRUPT.

 The effect is same: host sends interrupts only if there
 is an urgent descriptor.

Seems not, consider the case when event index was disabled. This will
turn on all interrupts.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC 08/11] virtio_blk: use virtio v1.0 endian

2014-10-13 Thread Rusty Russell
Cornelia Huck cornelia.h...@de.ibm.com writes:
 Note that we care only about the fields still in use for virtio v1.0.

 Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com
 Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
 Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com

Hi Cornelia,

These patches all look good; I'm a bit nervous about our testing
missing some conversion, so we'll need qemu patches for PCI so we can
test on other platforms too.

Thanks,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC 03/11] virtio: support more feature bits

2014-10-13 Thread Rusty Russell
Cornelia Huck cornelia.h...@de.ibm.com writes:
 With virtio-1, we support more than 32 feature bits. Let's make
 vdev-guest_features depend on the number of supported feature bits,
 allowing us to grow the feature bits automatically.

It's a judgement call, but I would say that simply using uint64_t
will be sufficient for quite a while.

Cheers,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 2/2] virtio_balloon: free some memory from baloon on OOM

2014-10-13 Thread Rusty Russell
Denis V. Lunev d...@parallels.com writes:
 From: Raushaniya Maksudova rmaksud...@parallels.com

 Excessive virtio_balloon inflation can cause invocation of OOM-killer,
 when Linux is under severe memory pressure. Various mechanisms are
 responsible for correct virtio_balloon memory management. Nevertheless
 it is often the case that these control tools does not have enough time
 to react on fast changing memory load. As a result OS runs out of memory
 and invokes OOM-killer. The balancing of memory by use of the virtio
 balloon should not cause the termination of processes while there are
 pages in the balloon. Now there is no way for virtio balloon driver to
 free some memory at the last moment before some process will be get
 killed by OOM-killer.

This makes some amount of sense.

But I suggest a few minor changes:

 +static int oom_vballoon_pages = OOM_VBALLOON_DEFAULT_PAGES;
 +module_param(oom_vballoon_pages, int, S_IRUSR | S_IWUSR);
 +MODULE_PARM_DESC(oom_vballoon_pages, pages to free on OOM);

Since this is already prefixed with virtio_balloon. I suggest just
calling it oom_pages.

 +static int virtballoon_oom_notify(struct notifier_block *self,
 +   unsigned long dummy, void *parm)
 +{
 + unsigned int num_freed_pages;
 + unsigned long *freed = (unsigned long *)parm;
 + struct virtio_balloon *vb = container_of((struct notifier_block *)self,
 +  struct virtio_balloon, nb);

Why cast self here?

 + num_freed_pages = leak_balloon(vb, oom_vballoon_pages);
 + update_balloon_size(vb);
 + *freed += num_freed_pages;
 +
 + return NOTIFY_OK;
 +}

Cheers,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: BUG_ON in virtio-ring.c

2014-10-13 Thread Rusty Russell
Alexey Lapitsky lex.pub...@gmail.com writes:
 Hi,

 I'm hitting this bug with both ext4 and btrfs.

 Here's an example of the backtrace:
 https://gist.github.com/vzctl/e888a821333979120932

 I tried raising this BUG only for direct ring and it solved the problem:

  -   BUG_ON(total_sg  vq-vring.num);
 +   BUG_ON(total_sg  vq-vring.num  !vq-indirect);

 Shall I submit the patch or is a more elaborate fix required?

This is wrong.  It looks like the block layer is spending down
more sg elements than we have ring entries, yet we tell it not to:

blk_queue_max_segments(q, vblk-sg_elems-2);

If you apply this patch, what happens?  Here it prints:

[0.616564] virtqueue elements = 128, max_segments = 126 (1 queues)
[0.621244]  vda: vda1 vda2  vda5 
[0.632290] virtqueue elements = 128, max_segments = 126 (1 queues)
[0.683526]  vdb: vdb1 vdb2  vdb5 

Cheers,
Rusty.

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 0a581400de0f..aa9d4d313587 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -683,6 +683,13 @@ static int virtblk_probe(struct virtio_device *vdev)
/* We can handle whatever the host told us to handle. */
blk_queue_max_segments(q, vblk-sg_elems-2);
 
+   printk(virtqueue elements = %u, max_segments = %u (%u queues), 
+  virtqueue_get_vring_size(vblk-vqs[0].vq),
+  vblk-sg_elems-2,
+  vblk-num_vqs);
+
+   BUG_ON(vblk-sg_elems-2  virtqueue_get_vring_size(vblk-vqs[0].vq));
+
/* No need to bounce any requests */
blk_queue_bounce_limit(q, BLK_BOUNCE_ANY);
 


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


Re: [PATCH v3 10/25] virtio: add API to enable VQs early

2014-10-13 Thread Rusty Russell
Michael S. Tsirkin m...@redhat.com writes:
 virtio spec 0.9.X requires DRIVER_OK to be set before
 VQs are used, but some drivers use VQs before probe
 function returns.
 Since DRIVER_OK is set after probe, this violates the spec.

 Even though under virtio 1.0 transitional devices support this
 behaviour, we want to make it possible for those early callers to become
 spec compliant and eventually support non-transitional devices.

 Add API for drivers to call before using VQs.

 Sets DRIVER_OK internally.

 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com

OK, this all looks good, but I think we should do two things:

1) rename virtio_enable_vqs_early() to virtio_device_ready().
2) Add a BUG_ON in the virtio_ring code to make sure device is ready
   before any ops are called.

Cheers,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH net-next RFC 1/3] virtio: support for urgent descriptors

2014-10-13 Thread Michael S. Tsirkin
On Mon, Oct 13, 2014 at 02:22:12PM +0800, Jason Wang wrote:
 On 10/12/2014 05:27 PM, Michael S. Tsirkin wrote:
  On Sat, Oct 11, 2014 at 03:16:44PM +0800, Jason Wang wrote:
  Below should be useful for some experiments Jason is doing.
  I thought I'd send it out for early review/feedback.
 
  event idx feature allows us to defer interrupts until
  a specific # of descriptors were used.
  Sometimes it might be useful to get an interrupt after
  a specific descriptor, regardless.
  This adds a descriptor flag for this, and an API
  to create an urgent output descriptor.
  This is still an RFC:
  we'll need a feature bit for drivers to detect this,
  but we've run out of feature bits for virtio 0.X.
  For experimentation purposes, drivers can assume
  this is set, or add a driver-specific feature bit.
 
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  Signed-off-by: Jason Wang jasow...@redhat.com
  I see that as compared to my original patch, you have
  added a new flag: VRING_AVAIL_F_NO_URGENT_INTERRUPT
  I don't think it's necessary, see below.
 
  As such, I think this patch should be split:
  - original patch adding support for urgent descriptors
  - a patch adding virtqueue_enable/disable_cb_urgent(_prepare)?
 
 Not sure this is a good idea, since the api of first patch is in-completed.
 
  ---
   drivers/virtio/virtio_ring.c | 75 
  +---
   include/linux/virtio.h   | 14 
   include/uapi/linux/virtio_ring.h |  5 ++-
   3 files changed, 89 insertions(+), 5 deletions(-)
 
 [...]
   
  +unsigned virtqueue_enable_cb_prepare_urgent(struct virtqueue *_vq)
  +{
  +  struct vring_virtqueue *vq = to_vvq(_vq);
  +  u16 last_used_idx;
  +
  +  START_USE(vq);
  +  vq-vring.avail-flags = ~VRING_AVAIL_F_NO_URGENT_INTERRUPT;
  +  last_used_idx = vq-last_used_idx;
  +  END_USE(vq);
  +  return last_used_idx;
  +}
  +EXPORT_SYMBOL_GPL(virtqueue_enable_cb_prepare_urgent);
  +
  You can implement virtqueue_enable_cb_prepare_urgent
  simply by clearing ~VRING_AVAIL_F_NO_INTERRUPT.
 
  The effect is same: host sends interrupts only if there
  is an urgent descriptor.
 
 Seems not, consider the case when event index was disabled. This will
 turn on all interrupts.

This means that a legacy device without event index support
will get more interrupts.
Sounds reasonable.

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


Re: [PATCH 2/2] virtio_balloon: free some memory from baloon on OOM

2014-10-13 Thread Michael S. Tsirkin
On Mon, Oct 13, 2014 at 04:02:52PM +1030, Rusty Russell wrote:
 Denis V. Lunev d...@parallels.com writes:
  From: Raushaniya Maksudova rmaksud...@parallels.com
 
  Excessive virtio_balloon inflation can cause invocation of OOM-killer,
  when Linux is under severe memory pressure. Various mechanisms are
  responsible for correct virtio_balloon memory management. Nevertheless
  it is often the case that these control tools does not have enough time
  to react on fast changing memory load. As a result OS runs out of memory
  and invokes OOM-killer. The balancing of memory by use of the virtio
  balloon should not cause the termination of processes while there are
  pages in the balloon. Now there is no way for virtio balloon driver to
  free some memory at the last moment before some process will be get
  killed by OOM-killer.
 
 This makes some amount of sense.

This reminds me of the balloon fs that Google once proposed.
This really needs to be controlled from host though.
At the moment host does not expect guest to deflate before
requests.
So as a minimum, add a feature bit for this.  what if you want a mix of
mandatory and optional balooning? I guess we can use multiple balloons,
is that the idea?


 But I suggest a few minor changes:
 
  +static int oom_vballoon_pages = OOM_VBALLOON_DEFAULT_PAGES;
  +module_param(oom_vballoon_pages, int, S_IRUSR | S_IWUSR);
  +MODULE_PARM_DESC(oom_vballoon_pages, pages to free on OOM);
 
 Since this is already prefixed with virtio_balloon. I suggest just
 calling it oom_pages.
 
  +static int virtballoon_oom_notify(struct notifier_block *self,
  + unsigned long dummy, void *parm)
  +{
  +   unsigned int num_freed_pages;
  +   unsigned long *freed = (unsigned long *)parm;
  +   struct virtio_balloon *vb = container_of((struct notifier_block *)self,
  +struct virtio_balloon, nb);
 
 Why cast self here?
 
  +   num_freed_pages = leak_balloon(vb, oom_vballoon_pages);
  +   update_balloon_size(vb);
  +   *freed += num_freed_pages;
  +
  +   return NOTIFY_OK;
  +}
 
 Cheers,
 Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 10/25] virtio: add API to enable VQs early

2014-10-13 Thread Michael S. Tsirkin
On Mon, Oct 13, 2014 at 05:22:39PM +1030, Rusty Russell wrote:
 Michael S. Tsirkin m...@redhat.com writes:
  virtio spec 0.9.X requires DRIVER_OK to be set before
  VQs are used, but some drivers use VQs before probe
  function returns.
  Since DRIVER_OK is set after probe, this violates the spec.
 
  Even though under virtio 1.0 transitional devices support this
  behaviour, we want to make it possible for those early callers to become
  spec compliant and eventually support non-transitional devices.
 
  Add API for drivers to call before using VQs.
 
  Sets DRIVER_OK internally.
 
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
 
 OK, this all looks good, but I think we should do two things:
 
 1) rename virtio_enable_vqs_early() to virtio_device_ready().

I considered this, I was concerned that the usage isn't clear from name:
- this has to be called before VQs are used
- this is optional, happens automatically after probe

device_ready seems to imply driver is fully initialized,
if it was it could just call probe.

But it's easy enough to make the change, so I'll go ahead and
post V4. It will be up to you which one to apply then.

 2) Add a BUG_ON in the virtio_ring code to make sure device is ready
before any ops are called.
 
 Cheers,
 Rusty.

On kick? This seems too expensive, especially considering that
we don't track the status.

Anyway, can be a patch on top?

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


[PATCH v4 00/25] virtio: fix spec compliance issues

2014-10-13 Thread Michael S. Tsirkin
Changes from v4:
rename virtio_enable_vqs_early() to virtio_device_ready()
Note: Rusty requested we add a BUG_ON in the virtio_ring code.
This can be done by a separate patch on top.
Good for bisectability in case BUG_ON starts triggering :)

Rusty, please review this, and consider for this merge window.

This fixes the following virtio spec compliance issues:
1. on restore, drivers use device before setting ACKNOWLEDGE and DRIVER bits
2. on probe, drivers aren't prepared to handle config interrupts
   arriving before probe returns
3. on probe, drivers use device before DRIVER_OK it set

Note that 1 is a clear violation of virtio spec 0.9 and 1.0,
so I am proposing the fix for stable. OTOH 2 is against 1.0 rules but
is a known documented bug in many drivers, so let's fix it going
forward, but it does not seem to be worth it to backport
the changes.

An error handling bugfix for virtio-net and a fix for a
theoretical race condition in virtio scsi is also included.

2 is merely a theoretical race condition, but it seems important
to address to make sure that changes to address 3 do not introduce
instability.

I also included patch to drop scan callback use from virtio
scsi: using this callback creates asymmetry between probe
and resume, and we actually had to add code comments so
people can figure out the flow. Code flow becomes clearer
with the new API.

After this change, the only user of the scan callback is virtio rng.
It's possible to modify it trivially and then drop scan callback
from core, but I'm rather inclined to look at ways to
make some rng core changes so that we don't need to
have so many variables tracking device state.
So this is deferred for now.

Michael S. Tsirkin (24):
  virtio_pci: fix virtio spec compliance on restore
  virtio: unify config_changed handling
  virtio-pci: move freeze/restore to virtio core
  virtio: defer config changed notifications
  virtio_blk: drop config_enable
  virtio-blk: drop config_mutex
  virtio_net: drop config_enable
  virtio-net: drop config_mutex
  virtio_net: minor cleanup
  virtio: add API to enable VQs early
  virtio_net: enable VQs early
  virtio_blk: enable VQs early
  virtio_console: enable VQs early
  9p/trans_virtio: enable VQs early
  virtio_net: fix use after free on allocation failure
  virtio_scsi: move kick event out from virtscsi_init
  virtio_blk: enable VQs early on restore
  virtio_scsi: enable VQs early on restore
  virtio_console: enable VQs early on restore
  virtio_net: enable VQs early on restore
  virtio_scsi: fix race on device removal
  virtio_balloon: enable VQs early on restore
  virtio_scsi: drop scan callback
  virtio-rng: refactor probe error handling

Paolo Bonzini (1):
  virito_scsi: use freezable WQ for events

 include/linux/virtio.h  |  14 +
 include/linux/virtio_config.h   |  17 ++
 drivers/block/virtio_blk.c  |  40 --
 drivers/char/hw_random/virtio-rng.c |  15 +++---
 drivers/char/virtio_console.c   |   4 ++
 drivers/misc/mic/card/mic_virtio.c  |   6 +--
 drivers/net/virtio_net.c|  44 +---
 drivers/s390/kvm/kvm_virtio.c   |   9 +---
 drivers/s390/kvm/virtio_ccw.c   |   6 +--
 drivers/scsi/virtio_scsi.c  |  42 +--
 drivers/virtio/virtio.c | 102 
 drivers/virtio/virtio_balloon.c |   2 +
 drivers/virtio/virtio_mmio.c|   7 +--
 drivers/virtio/virtio_pci.c |  33 ++--
 net/9p/trans_virtio.c   |   2 +
 15 files changed, 206 insertions(+), 137 deletions(-)

-- 
MST


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


[PATCH v4 01/25] virtio_pci: fix virtio spec compliance on restore

2014-10-13 Thread Michael S. Tsirkin
On restore, virtio pci does the following:
+ set features
+ init vqs etc - device can be used at this point!
+ set ACKNOWLEDGE,DRIVER and DRIVER_OK status bits

This is in violation of the virtio spec, which
requires the following order:
- ACKNOWLEDGE
- DRIVER
- init vqs
- DRIVER_OK

This behaviour will break with hypervisors that assume spec compliant
behaviour.  It seems like a good idea to have this patch applied to
stable branches to reduce the support butden for the hypervisors.

Cc: sta...@vger.kernel.org
Cc: Amit Shah amit.s...@redhat.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/virtio/virtio_pci.c | 33 ++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 3d1463c..add40d0 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -789,6 +789,7 @@ static int virtio_pci_restore(struct device *dev)
struct pci_dev *pci_dev = to_pci_dev(dev);
struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
struct virtio_driver *drv;
+   unsigned status = 0;
int ret;
 
drv = container_of(vp_dev-vdev.dev.driver,
@@ -799,14 +800,40 @@ static int virtio_pci_restore(struct device *dev)
return ret;
 
pci_set_master(pci_dev);
+   /* We always start by resetting the device, in case a previous
+* driver messed it up. */
+   vp_reset(vp_dev-vdev);
+
+   /* Acknowledge that we've seen the device. */
+   status |= VIRTIO_CONFIG_S_ACKNOWLEDGE;
+   vp_set_status(vp_dev-vdev, status);
+
+   /* Maybe driver failed before freeze.
+* Restore the failed status, for debugging. */
+   status |= vp_dev-saved_status  VIRTIO_CONFIG_S_FAILED;
+   vp_set_status(vp_dev-vdev, status);
+
+   if (!drv)
+   return 0;
+
+   /* We have a driver! */
+   status |= VIRTIO_CONFIG_S_DRIVER;
+   vp_set_status(vp_dev-vdev, status);
+
vp_finalize_features(vp_dev-vdev);
 
-   if (drv  drv-restore)
+   if (drv-restore) {
ret = drv-restore(vp_dev-vdev);
+   if (ret) {
+   status |= VIRTIO_CONFIG_S_FAILED;
+   vp_set_status(vp_dev-vdev, status);
+   return ret;
+   }
+   }
 
/* Finally, tell the device we're all set */
-   if (!ret)
-   vp_set_status(vp_dev-vdev, vp_dev-saved_status);
+   status |= VIRTIO_CONFIG_S_DRIVER_OK;
+   vp_set_status(vp_dev-vdev, status);
 
return ret;
 }
-- 
MST


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


[PATCH v4 02/25] virtio: unify config_changed handling

2014-10-13 Thread Michael S. Tsirkin
Replace duplicated code in all transports with a single wrapper in
virtio.c.

The only functional change is in virtio_mmio.c: if a buggy device sends
us an interrupt before driver is set, we previously returned IRQ_NONE,
now we return IRQ_HANDLED.

As this must not happen in practice, this does not look like a big deal.

See also commit 3fff0179e33cd7d0a688dab65700c46ad089e934
virtio-pci: do not oops on config change if driver not loaded.
for the original motivation behind the driver check.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 include/linux/virtio.h | 2 ++
 drivers/misc/mic/card/mic_virtio.c | 6 +-
 drivers/s390/kvm/kvm_virtio.c  | 9 +
 drivers/s390/kvm/virtio_ccw.c  | 6 +-
 drivers/virtio/virtio.c| 9 +
 drivers/virtio/virtio_mmio.c   | 7 ++-
 drivers/virtio/virtio_pci.c| 6 +-
 7 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index b46671e..3c19bd3 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -108,6 +108,8 @@ void unregister_virtio_device(struct virtio_device *dev);
 
 void virtio_break_device(struct virtio_device *dev);
 
+void virtio_config_changed(struct virtio_device *dev);
+
 /**
  * virtio_driver - operations for a virtio I/O driver
  * @driver: underlying device driver (populate name and owner).
diff --git a/drivers/misc/mic/card/mic_virtio.c 
b/drivers/misc/mic/card/mic_virtio.c
index f14b600..e647947 100644
--- a/drivers/misc/mic/card/mic_virtio.c
+++ b/drivers/misc/mic/card/mic_virtio.c
@@ -462,16 +462,12 @@ static void mic_handle_config_change(struct 
mic_device_desc __iomem *d,
struct mic_device_ctrl __iomem *dc
= (void __iomem *)d + mic_aligned_desc_size(d);
struct mic_vdev *mvdev = (struct mic_vdev *)ioread64(dc-vdev);
-   struct virtio_driver *drv;
 
if (ioread8(dc-config_change) != MIC_VIRTIO_PARAM_CONFIG_CHANGED)
return;
 
dev_dbg(mdrv-dev, %s %d\n, __func__, __LINE__);
-   drv = container_of(mvdev-vdev.dev.driver,
-   struct virtio_driver, driver);
-   if (drv-config_changed)
-   drv-config_changed(mvdev-vdev);
+   virtio_config_changed(mvdev-vdev);
iowrite8(1, dc-guest_ack);
 }
 
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index a134965..6431290 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -406,15 +406,8 @@ static void kvm_extint_handler(struct ext_code ext_code,
 
switch (param) {
case VIRTIO_PARAM_CONFIG_CHANGED:
-   {
-   struct virtio_driver *drv;
-   drv = container_of(vq-vdev-dev.driver,
-  struct virtio_driver, driver);
-   if (drv-config_changed)
-   drv-config_changed(vq-vdev);
-
+   virtio_config_changed(vq-vdev);
break;
-   }
case VIRTIO_PARAM_DEV_ADD:
schedule_work(hotplug_work);
break;
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index d2c0b44..6cbe6ef 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -940,11 +940,7 @@ static void virtio_ccw_int_handler(struct ccw_device *cdev,
vring_interrupt(0, vq);
}
if (test_bit(0, vcdev-indicators2)) {
-   drv = container_of(vcdev-vdev.dev.driver,
-  struct virtio_driver, driver);
-
-   if (drv  drv-config_changed)
-   drv-config_changed(vcdev-vdev);
+   virtio_config_changed(vcdev-vdev);
clear_bit(0, vcdev-indicators2);
}
 }
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index fed0ce1..3980687 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -239,6 +239,15 @@ void unregister_virtio_device(struct virtio_device *dev)
 }
 EXPORT_SYMBOL_GPL(unregister_virtio_device);
 
+void virtio_config_changed(struct virtio_device *dev)
+{
+   struct virtio_driver *drv = drv_to_virtio(dev-dev.driver);
+
+   if (drv  drv-config_changed)
+   drv-config_changed(dev);
+}
+EXPORT_SYMBOL_GPL(virtio_config_changed);
+
 static int virtio_init(void)
 {
if (bus_register(virtio_bus) != 0)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index c600ccf..ef9a165 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -234,8 +234,6 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
 {
struct virtio_mmio_device *vm_dev = opaque;
struct virtio_mmio_vq_info *info;
-   struct virtio_driver *vdrv = container_of(vm_dev-vdev.dev.driver,
-   struct virtio_driver, driver);
unsigned long status;

[PATCH v4 05/25] virtio_blk: drop config_enable

2014-10-13 Thread Michael S. Tsirkin
Now that virtio core ensures config changes don't
arrive during probing, drop config_enable flag
in virtio blk.
On removal, flush is now sufficient to guarantee that
no change work is queued.

This help simplify the driver, and will allow
setting DRIVER_OK earlier without losing config
change notifications.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 drivers/block/virtio_blk.c | 23 ---
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 0a58140..91272f1a 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -44,9 +44,6 @@ struct virtio_blk
/* Lock for config space updates */
struct mutex config_lock;
 
-   /* enable config space updates */
-   bool config_enable;
-
/* What host tells us, plus 2 for header  tailer. */
unsigned int sg_elems;
 
@@ -348,8 +345,6 @@ static void virtblk_config_changed_work(struct work_struct 
*work)
u64 capacity, size;
 
mutex_lock(vblk-config_lock);
-   if (!vblk-config_enable)
-   goto done;
 
/* Host must always specify the capacity. */
virtio_cread(vdev, struct virtio_blk_config, capacity, capacity);
@@ -374,7 +369,7 @@ static void virtblk_config_changed_work(struct work_struct 
*work)
set_capacity(vblk-disk, capacity);
revalidate_disk(vblk-disk);
kobject_uevent_env(disk_to_dev(vblk-disk)-kobj, KOBJ_CHANGE, envp);
-done:
+
mutex_unlock(vblk-config_lock);
 }
 
@@ -609,7 +604,6 @@ static int virtblk_probe(struct virtio_device *vdev)
mutex_init(vblk-config_lock);
 
INIT_WORK(vblk-config_work, virtblk_config_changed_work);
-   vblk-config_enable = true;
 
err = init_vq(vblk);
if (err)
@@ -771,10 +765,8 @@ static void virtblk_remove(struct virtio_device *vdev)
int index = vblk-index;
int refc;
 
-   /* Prevent config work handler from accessing the device. */
-   mutex_lock(vblk-config_lock);
-   vblk-config_enable = false;
-   mutex_unlock(vblk-config_lock);
+   /* Make sure no work handler is accessing the device. */
+   flush_work(vblk-config_work);
 
del_gendisk(vblk-disk);
blk_cleanup_queue(vblk-disk-queue);
@@ -784,8 +776,6 @@ static void virtblk_remove(struct virtio_device *vdev)
/* Stop all the virtqueues. */
vdev-config-reset(vdev);
 
-   flush_work(vblk-config_work);
-
refc = atomic_read(disk_to_dev(vblk-disk)-kobj.kref.refcount);
put_disk(vblk-disk);
vdev-config-del_vqs(vdev);
@@ -805,11 +795,7 @@ static int virtblk_freeze(struct virtio_device *vdev)
/* Ensure we don't receive any more interrupts */
vdev-config-reset(vdev);
 
-   /* Prevent config work handler from accessing the device. */
-   mutex_lock(vblk-config_lock);
-   vblk-config_enable = false;
-   mutex_unlock(vblk-config_lock);
-
+   /* Make sure no work handler is accessing the device. */
flush_work(vblk-config_work);
 
blk_mq_stop_hw_queues(vblk-disk-queue);
@@ -823,7 +809,6 @@ static int virtblk_restore(struct virtio_device *vdev)
struct virtio_blk *vblk = vdev-priv;
int ret;
 
-   vblk-config_enable = true;
ret = init_vq(vdev-priv);
if (!ret)
blk_mq_start_stopped_hw_queues(vblk-disk-queue, true);
-- 
MST


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


[PATCH v4 04/25] virtio: defer config changed notifications

2014-10-13 Thread Michael S. Tsirkin
Defer config changed notifications that arrive during
probe/scan/freeze/restore.

This will allow drivers to set DRIVER_OK earlier, without worrying about
racing with config change interrupts.

This change will also benefit old hypervisors (before 2009)
that send interrupts without checking DRIVER_OK: previously,
the callback could race with driver-specific initialization.

This will also help simplify drivers.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 include/linux/virtio.h  |  6 ++
 drivers/virtio/virtio.c | 57 +
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 8df7ba8..5636b11 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -79,6 +79,9 @@ bool virtqueue_is_broken(struct virtqueue *vq);
  * virtio_device - representation of a device using virtio
  * @index: unique position on the virtio bus
  * @failed: saved value for CONFIG_S_FAILED bit (for restore)
+ * @config_enabled: configuration change reporting enabled
+ * @config_changed: configuration change reported while disabled
+ * @config_lock: protects configuration change reporting
  * @dev: underlying device.
  * @id: the device type identification (used to match it with a driver).
  * @config: the configuration ops for this device.
@@ -90,6 +93,9 @@ bool virtqueue_is_broken(struct virtqueue *vq);
 struct virtio_device {
int index;
bool failed;
+   bool config_enabled;
+   bool config_changed;
+   spinlock_t config_lock;
struct device dev;
struct virtio_device_id id;
const struct virtio_config_ops *config;
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 8216b73..2536701 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -117,6 +117,42 @@ void virtio_check_driver_offered_feature(const struct 
virtio_device *vdev,
 }
 EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature);
 
+static void __virtio_config_changed(struct virtio_device *dev)
+{
+   struct virtio_driver *drv = drv_to_virtio(dev-dev.driver);
+
+   if (!dev-config_enabled)
+   dev-config_changed = true;
+   else if (drv  drv-config_changed)
+   drv-config_changed(dev);
+}
+
+void virtio_config_changed(struct virtio_device *dev)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(dev-config_lock, flags);
+   __virtio_config_changed(dev);
+   spin_unlock_irqrestore(dev-config_lock, flags);
+}
+EXPORT_SYMBOL_GPL(virtio_config_changed);
+
+static void virtio_config_disable(struct virtio_device *dev)
+{
+   spin_lock_irq(dev-config_lock);
+   dev-config_enabled = false;
+   spin_unlock_irq(dev-config_lock);
+}
+
+static void virtio_config_enable(struct virtio_device *dev)
+{
+   spin_lock_irq(dev-config_lock);
+   dev-config_enabled = true;
+   __virtio_config_changed(dev);
+   dev-config_changed = false;
+   spin_unlock_irq(dev-config_lock);
+}
+
 static int virtio_dev_probe(struct device *_d)
 {
int err, i;
@@ -153,6 +189,8 @@ static int virtio_dev_probe(struct device *_d)
add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
if (drv-scan)
drv-scan(dev);
+
+   virtio_config_enable(dev);
}
 
return err;
@@ -163,6 +201,8 @@ static int virtio_dev_remove(struct device *_d)
struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_driver *drv = drv_to_virtio(dev-dev.driver);
 
+   virtio_config_disable(dev);
+
drv-remove(dev);
 
/* Driver should have reset device. */
@@ -211,6 +251,10 @@ int register_virtio_device(struct virtio_device *dev)
dev-index = err;
dev_set_name(dev-dev, virtio%u, dev-index);
 
+   spin_lock_init(dev-config_lock);
+   dev-config_enabled = false;
+   dev-config_changed = false;
+
/* We always start by resetting the device, in case a previous
 * driver messed it up.  This also tests that code path a little. */
dev-config-reset(dev);
@@ -239,20 +283,13 @@ void unregister_virtio_device(struct virtio_device *dev)
 }
 EXPORT_SYMBOL_GPL(unregister_virtio_device);
 
-void virtio_config_changed(struct virtio_device *dev)
-{
-   struct virtio_driver *drv = drv_to_virtio(dev-dev.driver);
-
-   if (drv  drv-config_changed)
-   drv-config_changed(dev);
-}
-EXPORT_SYMBOL_GPL(virtio_config_changed);
-
 #ifdef CONFIG_PM_SLEEP
 int virtio_device_freeze(struct virtio_device *dev)
 {
struct virtio_driver *drv = drv_to_virtio(dev-dev.driver);
 
+   virtio_config_disable(dev);
+
dev-failed = dev-config-get_status(dev)  VIRTIO_CONFIG_S_FAILED;
 
if (drv  drv-freeze)
@@ -297,6 +334,8 @@ int virtio_device_restore(struct virtio_device *dev)
/* Finally, tell the device we're all set */

[PATCH v4 06/25] virtio-blk: drop config_mutex

2014-10-13 Thread Michael S. Tsirkin
config_mutex served two purposes: prevent multiple concurrent config
change handlers, and synchronize access to config_enable flag.

Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1
workqueue: make all workqueues non-reentrant
all workqueues are non-reentrant, and config_enable
is now gone.

Get rid of the unnecessary lock.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 drivers/block/virtio_blk.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 91272f1a..89ba8d6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -41,9 +41,6 @@ struct virtio_blk
/* Process context for config space updates */
struct work_struct config_work;
 
-   /* Lock for config space updates */
-   struct mutex config_lock;
-
/* What host tells us, plus 2 for header  tailer. */
unsigned int sg_elems;
 
@@ -344,8 +341,6 @@ static void virtblk_config_changed_work(struct work_struct 
*work)
char *envp[] = { RESIZE=1, NULL };
u64 capacity, size;
 
-   mutex_lock(vblk-config_lock);
-
/* Host must always specify the capacity. */
virtio_cread(vdev, struct virtio_blk_config, capacity, capacity);
 
@@ -369,8 +364,6 @@ static void virtblk_config_changed_work(struct work_struct 
*work)
set_capacity(vblk-disk, capacity);
revalidate_disk(vblk-disk);
kobject_uevent_env(disk_to_dev(vblk-disk)-kobj, KOBJ_CHANGE, envp);
-
-   mutex_unlock(vblk-config_lock);
 }
 
 static void virtblk_config_changed(struct virtio_device *vdev)
@@ -601,7 +594,6 @@ static int virtblk_probe(struct virtio_device *vdev)
 
vblk-vdev = vdev;
vblk-sg_elems = sg_elems;
-   mutex_init(vblk-config_lock);
 
INIT_WORK(vblk-config_work, virtblk_config_changed_work);
 
-- 
MST


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


[PATCH v4 08/25] virtio-net: drop config_mutex

2014-10-13 Thread Michael S. Tsirkin
config_mutex served two purposes: prevent multiple concurrent config
change handlers, and synchronize access to config_enable flag.

Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1
workqueue: make all workqueues non-reentrant
all workqueues are non-reentrant, and config_enable
is now gone.

Get rid of the unnecessary lock.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 drivers/net/virtio_net.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 743fb04..23e4a69 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -132,9 +132,6 @@ struct virtnet_info {
/* Work struct for config space updates */
struct work_struct config_work;
 
-   /* Lock for config space updates */
-   struct mutex config_lock;
-
/* Does the affinity hint is set for virtqueues? */
bool affinity_hint_set;
 
@@ -1404,7 +1401,6 @@ static void virtnet_config_changed_work(struct 
work_struct *work)
container_of(work, struct virtnet_info, config_work);
u16 v;
 
-   mutex_lock(vi-config_lock);
if (virtio_cread_feature(vi-vdev, VIRTIO_NET_F_STATUS,
 struct virtio_net_config, status, v)  0)
goto done;
@@ -1430,7 +1426,7 @@ static void virtnet_config_changed_work(struct 
work_struct *work)
netif_tx_stop_all_queues(vi-dev);
}
 done:
-   mutex_unlock(vi-config_lock);
+   return;
 }
 
 static void virtnet_config_changed(struct virtio_device *vdev)
@@ -1751,7 +1747,6 @@ static int virtnet_probe(struct virtio_device *vdev)
u64_stats_init(virtnet_stats-rx_syncp);
}
 
-   mutex_init(vi-config_lock);
INIT_WORK(vi-config_work, virtnet_config_changed_work);
 
/* If we can receive ANY GSO packets, we must allocate large ones. */
-- 
MST


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


[PATCH v4 09/25] virtio_net: minor cleanup

2014-10-13 Thread Michael S. Tsirkin
goto done;
done:
return;
is ugly, it was put there to make diff review easier.
replace by open-coded return.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Acked-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 drivers/net/virtio_net.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 23e4a69..ef04d23 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1403,7 +1403,7 @@ static void virtnet_config_changed_work(struct 
work_struct *work)
 
if (virtio_cread_feature(vi-vdev, VIRTIO_NET_F_STATUS,
 struct virtio_net_config, status, v)  0)
-   goto done;
+   return;
 
if (v  VIRTIO_NET_S_ANNOUNCE) {
netdev_notify_peers(vi-dev);
@@ -1414,7 +1414,7 @@ static void virtnet_config_changed_work(struct 
work_struct *work)
v = VIRTIO_NET_S_LINK_UP;
 
if (vi-status == v)
-   goto done;
+   return;
 
vi-status = v;
 
@@ -1425,8 +1425,6 @@ static void virtnet_config_changed_work(struct 
work_struct *work)
netif_carrier_off(vi-dev);
netif_tx_stop_all_queues(vi-dev);
}
-done:
-   return;
 }
 
 static void virtnet_config_changed(struct virtio_device *vdev)
-- 
MST


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


[PATCH v4 07/25] virtio_net: drop config_enable

2014-10-13 Thread Michael S. Tsirkin
Now that virtio core ensures config changes don't arrive during probing,
drop config_enable flag in virtio net.
On removal, flush is now sufficient to guarantee that no change work is
queued.

This help simplify the driver, and will allow setting DRIVER_OK earlier
without losing config change notifications.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 drivers/net/virtio_net.c | 27 ---
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 59caa06..743fb04 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -123,9 +123,6 @@ struct virtnet_info {
/* Host can handle any s/g split between our header and packet data */
bool any_header_sg;
 
-   /* enable config space updates */
-   bool config_enable;
-
/* Active statistics */
struct virtnet_stats __percpu *stats;
 
@@ -1408,9 +1405,6 @@ static void virtnet_config_changed_work(struct 
work_struct *work)
u16 v;
 
mutex_lock(vi-config_lock);
-   if (!vi-config_enable)
-   goto done;
-
if (virtio_cread_feature(vi-vdev, VIRTIO_NET_F_STATUS,
 struct virtio_net_config, status, v)  0)
goto done;
@@ -1758,7 +1752,6 @@ static int virtnet_probe(struct virtio_device *vdev)
}
 
mutex_init(vi-config_lock);
-   vi-config_enable = true;
INIT_WORK(vi-config_work, virtnet_config_changed_work);
 
/* If we can receive ANY GSO packets, we must allocate large ones. */
@@ -1875,17 +1868,13 @@ static void virtnet_remove(struct virtio_device *vdev)
 
unregister_hotcpu_notifier(vi-nb);
 
-   /* Prevent config work handler from accessing the device. */
-   mutex_lock(vi-config_lock);
-   vi-config_enable = false;
-   mutex_unlock(vi-config_lock);
+   /* Make sure no work handler is accessing the device. */
+   flush_work(vi-config_work);
 
unregister_netdev(vi-dev);
 
remove_vq_common(vi);
 
-   flush_work(vi-config_work);
-
free_percpu(vi-stats);
free_netdev(vi-dev);
 }
@@ -1898,10 +1887,8 @@ static int virtnet_freeze(struct virtio_device *vdev)
 
unregister_hotcpu_notifier(vi-nb);
 
-   /* Prevent config work handler from accessing the device */
-   mutex_lock(vi-config_lock);
-   vi-config_enable = false;
-   mutex_unlock(vi-config_lock);
+   /* Make sure no work handler is accessing the device */
+   flush_work(vi-config_work);
 
netif_device_detach(vi-dev);
cancel_delayed_work_sync(vi-refill);
@@ -1916,8 +1903,6 @@ static int virtnet_freeze(struct virtio_device *vdev)
 
remove_vq_common(vi);
 
-   flush_work(vi-config_work);
-
return 0;
 }
 
@@ -1941,10 +1926,6 @@ static int virtnet_restore(struct virtio_device *vdev)
 
netif_device_attach(vi-dev);
 
-   mutex_lock(vi-config_lock);
-   vi-config_enable = true;
-   mutex_unlock(vi-config_lock);
-
rtnl_lock();
virtnet_set_queues(vi, vi-curr_queue_pairs);
rtnl_unlock();
-- 
MST


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


[PATCH v4 12/25] virtio_blk: enable VQs early

2014-10-13 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after probe returns, virtio block violated this
rule by calling add_disk, which causes the VQ to be used directly within
probe.

To fix, call virtio_device_ready before using VQs.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 drivers/block/virtio_blk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 89ba8d6..46b04bf 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -719,6 +719,8 @@ static int virtblk_probe(struct virtio_device *vdev)
if (!err  opt_io_size)
blk_queue_io_opt(q, blk_size * opt_io_size);
 
+   virtio_device_ready(vdev);
+
add_disk(vblk-disk);
err = device_create_file(disk_to_dev(vblk-disk), dev_attr_serial);
if (err)
-- 
MST


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


[PATCH v4 11/25] virtio_net: enable VQs early

2014-10-13 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after probe returns, virtio net violated this
rule by using receive VQs within probe.

To fix, call virtio_device_ready before using VQs.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 drivers/net/virtio_net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ef04d23..430f3ae 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1792,6 +1792,8 @@ static int virtnet_probe(struct virtio_device *vdev)
goto free_vqs;
}
 
+   virtio_device_ready(vdev);
+
/* Last of all, set up some receive buffers. */
for (i = 0; i  vi-curr_queue_pairs; i++) {
try_fill_recv(vi-rq[i], GFP_KERNEL);
-- 
MST


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


[PATCH v4 13/25] virtio_console: enable VQs early

2014-10-13 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after probe returns, virtio console violated this
rule by adding inbufs, which causes the VQ to be used directly within
probe.

To fix, call virtio_device_ready before using VQs.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/char/virtio_console.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index b585b47..6ebe8f6 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1449,6 +1449,8 @@ static int add_port(struct ports_device *portdev, u32 id)
spin_lock_init(port-outvq_lock);
init_waitqueue_head(port-waitqueue);
 
+   virtio_device_ready(portdev-vdev);
+
/* Fill the in_vq with buffers so the host can send us data. */
nr_added_bufs = fill_queue(port-in_vq, port-inbuf_lock);
if (!nr_added_bufs) {
-- 
MST


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


[PATCH v4 15/25] virtio_net: fix use after free on allocation failure

2014-10-13 Thread Michael S. Tsirkin
In the extremely unlikely event that driver initialization fails after
RX buffers are added, virtio net frees RX buffers while VQs are
still active, potentially causing device to use a freed buffer.

To fix, reset device first - same as we do on device removal.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 drivers/net/virtio_net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 430f3ae..3551417 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1830,6 +1830,8 @@ static int virtnet_probe(struct virtio_device *vdev)
return 0;
 
 free_recv_bufs:
+   vi-vdev-config-reset(vdev);
+
free_receive_bufs(vi);
unregister_netdev(dev);
 free_vqs:
-- 
MST


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


[PATCH v4 14/25] 9p/trans_virtio: enable VQs early

2014-10-13 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after probe returns, but virtio 9p device
adds self to channel list within probe, at which point VQ can be
used in violation of the spec.

To fix, call virtio_device_ready before using VQs.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 net/9p/trans_virtio.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 6940d8f..766ba48 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -575,6 +575,8 @@ static int p9_virtio_probe(struct virtio_device *vdev)
/* Ceiling limit to avoid denial of service attacks */
chan-p9_max_pages = nr_free_buffer_pages()/4;
 
+   virtio_device_ready(vdev);
+
mutex_lock(virtio_9p_lock);
list_add_tail(chan-chan_list, virtio_chan_list);
mutex_unlock(virtio_9p_lock);
-- 
MST


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


[PATCH v4 17/25] virtio_blk: enable VQs early on restore

2014-10-13 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after restore returns, virtio block violated
this rule on restore by restarting queues, which might in theory
cause the VQ to be used directly within restore.

To fix, call virtio_device_ready before using starting queues.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/block/virtio_blk.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 46b04bf..1c95af5 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -804,10 +804,13 @@ static int virtblk_restore(struct virtio_device *vdev)
int ret;
 
ret = init_vq(vdev-priv);
-   if (!ret)
-   blk_mq_start_stopped_hw_queues(vblk-disk-queue, true);
+   if (ret)
+   return ret;
+
+   virtio_device_ready(vdev);
 
-   return ret;
+   blk_mq_start_stopped_hw_queues(vblk-disk-queue, true);
+   return 0;
 }
 #endif
 
-- 
MST


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


[PATCH v4 18/25] virtio_scsi: enable VQs early on restore

2014-10-13 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after restore returns, virtio scsi violated
this rule on restore by kicking event vq within restore.

To fix, call virtio_device_ready before using event queue.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/scsi/virtio_scsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 0642ce3..2b565b3 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -1054,6 +1054,8 @@ static int virtscsi_restore(struct virtio_device *vdev)
return err;
}
 
+   virtio_device_ready(vdev);
+
if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
virtscsi_kick_event_all(vscsi);
 
-- 
MST


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


[PATCH v4 16/25] virtio_scsi: move kick event out from virtscsi_init

2014-10-13 Thread Michael S. Tsirkin
We currently kick event within virtscsi_init,
before host is fully initialized.

This can in theory confuse guest if device
consumes the buffers immediately.

To fix,  move virtscsi_kick_event_all out to scan/restore.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/scsi/virtio_scsi.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index eee1bc0..0642ce3 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -853,7 +853,11 @@ static void virtscsi_init_vq(struct virtio_scsi_vq 
*virtscsi_vq,
 
 static void virtscsi_scan(struct virtio_device *vdev)
 {
-   struct Scsi_Host *shost = (struct Scsi_Host *)vdev-priv;
+   struct Scsi_Host *shost = virtio_scsi_host(vdev);
+   struct virtio_scsi *vscsi = shost_priv(shost);
+
+   if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
+   virtscsi_kick_event_all(vscsi);
 
scsi_scan_host(shost);
 }
@@ -916,9 +920,6 @@ static int virtscsi_init(struct virtio_device *vdev,
virtscsi_config_set(vdev, cdb_size, VIRTIO_SCSI_CDB_SIZE);
virtscsi_config_set(vdev, sense_size, VIRTIO_SCSI_SENSE_SIZE);
 
-   if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
-   virtscsi_kick_event_all(vscsi);
-
err = 0;
 
 out:
@@ -1048,8 +1049,13 @@ static int virtscsi_restore(struct virtio_device *vdev)
return err;
 
err = register_hotcpu_notifier(vscsi-nb);
-   if (err)
+   if (err) {
vdev-config-del_vqs(vdev);
+   return err;
+   }
+
+   if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
+   virtscsi_kick_event_all(vscsi);
 
return err;
 }
-- 
MST


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


[PATCH v4 20/25] virtio_net: enable VQs early on restore

2014-10-13 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after restore returns, virtio net violated this
rule by using receive VQs within restore.

To fix, call virtio_device_ready before using VQs.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 drivers/net/virtio_net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 3551417..6b6e136 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1912,6 +1912,8 @@ static int virtnet_restore(struct virtio_device *vdev)
if (err)
return err;
 
+   virtio_device_ready(vdev);
+
if (netif_running(vi-dev)) {
for (i = 0; i  vi-curr_queue_pairs; i++)
if (!try_fill_recv(vi-rq[i], GFP_KERNEL))
-- 
MST


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


[PATCH v4 19/25] virtio_console: enable VQs early on restore

2014-10-13 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after resume returns, virtio console violated this
rule by adding inbufs, which causes the VQ to be used directly within
restore.

To fix, call virtio_device_ready before using VQs.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/char/virtio_console.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 6ebe8f6..2ae843f 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2184,6 +2184,8 @@ static int virtcons_restore(struct virtio_device *vdev)
if (ret)
return ret;
 
+   virtio_device_ready(portdev-vdev);
+
if (use_multiport(portdev))
fill_queue(portdev-c_ivq, portdev-c_ivq_lock);
 
-- 
MST


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


[PATCH v4 22/25] virtio_scsi: fix race on device removal

2014-10-13 Thread Michael S. Tsirkin
We cancel event work on device removal, but an interrupt
could trigger immediately after this, and queue it
again.

To fix, set a flag.

Loosely based on patch by Paolo Bonzini

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/scsi/virtio_scsi.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 501838d..327eba0 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -110,6 +110,9 @@ struct virtio_scsi {
/* CPU hotplug notifier */
struct notifier_block nb;
 
+   /* Protected by event_vq lock */
+   bool stop_events;
+
struct virtio_scsi_vq ctrl_vq;
struct virtio_scsi_vq event_vq;
struct virtio_scsi_vq req_vqs[];
@@ -303,6 +306,11 @@ static void virtscsi_cancel_event_work(struct virtio_scsi 
*vscsi)
 {
int i;
 
+   /* Stop scheduling work before calling cancel_work_sync.  */
+   spin_lock_irq(vscsi-event_vq.vq_lock);
+   vscsi-stop_events = true;
+   spin_unlock_irq(vscsi-event_vq.vq_lock);
+
for (i = 0; i  VIRTIO_SCSI_EVENT_LEN; i++)
cancel_work_sync(vscsi-event_list[i].work);
 }
@@ -390,7 +398,8 @@ static void virtscsi_complete_event(struct virtio_scsi 
*vscsi, void *buf)
 {
struct virtio_scsi_event_node *event_node = buf;
 
-   queue_work(system_freezable_wq, event_node-work);
+   if (!vscsi-stop_events)
+   queue_work(system_freezable_wq, event_node-work);
 }
 
 static void virtscsi_event_done(struct virtqueue *vq)
-- 
MST


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


[PATCH v4 21/25] virito_scsi: use freezable WQ for events

2014-10-13 Thread Michael S. Tsirkin
From: Paolo Bonzini pbonz...@redhat.com

Michael S. Tsirkin noticed a race condition:
we reset device on freeze, but system WQ is still
running so it might try adding bufs to a VQ meanwhile.

To fix, switch to handling events from the freezable WQ.

Reported-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/scsi/virtio_scsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 2b565b3..501838d 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -390,7 +390,7 @@ static void virtscsi_complete_event(struct virtio_scsi 
*vscsi, void *buf)
 {
struct virtio_scsi_event_node *event_node = buf;
 
-   schedule_work(event_node-work);
+   queue_work(system_freezable_wq, event_node-work);
 }
 
 static void virtscsi_event_done(struct virtqueue *vq)
-- 
MST


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


[PATCH v4 24/25] virtio_scsi: drop scan callback

2014-10-13 Thread Michael S. Tsirkin
Enable VQs early like we do for restore.
This makes it possible to drop the scan callback,
moving scanning into the probe function, and making
code simpler.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/scsi/virtio_scsi.c | 23 +++
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 327eba0..5f022ff 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -860,17 +860,6 @@ static void virtscsi_init_vq(struct virtio_scsi_vq 
*virtscsi_vq,
virtscsi_vq-vq = vq;
 }
 
-static void virtscsi_scan(struct virtio_device *vdev)
-{
-   struct Scsi_Host *shost = virtio_scsi_host(vdev);
-   struct virtio_scsi *vscsi = shost_priv(shost);
-
-   if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
-   virtscsi_kick_event_all(vscsi);
-
-   scsi_scan_host(shost);
-}
-
 static void virtscsi_remove_vqs(struct virtio_device *vdev)
 {
struct Scsi_Host *sh = virtio_scsi_host(vdev);
@@ -1007,10 +996,13 @@ static int virtscsi_probe(struct virtio_device *vdev)
err = scsi_add_host(shost, vdev-dev);
if (err)
goto scsi_add_host_failed;
-   /*
-* scsi_scan_host() happens in virtscsi_scan() via virtio_driver-scan()
-* after VIRTIO_CONFIG_S_DRIVER_OK has been set..
-*/
+
+   virtio_device_ready(vdev);
+
+   if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
+   virtscsi_kick_event_all(vscsi);
+
+   scsi_scan_host(shost);
return 0;
 
 scsi_add_host_failed:
@@ -1090,7 +1082,6 @@ static struct virtio_driver virtio_scsi_driver = {
.driver.owner = THIS_MODULE,
.id_table = id_table,
.probe = virtscsi_probe,
-   .scan = virtscsi_scan,
 #ifdef CONFIG_PM_SLEEP
.freeze = virtscsi_freeze,
.restore = virtscsi_restore,
-- 
MST


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


[PATCH v4 23/25] virtio_balloon: enable VQs early on restore

2014-10-13 Thread Michael S. Tsirkin
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after resume returns, virtio balloon
violated this rule by adding bufs, which causes the VQ to be used
directly within restore.

To fix, call virtio_device_ready before using VQ.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/virtio/virtio_balloon.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 25ebe8e..9629fad 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -538,6 +538,8 @@ static int virtballoon_restore(struct virtio_device *vdev)
if (ret)
return ret;
 
+   virtio_device_ready(vdev);
+
fill_balloon(vb, towards_target(vb));
update_balloon_size(vb);
return 0;
-- 
MST


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


[PATCH v4 25/25] virtio-rng: refactor probe error handling

2014-10-13 Thread Michael S. Tsirkin
Code like
vi-vq = NULL;
kfree(vi)
does not make sense.

Clean it up, use goto error labels for cleanup.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 drivers/char/hw_random/virtio-rng.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/char/hw_random/virtio-rng.c 
b/drivers/char/hw_random/virtio-rng.c
index 132c9cc..72295ea 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -109,8 +109,8 @@ static int probe_common(struct virtio_device *vdev)
 
vi-index = index = ida_simple_get(rng_index_ida, 0, 0, GFP_KERNEL);
if (index  0) {
-   kfree(vi);
-   return index;
+   err = index;
+   goto err_ida;
}
sprintf(vi-name, virtio_rng.%d, index);
init_completion(vi-have_data);
@@ -128,13 +128,16 @@ static int probe_common(struct virtio_device *vdev)
vi-vq = virtio_find_single_vq(vdev, random_recv_done, input);
if (IS_ERR(vi-vq)) {
err = PTR_ERR(vi-vq);
-   vi-vq = NULL;
-   kfree(vi);
-   ida_simple_remove(rng_index_ida, index);
-   return err;
+   goto err_find;
}
 
return 0;
+
+err_find:
+   ida_simple_remove(rng_index_ida, index);
+err_ida:
+   kfree(vi);
+   return err;
 }
 
 static void remove_common(struct virtio_device *vdev)
-- 
MST


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


Re: [PATCH RFC 08/11] virtio_blk: use virtio v1.0 endian

2014-10-13 Thread Cornelia Huck
On Mon, 13 Oct 2014 16:28:32 +1030
Rusty Russell ru...@rustcorp.com.au wrote:

 Cornelia Huck cornelia.h...@de.ibm.com writes:
  Note that we care only about the fields still in use for virtio v1.0.
 
  Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com
  Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
  Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
 
 Hi Cornelia,
 
 These patches all look good; 

Cool, thanks.

 I'm a bit nervous about our testing
 missing some conversion, so we'll need qemu patches for PCI so we can
 test on other platforms too.

The devices I looked at (blk and net) are probably OK as ccw needs to
convert always. I agree that we want to test on other platforms as
well, but unfortunately I not familiar enough with other platforms to
feel confident enough to convert virtio-pci and test it :(

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


Re: [PATCH RFC 03/11] virtio: support more feature bits

2014-10-13 Thread Cornelia Huck
On Mon, 13 Oct 2014 16:23:58 +1030
Rusty Russell ru...@rustcorp.com.au wrote:

 Cornelia Huck cornelia.h...@de.ibm.com writes:
  With virtio-1, we support more than 32 feature bits. Let's make
  vdev-guest_features depend on the number of supported feature bits,
  allowing us to grow the feature bits automatically.
 
 It's a judgement call, but I would say that simply using uint64_t
 will be sufficient for quite a while.

I prefer this as an array for two reasons:

- It matches what ccw does anyway (we read/write features in chunks of
  32 bit), so this makes the backend handling for this transport very
  straightforward.
- While I don't see us running out of 64 feature bits for a while, we'd
  have to touch every device and transport again when we do. I'd prefer
  to do this once, as we need to change code anyway.

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


Re: [PATCH 2/2] virtio_balloon: free some memory from baloon on OOM

2014-10-13 Thread Denis V. Lunev

On 13/10/14 09:32, Rusty Russell wrote:

Denis V. Lunev d...@parallels.com writes:

From: Raushaniya Maksudova rmaksud...@parallels.com

Excessive virtio_balloon inflation can cause invocation of OOM-killer,
when Linux is under severe memory pressure. Various mechanisms are
responsible for correct virtio_balloon memory management. Nevertheless
it is often the case that these control tools does not have enough time
to react on fast changing memory load. As a result OS runs out of memory
and invokes OOM-killer. The balancing of memory by use of the virtio
balloon should not cause the termination of processes while there are
pages in the balloon. Now there is no way for virtio balloon driver to
free some memory at the last moment before some process will be get
killed by OOM-killer.

This makes some amount of sense.

But I suggest a few minor changes:


+static int oom_vballoon_pages = OOM_VBALLOON_DEFAULT_PAGES;
+module_param(oom_vballoon_pages, int, S_IRUSR | S_IWUSR);
+MODULE_PARM_DESC(oom_vballoon_pages, pages to free on OOM);

Since this is already prefixed with virtio_balloon. I suggest just
calling it oom_pages.

ok, will do


+static int virtballoon_oom_notify(struct notifier_block *self,
+ unsigned long dummy, void *parm)
+{
+   unsigned int num_freed_pages;
+   unsigned long *freed = (unsigned long *)parm;
+   struct virtio_balloon *vb = container_of((struct notifier_block *)self,
+struct virtio_balloon, nb);

Why cast self here?
this is a piece from a previous version of the patch, I'll fix this and 
resend.



+   num_freed_pages = leak_balloon(vb, oom_vballoon_pages);
+   update_balloon_size(vb);
+   *freed += num_freed_pages;
+
+   return NOTIFY_OK;
+}

Cheers,
Rusty.


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