CFP: The 24th Int. ACM Symp. on High-Performance Parallel and Distributed Computing (HPDC) 2015
CALL FOR PAPERS The 24th International ACM Symposium on High-Performance Parallel and Distributed Computing (HPDC-2015) Portland, Oregon, USA - June 15-19, 2015 http://www.hpdc.org/2015 The ACM International Symposium on High-Performance Parallel and Distributed Computing (HPDC) is the premier annual conference for presenting the latest research on the design, implementation, evaluation, and the use of parallel and distributed systems for high-end computing. The 24th HPDC will take place in the city of roses, Portland, Oregon on June 15-19, 2015. (Workshops on June 15-16, and the main conference on June 17-19.) IMPORTANT DATES Abstracts (required) due: January 12, 2015 Full Papers due:January 19, 2015 (no extensions) Author rebuttal period: March 4-7, 2015 Author notifications: March 16, 2015 Final Manuscripts: April 1, 2015 SCOPE AND TOPICS Submissions are welcomed on high-performance parallel and distributed computing topics including but not limited to: clouds, clusters, grids, big data, massively multicore, and global-scale computing systems. Submissions that focus on the architectures, systems, and networks of cloud infrastructures are particularly encouraged, as are experience reports of operational deployments that can provide insights for future research on HPDC applications and systems. All papers will be evaluated for their originality, technical depth and correctness, potential impact, relevance to the conference, and quality of presentation. Research papers must clearly demonstrate research contributions and novelty, while experience reports must clearly describe lessons learned and demonstrate impact. In the context of high-performance parallel and distributed computing, the topics of interest include, but are not limited to: - Systems, networks, and architectures - Massively multicore systems - Resource virtualization - Programming languages and environments - File and storage systems, I/O, and data management - Resource management and scheduling, including energy-aware techniques - Performance modeling and analysis - Fault tolerance, reliability, and availability - Data-intensive computing - Applications and services that depend upon high-end computing PAPER SUBMISSION GUIDELINES Authors are invited to submit technical papers of at most 12 pages in PDF format, including figures and references. Papers should be formatted in the ACM Proceedings Style and submitted via the conference web site. No changes to the margins, spacing, or font sizes as specified by the style file are allowed. Accepted papers will appear in the conference proceedings, and will be incorporated into the ACM Digital Library. A limited number of papers will be accepted as posters. Papers must be self-contained and provide the technical substance required for the program committee to evaluate their contributions. Submitted papers must be original work that has not appeared in and is not under consideration for another conference or a journal. See the ACM Prior Publication Policy for more details. Papers can be submitted at https://ssl.linklings.net/conferences/hpdc/. HPDC'15 GENERAL CO-CHAIRS Thilo Kielmann, VU University Amsterdam, The Netherlands HPDC'15 PROGRAM CO-CHAIRS Dean Hildebrand, IBM Research Almaden, USA Michela Taufer, University of Delaware, USA HPDC'15 WORKSHOP CHAIRS Abhishek Chandra, University of Minnesota, Twin Cities, USA Ioan Raicu, Illinois Institute of Technology and Argonne National Laboratory, USA HPDC'15 POSTERS CHAIR Ana-Maria Oprescu, VU University Amsterdam, The Netherlands HPDC'15 PUBLICITY CHAIR Ioan Raicu, Illinois Institute of Technology and Argonne National Laboratory, USA Torsten Hoefler, ETH Zurich, Switzerland Naoya Maruyama, RIKEN Advanced Institute for Computational Science, Japan HPDC'15 PUBLICATIONS CHAIR Antonino Tumeo, Pacific Northwest National Laboratory, USA HPDC'15 TRAVEL AWARD CHAIR Ming Zhao, Florida International University, USA HPDC'15 WEBMASTER CHAIR Kaveh Razavi, VU University Amsterdam, The Netherlands HPDC'15 PROGRAM COMMITTEE David Abramson, The University of Queensland, Australia Dong Ahn, Lawrence Livermore National Laboratory, USA Gabriel Antoniu, INRIA, France Henri Bal, VU University Amsterdam, The Netherlands Pavan Balaji, Argonne National Laboratory, USA Michela Becchi, University of Missouri, USA John Bent, EMC, USA Greg Bronevetsky, Lawrence Livermore National Laboratory, USA Ali Butt, Virginia Tech, USA Franck Cappello, Argonne National Lab, USA Abhishek Chandra, University of Minnesota, USA Andrew A. Chien, University of Chicago, USA Paolo Costa, Microsoft Research Cambridge, UK Kei Davis, Los Alamos National Laboratory, USA Peter Dinda
Re: [PATCH] virtio: pci: Use SIMPLE_DEV_PM_OPS macro
On Thu, Sep 18, 2014 at 02:53:10PM +0930, Rusty Russell wrote: > Jingoo Han writes: > > On Tuesday, September 09, 2014 9:14 AM, Rusty Russell wrote: > >> Jingoo Han writes: > >> > Use SIMPLE_DEV_PM_OPS macro in order to make the code simpler. > >> > > >> > Signed-off-by: Jingoo Han > >> > >> This patch is obviously wrong. It won't compile without > >> CONFIG_PM_SLEEP. > > > > No, there is no compile issue. > > When, CONFIG_PM_SLEEP=n, there is no build error. > > My mistake. Thanks, I've applied it. It probably won't go in until the > next merge window, however, since I'm travelling for this one. > > Cheers, > Rusty. I have some bugfixes that are I think worth merging, so maybe I'll do a pull request. If so, this cleanup could go in, on top. Rusty, what do you say? -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 01/16] virtio_pci: fix virtio spec compliance on restore
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 Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_pci.c | 36 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index 3d1463c..0f2db51 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,41 @@ 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) - ret = drv->restore(&vp_dev->vdev); + if (!drv->restore) + return 0; + + 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 00/16] virtio: fix spec compliance issues
Rusty, I have a mind to send this patches for the next merge window. Any input on this? 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 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. Michael S. Tsirkin (16): virtio_pci: fix virtio spec compliance on restore virtio: unify config_changed handling virtio: refactor to use drv_to_virtio 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: add API to enable VQs early virtio_net: minor cleanup 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 include/linux/virtio.h | 14 ++ include/linux/virtio_config.h | 17 +++ drivers/block/virtio_blk.c | 27 ++- drivers/char/virtio_console.c | 2 + drivers/misc/mic/card/mic_virtio.c | 6 +-- drivers/net/virtio_net.c | 38 +++ drivers/s390/kvm/kvm_virtio.c | 9 +--- drivers/s390/kvm/virtio_ccw.c | 6 +-- drivers/virtio/virtio.c| 99 ++ drivers/virtio/virtio_mmio.c | 7 +-- drivers/virtio/virtio_pci.c| 33 ++--- net/9p/trans_virtio.c | 2 + 12 files changed, 153 insertions(+), 107 deletions(-) -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 03/16] virtio: refactor to use drv_to_virtio
Use drv_to_virtio instead of open-coding it. Fix whitespace damage introduced by virtio: unify config_changed handling Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index 470b74f..3980687 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -241,8 +241,7 @@ EXPORT_SYMBOL_GPL(unregister_virtio_device); void virtio_config_changed(struct virtio_device *dev) { -struct virtio_driver *drv = container_of(dev->dev.driver, -struct virtio_driver, driver); + struct virtio_driver *drv = drv_to_virtio(dev->dev.driver); if (drv && drv->config_changed) drv->config_changed(dev); -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 04/16] virtio-pci: move freeze/restore to virtio core
This is in preparation to extending config changed event handling in core. Wrapping these in an API also seems to make for a cleaner code. Signed-off-by: Michael S. Tsirkin --- include/linux/virtio.h | 6 + drivers/virtio/virtio.c | 53 +++ drivers/virtio/virtio_pci.c | 55 ++--- 3 files changed, 61 insertions(+), 53 deletions(-) diff --git a/include/linux/virtio.h b/include/linux/virtio.h index 3c19bd3..8df7ba8 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -78,6 +78,7 @@ 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) * @dev: underlying device. * @id: the device type identification (used to match it with a driver). * @config: the configuration ops for this device. @@ -88,6 +89,7 @@ bool virtqueue_is_broken(struct virtqueue *vq); */ struct virtio_device { int index; + bool failed; struct device dev; struct virtio_device_id id; const struct virtio_config_ops *config; @@ -109,6 +111,10 @@ void unregister_virtio_device(struct virtio_device *dev); void virtio_break_device(struct virtio_device *dev); void virtio_config_changed(struct virtio_device *dev); +#ifdef CONFIG_PM_SLEEP +int virtio_device_freeze(struct virtio_device *dev); +int virtio_device_restore(struct virtio_device *dev); +#endif /** * virtio_driver - operations for a virtio I/O driver diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index 3980687..657f817 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -248,6 +248,59 @@ void virtio_config_changed(struct virtio_device *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); + + dev->failed = dev->config->get_status(dev) & VIRTIO_CONFIG_S_FAILED; + + if (drv && drv->freeze) + return drv->freeze(dev); + + return 0; +} +EXPORT_SYMBOL_GPL(virtio_device_freeze); + +int virtio_device_restore(struct virtio_device *dev) +{ + struct virtio_driver *drv = drv_to_virtio(dev->dev.driver); + + /* We always start by resetting the device, in case a previous +* driver messed it up. */ + dev->config->reset(dev); + + /* Acknowledge that we've seen the device. */ + add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE); + + /* Maybe driver failed before freeze. +* Restore the failed status, for debugging. */ + if (dev->failed) + add_status(dev, VIRTIO_CONFIG_S_FAILED); + + if (!drv) + return 0; + + /* We have a driver! */ + add_status(dev, VIRTIO_CONFIG_S_DRIVER); + + dev->config->finalize_features(dev); + + if (drv->restore) { + int ret = drv->restore(dev); + if (ret) { + add_status(dev, VIRTIO_CONFIG_S_FAILED); + return ret; + } + /* Finally, tell the device we're all set */ + add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK); + } + + return 0; +} +EXPORT_SYMBOL_GPL(virtio_device_restore); +#endif + static int virtio_init(void) { if (bus_register(&virtio_bus) != 0) diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index 58cbf6e..d34ebfa 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -57,9 +57,6 @@ struct virtio_pci_device /* Vectors allocated, excluding per-vq vectors if any */ unsigned msix_used_vectors; - /* Status saved during hibernate/restore */ - u8 saved_status; - /* Whether we have vector per vq */ bool per_vq_vectors; }; @@ -764,16 +761,9 @@ static int virtio_pci_freeze(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; int ret; - drv = container_of(vp_dev->vdev.dev.driver, - struct virtio_driver, driver); - - ret = 0; - vp_dev->saved_status = vp_get_status(&vp_dev->vdev); - if (drv && drv->freeze) - ret = drv->freeze(&vp_dev->vdev); + ret = virtio_device_freeze(&vp_dev->vdev); if (!ret) pci_disable_device(pci_dev); @@ -784,55 +774,14 @@ 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, - struct virtio_driver, driver); -
[PATCH 05/16] virtio: defer config changed notifications
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 --- include/linux/virtio.h | 6 ++ drivers/virtio/virtio.c | 55 + 2 files changed, 52 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 657f817..578e02d 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -117,6 +117,40 @@ 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) +{ + spin_lock_irq(&dev->config_lock); + __virtio_config_changed(dev); + spin_unlock_irq(&dev->config_lock); +} +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 +187,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 +199,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 +249,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 +281,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) @@ -296,6 +331,8 @@ int virtio_device_restore(struct virtio_device *dev) add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK); } + virtio_config_enable(dev); + return 0; } EXPORT_SYM
[PATCH 06/16] virtio_blk: drop config_enable
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 --- drivers/block/virtio_blk.c | 19 ++- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 0a58140..c8cf6a1 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) @@ -772,9 +766,7 @@ static void virtblk_remove(struct virtio_device *vdev) int refc; /* Prevent config work handler from accessing the device. */ - mutex_lock(&vblk->config_lock); - vblk->config_enable = false; - mutex_unlock(&vblk->config_lock); + 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); @@ -806,10 +796,6 @@ static int virtblk_freeze(struct virtio_device *vdev) 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); - 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 02/16] virtio: unify config_changed handling
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 --- 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| 10 ++ drivers/virtio/virtio_mmio.c | 7 ++- drivers/virtio/virtio_pci.c| 6 +- 7 files changed, 18 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..470b74f 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -239,6 +239,16 @@ 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 = container_of(dev->dev.driver, +struct virtio_driver, 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
[PATCH 07/16] virtio-blk: drop config_mutex
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 --- 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 c8cf6a1..13c384b 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 08/16] virtio_net: drop config_enable
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 --- drivers/net/virtio_net.c | 23 ++- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 59caa06..fa17afa 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. */ @@ -1876,16 +1869,12 @@ 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); + 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); } @@ -1899,9 +1888,7 @@ 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); + 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 09/16] virtio-net: drop config_mutex
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 --- 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 fa17afa..d80fef4 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 10/16] virtio: add API to enable VQs early
virtio spec 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. Add API for drivers to call before using VQs. Sets DRIVER_OK internally. Signed-off-by: Michael S. Tsirkin --- include/linux/virtio_config.h | 17 + 1 file changed, 17 insertions(+) diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index e8f8f71..6127fc8 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -109,6 +109,23 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, return vq; } +/** + * virtio_early_enable_vqs - enable vq use in probe function + * @vdev: the device + * + * Driver must call this to use vqs in the probe function. + * + * Note: vqs are enabled automatically after probe returns. + */ +static inline +void virtio_early_enable_vqs(struct virtio_device *dev) +{ + unsigned status = dev->config->get_status(dev); + + BUG_ON(status & VIRTIO_CONFIG_S_DRIVER_OK); + dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK); +} + static inline const char *virtio_bus_name(struct virtio_device *vdev) { -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 11/16] virtio_net: minor cleanup
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 --- 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 d80fef4..4c8c314 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 12/16] virtio_net: enable VQs early
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_early_enable_vqs before using VQs. Signed-off-by: Michael S. Tsirkin --- 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 4c8c314..7afc990 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_early_enable_vqs(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 14/16] virtio_console: enable VQs early
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_early_enable_vqs before using VQs. Signed-off-by: Michael S. Tsirkin --- 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..8605009 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_early_enable_vqs(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 13/16] virtio_blk: enable VQs early
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_early_enable_vqs before using VQs. Signed-off-by: Michael S. Tsirkin --- 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 13c384b..e59ae859 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_early_enable_vqs(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 16/16] virtio_net: fix use after free on allocation failure
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 --- 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 7afc990..85e6098 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