* [PATCH -next] VSOCK: Use kvfree()
From: Wei Yongjun @ 2016-08-02 13:50 UTC (permalink / raw)
To: Stefan Hajnoczi, Michael S. Tsirkin; +Cc: Wei Yongjun, kvm, virtualization
Use kvfree() instead of open-coding it.
Signed-off-by: Wei Yongjun <weiyj.lk@gmail.com>
---
drivers/vhost/vsock.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 028ca16..0ddf3a2 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -434,10 +434,7 @@ err:
static void vhost_vsock_free(struct vhost_vsock *vsock)
{
- if (is_vmalloc_addr(vsock))
- vfree(vsock);
- else
- kfree(vsock);
+ kvfree(vsock);
}
static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
^ permalink raw reply related
* [PATCH -next] vhost: fix missing unlock on error in vhost_net_set_features()
From: Wei Yongjun @ 2016-08-02 13:50 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang; +Cc: Wei Yongjun, kvm, virtualization
Add the missing unlock before return from function
vhost_net_set_features() in the error handling case.
Fixes: eefe82d9b81f ('vhost: new device IOTLB API')
Signed-off-by: Wei Yongjun <weiyj.lk@gmail.com>
---
drivers/vhost/net.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index c6bdd90..5dc128a 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1104,13 +1104,12 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features)
}
mutex_lock(&n->dev.mutex);
if ((features & (1 << VHOST_F_LOG_ALL)) &&
- !vhost_log_access_ok(&n->dev)) {
- mutex_unlock(&n->dev.mutex);
- return -EFAULT;
- }
+ !vhost_log_access_ok(&n->dev))
+ goto out_unlock;
+
if ((features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) {
if (vhost_init_device_iotlb(&n->dev, true))
- return -EFAULT;
+ goto out_unlock;
}
for (i = 0; i < VHOST_NET_VQ_MAX; ++i) {
@@ -1122,6 +1121,10 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features)
}
mutex_unlock(&n->dev.mutex);
return 0;
+
+out_unlock:
+ mutex_unlock(&n->dev.mutex);
+ return -EFAULT;
}
static long vhost_net_set_owner(struct vhost_net *n)
^ permalink raw reply related
* [PATCH 1024/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 12:09 UTC (permalink / raw)
To: mst, plagnioj, tomi.valkeinen, m.chehab, gregkh, m.szyprowski,
kyungmin.park, k.kozlowski
Cc: baolex.ni, oneukum, chuansheng.liu, linux-kernel, virtualization
I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
drivers/virtio/virtio_pci_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index d9a9058..f7d2262 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -22,7 +22,7 @@
static bool force_legacy = false;
#if IS_ENABLED(CONFIG_VIRTIO_PCI_LEGACY)
-module_param(force_legacy, bool, 0444);
+module_param(force_legacy, bool, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(force_legacy,
"Force legacy mode for transitional virtio 1 devices");
#endif
--
2.9.2
^ permalink raw reply related
* [PATCH 0992/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 12:06 UTC (permalink / raw)
To: mst, gregkh, m.chehab, m.szyprowski, kyungmin.park, k.kozlowski
Cc: mathias.nyman, kvm, netdev, oneukum, linux-kernel, virtualization,
baolex.ni, stern, chuansheng.liu
I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
drivers/vhost/vhost.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 669fef1..bf338d1 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -31,7 +31,7 @@
#include "vhost.h"
static ushort max_mem_regions = 64;
-module_param(max_mem_regions, ushort, 0444);
+module_param(max_mem_regions, ushort, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(max_mem_regions,
"Maximum number of memory regions in memory map. (default: 64)");
--
2.9.2
^ permalink raw reply related
* [PATCH 0991/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 12:06 UTC (permalink / raw)
To: mst, gregkh, m.chehab, m.szyprowski, kyungmin.park, k.kozlowski
Cc: mathias.nyman, kvm, netdev, oneukum, linux-kernel, virtualization,
baolex.ni, stern, chuansheng.liu
I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
drivers/vhost/net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index f744eeb..e6d5620 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -31,7 +31,7 @@
#include "vhost.h"
static int experimental_zcopytx = 1;
-module_param(experimental_zcopytx, int, 0444);
+module_param(experimental_zcopytx, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
" 1 -Enable; 0 - Disable");
--
2.9.2
^ permalink raw reply related
* [PATCH 0904/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 11:56 UTC (permalink / raw)
To: borntraeger, cornelia.huck, schwidefsky, heiko.carstens, dwmw2,
m.chehab, pawel, m.szyprowski, kyungmin.park, k.kozlowski
Cc: linux-s390, kvm, linux-kernel, virtualization, baolex.ni,
chuansheng.liu
I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
drivers/s390/virtio/virtio_ccw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 8688ad4..56d3671 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -1074,7 +1074,7 @@ static unsigned long devs_no_auto[__MAX_SSID + 1][__DEV_WORDS];
static char *no_auto = "";
-module_param(no_auto, charp, 0444);
+module_param(no_auto, charp, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(no_auto, "list of ccw bus id ranges not to be auto-onlined");
static int virtio_ccw_check_autoonline(struct ccw_device *cdev)
--
2.9.2
^ permalink raw reply related
* [PATCH 0753/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 11:42 UTC (permalink / raw)
To: mst, f.fainelli, bkenward, linux-driver, computersforpeace,
m.chehab, pawel, m.szyprowski, kyungmin.park, k.kozlowski
Cc: pabeni, daniel, netdev, linux-kernel, virtualization, baolex.ni,
hannes, chuansheng.liu
I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
drivers/net/virtio_net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index e0638e5..39815e4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -30,11 +30,11 @@
#include <net/busy_poll.h>
static int napi_weight = NAPI_POLL_WEIGHT;
-module_param(napi_weight, int, 0444);
+module_param(napi_weight, int, S_IRUSR | S_IRGRP | S_IROTH);
static bool csum = true, gso = true;
-module_param(csum, bool, 0444);
-module_param(gso, bool, 0444);
+module_param(csum, bool, S_IRUSR | S_IRGRP | S_IROTH);
+module_param(gso, bool, S_IRUSR | S_IRGRP | S_IROTH);
/* FIXME: MTU in config. */
#define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
--
2.9.2
^ permalink raw reply related
* [PATCH 0214/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 10:50 UTC (permalink / raw)
To: airlied, kraxel, kyungmin.park, kgene, k.kozlowski, dougthompson,
bp
Cc: linux-kernel, dri-devel, virtualization, baolex.ni,
alexander.deucher, chuansheng.liu, ville.syrjala
I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
drivers/gpu/drm/virtio/virtgpu_kms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 4150873..1b7bd86 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -31,7 +31,7 @@
static int virtio_gpu_fbdev = 1;
MODULE_PARM_DESC(fbdev, "Disable/Enable framebuffer device & console");
-module_param_named(fbdev, virtio_gpu_fbdev, int, 0400);
+module_param_named(fbdev, virtio_gpu_fbdev, int, S_IRUSR);
static void virtio_gpu_config_changed_work_func(struct work_struct *work)
{
--
2.9.2
^ permalink raw reply related
* [PATCH 0213/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 10:50 UTC (permalink / raw)
To: airlied, kraxel, kyungmin.park, kgene, k.kozlowski, dougthompson,
bp
Cc: linux-kernel, dri-devel, virtualization, baolex.ni,
alexander.deucher, chuansheng.liu, ville.syrjala
I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
drivers/gpu/drm/virtio/virtgpu_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 3cc7afa..d40bc45 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -38,7 +38,7 @@ static struct drm_driver driver;
static int virtio_gpu_modeset = -1;
MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
-module_param_named(modeset, virtio_gpu_modeset, int, 0400);
+module_param_named(modeset, virtio_gpu_modeset, int, S_IRUSR);
static int virtio_gpu_probe(struct virtio_device *vdev)
{
--
2.9.2
^ permalink raw reply related
* [PATCH 0095/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 10:40 UTC (permalink / raw)
To: mst, pjk1939, pavel, gregkh, hpa, x86
Cc: martin.petersen, linux-kernel, virtualization, baolex.ni,
chuansheng.liu, kent.overstreet
I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
drivers/block/virtio_blk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 42758b5..786a393 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -561,7 +561,7 @@ static struct blk_mq_ops virtio_mq_ops = {
};
static unsigned int virtblk_queue_depth;
-module_param_named(queue_depth, virtblk_queue_depth, uint, 0444);
+module_param_named(queue_depth, virtblk_queue_depth, uint, S_IRUSR | S_IRGRP | S_IROTH);
static int virtblk_probe(struct virtio_device *vdev)
{
--
2.9.2
^ permalink raw reply related
* [PATCH] drm/virtio: fix building without CONFIG_FBDEV
From: Arnd Bergmann @ 2016-08-02 10:09 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann
Cc: Archit Taneja, Arnd Bergmann, Daniel Vetter, Emil Velikov,
linux-kernel, dri-devel, virtualization, Tobias Jakobi
Removing the build-time dependency on DRM_KMS_FB_HELPER means
we can now build with CONFIG_FB disabled or as a loadable module,
leading to a link error:
ERROR: "remove_conflicting_framebuffers" [drivers/gpu/drm/virtio/virtio-gpu.ko] undefined!
There is no need to call remove_conflicting_framebuffers() if
CONFIG_FB is disabled, or if the virtio-gpu driver is built-in
and CONFIG_FB=m, as there won't be any prior consoles that
are registered at that point.
This adds a compile-time check in the driver to ensure we only
attempt to call that function if either CONFIG_FB=y or
both subsystems are configured as loadable modules.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 0b6320dfdfea ("drm/virtio: make fbdev support really optional")
---
drivers/gpu/drm/virtio/virtgpu_drm_bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
index 7f0e93f87a55..2087569ae8d7 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
@@ -65,7 +65,7 @@ int drm_virtio_init(struct drm_driver *driver, struct virtio_device *vdev)
DRM_INFO("pci: %s detected\n",
vga ? "virtio-vga" : "virtio-gpu-pci");
dev->pdev = pdev;
- if (vga)
+ if (IS_REACHABLE(CONFIG_FB) && vga)
virtio_pci_kick_out_firmware_fb(pdev);
}
--
2.9.0
^ permalink raw reply related
* [PATCH] vhost: drop vringh dependency
From: Michael S. Tsirkin @ 2016-08-02 5:09 UTC (permalink / raw)
To: linux-kernel
Cc: Krzysztof Kozlowski, Matias Bjørling, kvm,
Alexander Shishkin, Greg Kroah-Hartman, virtualization,
Jens Axboe, netdev, Srinivas Kandagatla, Jay Sternberg,
Maxime Ripard, Dan Williams
vringh isn't used by vhost net or scsi - it's used
by CAIF only at the moment. Drop the dependency.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Including vhost directory twice isn't pretty - we just want it
scanned if either of the symbols is defined.
Something along the lines of
obj-$(subst %m%,m,$(subst %y%,y,$(CONFIG_VHOST)(CONFIG_VHOST_RING))) += vhost/
would also work, but seems uglier.
drivers/Makefile | 1 +
drivers/vhost/Kconfig | 2 --
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/Makefile b/drivers/Makefile
index 0b6f3d6..5d03dba 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -139,6 +139,7 @@ obj-$(CONFIG_OF) += of/
obj-$(CONFIG_SSB) += ssb/
obj-$(CONFIG_BCMA) += bcma/
obj-$(CONFIG_VHOST_RING) += vhost/
+obj-$(CONFIG_VHOST) += vhost/
obj-$(CONFIG_VLYNQ) += vlynq/
obj-$(CONFIG_STAGING) += staging/
obj-y += platform/
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index 533eaf0..387d30d 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -2,7 +2,6 @@ config VHOST_NET
tristate "Host kernel accelerator for virtio net"
depends on NET && EVENTFD && (TUN || !TUN) && (MACVTAP || !MACVTAP)
select VHOST
- select VHOST_RING
---help---
This kernel module can be loaded in host kernel to accelerate
guest networking with virtio_net. Not to be confused with virtio_net
@@ -15,7 +14,6 @@ config VHOST_SCSI
tristate "VHOST_SCSI TCM fabric driver"
depends on TARGET_CORE && EVENTFD && m
select VHOST
- select VHOST_RING
default n
---help---
Say M here to enable the vhost_scsi TCM fabric module
--
MST
^ permalink raw reply related
* [vhost:vhost 6/17] ERROR: "vhost_work_flush" [drivers/vhost/vhost_scsi.ko] undefined!
From: kbuild test robot @ 2016-08-02 4:10 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, kbuild-all, kvm, virtualization
[-- Attachment #1: Type: text/plain, Size: 2130 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
head: b5311f2ee9cf07e73303ae792bdc363a91b9df0b
commit: 6190efb08c16dcd68c64b096a28f47ab33f017d7 [6/17] vhost: drop vringh dependency
config: x86_64-randconfig-n0-08020724 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
git checkout 6190efb08c16dcd68c64b096a28f47ab33f017d7
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> ERROR: "vhost_work_flush" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_dev_cleanup" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_log_access_ok" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_enable_notify" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_poll_flush" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_disable_notify" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_signal" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_dev_ioctl" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_get_vq_desc" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_work_queue" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_add_used_and_signal" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_work_init" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_vq_init_access" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_dev_init" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_dev_stop" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_vq_access_ok" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_vring_ioctl" [drivers/vhost/vhost_scsi.ko] undefined!
>> ERROR: "vhost_add_used" [drivers/vhost/vhost_scsi.ko] undefined!
make[2]: *** [__modpost] Error 1
make[2]: Target '_modpost' not remade because of errors.
make[1]: *** [modules] Error 2
make[1]: Target '_all' not remade because of errors.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 29555 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* RE: [virtio-dev] Re: [PATCH v2 repost 4/7] virtio-balloon: speed up inflate/deflate process
From: Li, Liang Z @ 2016-08-02 0:28 UTC (permalink / raw)
To: Hansen, Dave, Michael S. Tsirkin
Cc: virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org, Amit Shah,
qemu-devel@nongnu.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, Vlastimil Babka, Paolo Bonzini, Andrew Morton,
virtualization@lists.linux-foundation.org, Mel Gorman,
dgilbert@redhat.com
In-Reply-To: <579BB30B.2040704@intel.com>
> > It's only small because it makes you rescan the free list.
> > So maybe you should do something else.
> > I looked at it a bit. Instead of scanning the free list, how about
> > scanning actual page structures? If page is unused, pass it to host.
> > Solves the problem of rescanning multiple times, does it not?
>
> FWIW, I think the new data structure needs some work.
>
> Before, we had a potentially very long list of 4k areas. Now, we've just got a
> very large bitmap. The bitmap might not even be very dense if we are
> ballooning relatively few things.
>
> Can I suggest an alternate scheme? I think you actually need a hybrid
> scheme that has bitmaps but also allows more flexibility in the pfn ranges.
> The payload could be a number of records each containing 3 things:
>
> pfn, page order, length of bitmap (maybe in powers of 2)
>
> Each record is followed by the bitmap. Or, if the bitmap length is 0,
> immediately followed by another record. A bitmap length of 0 implies a
> bitmap with the least significant bit set. Page order specifies how many
> pages each bit represents.
>
> This scheme could easily encode the new data structure you are proposing
> by just setting pfn=0, order=0, and a very long bitmap length. But, it could
> handle sparse bitmaps much better *and* represent large pages much more
> efficiently.
>
> There's plenty of space to fit a whole record in 64 bits.
I like your idea and it's more flexible, and it's very useful if we want to optimize the
page allocating stage further. I believe the memory fragmentation will not be very
serious, so the performance won't be too bad in the worst case.
Thanks!
Liang
^ permalink raw reply
* [vhost:vhost 5/15] warning: (VOP && ..) selects VHOST_RING which has unmet direct dependencies (NETDEVICES && ..)
From: kbuild test robot @ 2016-08-01 23:19 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, kbuild-all, kvm, virtualization
[-- Attachment #1: Type: text/plain, Size: 808 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
head: ccbb911915b99ba9ff81a1cfa4f07c98f98fff83
commit: 4d167a33a6669c309f8fcdc6cd7143de3b5445b3 [5/15] vhost: drop vringh dependency
config: x86_64-randconfig-x005-201631 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
git checkout 4d167a33a6669c309f8fcdc6cd7143de3b5445b3
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
warning: (VOP && CAIF_VIRTIO) selects VHOST_RING which has unmet direct dependencies (NETDEVICES && CAIF_VIRTIO || VIRTUALIZATION)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 22489 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [vhost:vhost 11/15] warning: (VOP && ..) selects VHOST_RING which has unmet direct dependencies (NETDEVICES && ..)
From: Michael S. Tsirkin @ 2016-08-01 21:18 UTC (permalink / raw)
To: kbuild test robot
Cc: kvm, netdev, virtualization, kbuild-all, Stefan Hajnoczi,
Asias He
In-Reply-To: <201608020449.ake06oK8%fengguang.wu@intel.com>
On Tue, Aug 02, 2016 at 04:56:54AM +0800, kbuild test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
> head: c4ea43c2c1779f5dde3ff5645b830c90f75ccc15
> commit: efd7eb77f631e1ed3533db7725df157a379c78ef [11/15] VSOCK: Add Makefile and Kconfig
> config: x86_64-randconfig-x005-201631 (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> git checkout efd7eb77f631e1ed3533db7725df157a379c78ef
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All warnings (new ones prefixed by >>):
>
> warning: (VOP && CAIF_VIRTIO && VHOST_NET && VHOST_SCSI && VHOST_VSOCK && VHOST_NET && VHOST_SCSI && VHOST_VSOCK) selects VHOST_RING which has unmet direct dependencies (NETDEVICES && CAIF_VIRTIO || VIRTUALIZATION)
I don't see what is going on here.
VHOST_RING is in the same config file as VHOST_NET so it
has the same dependencies. How can one set VHOST_NET without
enabling dependencies for VHOST_RING?
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* [vhost:vhost 11/15] warning: (VOP && ..) selects VHOST_RING which has unmet direct dependencies (NETDEVICES && ..)
From: kbuild test robot @ 2016-08-01 20:56 UTC (permalink / raw)
To: Asias He
Cc: kvm, Michael S. Tsirkin, netdev, virtualization, kbuild-all,
Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 895 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
head: c4ea43c2c1779f5dde3ff5645b830c90f75ccc15
commit: efd7eb77f631e1ed3533db7725df157a379c78ef [11/15] VSOCK: Add Makefile and Kconfig
config: x86_64-randconfig-x005-201631 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
git checkout efd7eb77f631e1ed3533db7725df157a379c78ef
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
warning: (VOP && CAIF_VIRTIO && VHOST_NET && VHOST_SCSI && VHOST_VSOCK && VHOST_NET && VHOST_SCSI && VHOST_VSOCK) selects VHOST_RING which has unmet direct dependencies (NETDEVICES && CAIF_VIRTIO || VIRTUALIZATION)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 22489 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [vhost:vhost 14/14] drivers/vhost/vhost.c:915:30: warning: passing argument 2 of 'access_ok' makes pointer from integer without a cast
From: kbuild test robot @ 2016-08-01 20:03 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, Michael S. Tsirkin, kbuild-all, kvm, virtualization
[-- Attachment #1: Type: text/plain, Size: 3000 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
head: 51ce50c54895044f949129e595ed9a37e4d6c13a
commit: 51ce50c54895044f949129e595ed9a37e4d6c13a [14/14] vhost: new device IOTLB API
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 51ce50c54895044f949129e595ed9a37e4d6c13a
# save the attached .config to linux build tree
make.cross ARCH=sparc64
All warnings (new ones prefixed by >>):
drivers/vhost/vhost.c: In function 'umem_access_ok':
>> drivers/vhost/vhost.c:915:30: warning: passing argument 2 of 'access_ok' makes pointer from integer without a cast [-Wint-conversion]
!access_ok(VERIFY_READ, uaddr, size))
^
In file included from arch/sparc/include/asm/uaccess.h:4:0,
from include/linux/poll.h:11,
from drivers/vhost/vhost.c:21:
arch/sparc/include/asm/uaccess_64.h:79:19: note: expected 'const void *' but argument is of type 'u64 {aka long long unsigned int}'
static inline int access_ok(int type, const void __user * addr, unsigned long size)
^
drivers/vhost/vhost.c:918:31: warning: passing argument 2 of 'access_ok' makes pointer from integer without a cast [-Wint-conversion]
!access_ok(VERIFY_WRITE, uaddr, size))
^
In file included from arch/sparc/include/asm/uaccess.h:4:0,
from include/linux/poll.h:11,
from drivers/vhost/vhost.c:21:
arch/sparc/include/asm/uaccess_64.h:79:19: note: expected 'const void *' but argument is of type 'u64 {aka long long unsigned int}'
static inline int access_ok(int type, const void __user * addr, unsigned long size)
^
vim +/access_ok +915 drivers/vhost/vhost.c
899 struct vhost_iotlb_msg *vq_msg = &node->msg.iotlb;
900 if (msg->iova <= vq_msg->iova &&
901 msg->iova + msg->size - 1 > vq_msg->iova &&
902 vq_msg->type == VHOST_IOTLB_MISS) {
903 vhost_poll_queue(&node->vq->poll);
904 list_del(&node->node);
905 kfree(node);
906 }
907 }
908
909 spin_unlock(&d->iotlb_lock);
910 }
911
912 static int umem_access_ok(u64 uaddr, u64 size, int access)
913 {
914 if ((access & VHOST_ACCESS_RO) &&
> 915 !access_ok(VERIFY_READ, uaddr, size))
916 return -EFAULT;
917 if ((access & VHOST_ACCESS_WO) &&
918 !access_ok(VERIFY_WRITE, uaddr, size))
919 return -EFAULT;
920 return 0;
921 }
922
923 int vhost_process_iotlb_msg(struct vhost_dev *dev,
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 46360 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device
From: Namhyung Kim @ 2016-08-01 15:52 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Tony Luck, Kees Cook, kvm, Radim Kr??m????, LKML,
Michael S. Tsirkin, qemu-devel, Steven Rostedt, virtualization,
Minchan Kim, Anton Vorontsov, Anthony Liguori, Colin Cross,
Paolo Bonzini, Ingo Molnar
In-Reply-To: <20160801092439.GF6455@redhat.com>
On Mon, Aug 01, 2016 at 10:24:39AM +0100, Daniel P. Berrange wrote:
> On Sat, Jul 30, 2016 at 05:57:02PM +0900, Namhyung Kim wrote:
> > On Thu, Jul 28, 2016 at 02:22:39PM +0100, Daniel P. Berrange wrote:
> > > > +static void virtio_pstore_from_filename(VirtIOPstore *s, char *name,
> > > > + char *buf, size_t sz,
> > > > + struct virtio_pstore_fileinfo *info)
> > > > +{
> > > > + snprintf(buf, sz, "%s/%s", s->directory, name);
> > > > +
> > > > + if (g_str_has_prefix(name, "dmesg-")) {
> > > > + info->type = VIRTIO_PSTORE_TYPE_DMESG;
> > > > + name += strlen("dmesg-");
> > > > + } else if (g_str_has_prefix(name, "console-")) {
> > > > + info->type = VIRTIO_PSTORE_TYPE_CONSOLE;
> > > > + name += strlen("console-");
> > > > + } else if (g_str_has_prefix(name, "unknown-")) {
> > > > + info->type = VIRTIO_PSTORE_TYPE_UNKNOWN;
> > > > + name += strlen("unknown-");
> > > > + }
>
> [snip]
>
> > > > + struct virtio_pstore_fileinfo info;
> > > > + size_t offset = sizeof(*res) + sizeof(info);
> > > > +
> > > > + if (s->dirp == NULL) {
> > > > + return -1;
> > > > + }
> > > > +
> > > > + dent = readdir(s->dirp);
> > > > + while (dent) {
> > > > + if (dent->d_name[0] != '.') {
> > > > + break;
> > > > + }
> > > > + dent = readdir(s->dirp);
> > > > + }
> > > > +
> > > > + if (dent == NULL) {
> > > > + return 0;
> > > > + }
> > >
> > > So this seems to just be picking the first filename reported by
> > > readdir that isn't starting with '.'. Surely this can't the right
> > > logic when your corresponding do_write method can pick several
> > > different filenames, its potluck which do_read will give back.
> >
> > Do you mean that it'd be better to check a list of known filenames and
> > fail if not?
>
> No, I mean that you have several different VIRTIO_PSTORE_TYPE_nnn and
> use a different file for each constant. When reading this directory
> though you're not looking for the file corresponding to any given
> VIRTIO_PSTORE_TYPE_nnn - you're simply reading whichever file is found
> first. So you might have just read a TYPE_CONSOLE or have read a
> TYPE_DMESG - it surely doesn't make sense to randonly read either
> TYPE_CONSOLE or TYPE_DMESG based on whatever order readdir() lists
> the files.
In fact, each VIRTIO_PSTORE_TYPE_xxx can have more than one files and
I don't care about the ordering between them. They will be fed to the
pstore filesystem on the guest.
But I also think it'd be better to scan files of known types only
rather than read out whatever readdir() gives.
Thanks,
Namhyung
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device
From: Namhyung Kim @ 2016-08-01 15:34 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Tony Luck, Kees Cook, kvm, Radim Kr??m????, Anton Vorontsov, LKML,
Steven Rostedt, qemu-devel, Minchan Kim, Michael S. Tsirkin,
Anthony Liguori, Colin Cross, Paolo Bonzini, virtualization,
Ingo Molnar
In-Reply-To: <20160801092130.GE6455@redhat.com>
Hi Daniel,
On Mon, Aug 01, 2016 at 10:21:30AM +0100, Daniel P. Berrange wrote:
> On Sat, Jul 30, 2016 at 05:38:27PM +0900, Namhyung Kim wrote:
> > Hello,
> >
> > On Thu, Jul 28, 2016 at 02:08:41PM +0100, Daniel P. Berrange wrote:
> > > On Thu, Jul 28, 2016 at 01:56:07PM +0100, Stefan Hajnoczi wrote:
> > > > On Thu, Jul 28, 2016 at 02:39:53PM +0900, Namhyung Kim wrote:
> > > > > On Thu, Jul 28, 2016 at 03:02:54AM +0300, Michael S. Tsirkin wrote:
> > > > > > On Thu, Jul 28, 2016 at 12:08:30AM +0900, Namhyung Kim wrote:
> > > > > > > +static ssize_t virtio_pstore_do_write(VirtIOPstore *s, struct iovec *out_sg,
> > > > > > > + unsigned int out_num,
> > > > > > > + struct virtio_pstore_req *req)
> > > > > > > +{
> > > > > > > + char path[PATH_MAX];
> > > > > > > + int fd;
> > > > > > > + ssize_t len;
> > > > > > > + unsigned short type;
> > > > > > > + int flags = O_WRONLY | O_CREAT;
> > > > > > > +
> > > > > > > + /* we already consume the req */
> > > > > > > + iov_discard_front(&out_sg, &out_num, sizeof(*req));
> > > > > > > +
> > > > > > > + virtio_pstore_to_filename(s, path, sizeof(path), req);
> > > > > > > +
> > > > > > > + type = le16_to_cpu(req->type);
> > > > > > > +
> > > > > > > + if (type == VIRTIO_PSTORE_TYPE_DMESG) {
> > > > > > > + flags |= O_TRUNC;
> > > > > > > + } else if (type == VIRTIO_PSTORE_TYPE_CONSOLE) {
> > > > > > > + flags |= O_APPEND;
> > > > > > > + }
> > > > > > > +
> > > > > > > + fd = open(path, flags, 0644);
> > > > > > > + if (fd < 0) {
> > > > > > > + error_report("cannot open %s", path);
> > > > > > > + return -1;
> > > > > > > + }
> > > > > > > + len = writev(fd, out_sg, out_num);
> > > > > > > + close(fd);
> > > > > > > +
> > > > > > > + return len;
> > > > > >
> > > > > > All this is blocking VM until host io completes.
> > > > >
> > > > > Hmm.. I don't know about the internals of qemu. So does it make guest
> > > > > stop? If so, that's what I want to do for _DMESG. :) As it's called
> > > > > only on kernel oops I think it's admittable. But for _CONSOLE, it
> > > > > needs to do asynchronously. Maybe I can add a thread to do the work.
> > > >
> > > > Please look at include/io/channel.h. QEMU is event-driven and tends to
> > > > use asynchronous I/O instead of spawning threads. The include/io/ APIs
> > > > allow you to do asynchronous I/O in the event loop.
> > >
> > > That is true, except for I/O to/from plain files - the QIOChannelFile
> > > impl doesn't do anything special (yet) to make that work correctly in
> > > non-blocking mode. Of course that could be fixed...
> >
> > Yep, I don't know how I can use the QIOChannelFile for async IO.
> > AFAICS it's just a wrapper for normal readv/writev. Who is
> > responsible to do these IO when I use the IO channel API? Also does
> > it guarantee that all IOs are processed in order?
>
> I'd suggest just using QIOChannelFile regardless - we need to fix the
> blocking behaviour already for sake of the qemu chardev code, and your
> code just adds more pressure to fix it. I/O will be done in the order
> in which you make the calls, as with regular POSIX I/O funcs you're
> currently using.
Thanks for the info.
So can I simply replace regular IO funcs to QIOChannel API like below?
ioc = qio_channel_file_new_path(path, flags, 0644, &err);
qio_channel_set_blocking(ioc, false, &err);
len = qio_channel_writev(ioc, out_sg, out_num, &err);
qio_channel_file_close(ioc, &err);
return len;
Or do I need to call qio_channel_add_watch() or something?
Thanks,
Namhyung
^ permalink raw reply
* Re: [PATCH] drivers: virtio_blk: notify blk-core when hw-queue number changes
From: Paolo Bonzini @ 2016-08-01 9:58 UTC (permalink / raw)
To: Bob Liu; +Cc: mst, linux-kernel, virtualization
In-Reply-To: <5799BFC0.1060804@oracle.com>
On 28/07/2016 10:18, Bob Liu wrote:
>>> >> A guest might be migrated to other hosts with different num_queues, the
>>> >> blk-core should aware of that else the reference of &vblk->vqs[qid] may be wrong.
>>> >>
>>> >> Signed-off-by: Bob Liu <bob.liu@oracle.com>
>>> >> ---
>>> >> drivers/block/virtio_blk.c | 3 +++
>>> >> 1 file changed, 3 insertions(+)
>>> >>
>>> >> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>>> >> index 42758b5..c169238 100644
>>> >> --- a/drivers/block/virtio_blk.c
>>> >> +++ b/drivers/block/virtio_blk.c
>>> >> @@ -819,6 +819,9 @@ static int virtblk_restore(struct virtio_device *vdev)
>>> >> if (ret)
>>> >> return ret;
>>> >>
>>> >> + if (vblk->num_vqs != vblk->tag_set.nr_hw_queues)
>>> >> + blk_mq_update_nr_hw_queues(&vblk->tag_set, vblk->num_vqs);
>>> >> +
>>> >> virtio_device_ready(vdev);
>>> >>
>>> >> blk_mq_start_stopped_hw_queues(vblk->disk->queue, true);
>>> >>
>> >
>> > This should never happen; it'd be a configuration problem.
>> >
> Do you mean all hosts have to be configured with the same number of ->num_vqs?
> What about cases like migrating a guest from HostA to HostB while HostB is much more powerful
> and would like to run more hardware queues to get better performance.
The number of queues should be equal to the number of CPUs for the
guest, not to the number of CPUs or queues on the host. The idea is to
have one queue per guest vCPU.
Paolo
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device
From: Daniel P. Berrange @ 2016-08-01 9:24 UTC (permalink / raw)
To: Namhyung Kim
Cc: Tony Luck, Kees Cook, kvm, Radim Kr??m????, LKML,
Michael S. Tsirkin, qemu-devel, Steven Rostedt, virtualization,
Minchan Kim, Anton Vorontsov, Anthony Liguori, Colin Cross,
Paolo Bonzini, Ingo Molnar
In-Reply-To: <20160730085702.GB26275@danjae.aot.lge.com>
On Sat, Jul 30, 2016 at 05:57:02PM +0900, Namhyung Kim wrote:
> On Thu, Jul 28, 2016 at 02:22:39PM +0100, Daniel P. Berrange wrote:
> > > +static void virtio_pstore_from_filename(VirtIOPstore *s, char *name,
> > > + char *buf, size_t sz,
> > > + struct virtio_pstore_fileinfo *info)
> > > +{
> > > + snprintf(buf, sz, "%s/%s", s->directory, name);
> > > +
> > > + if (g_str_has_prefix(name, "dmesg-")) {
> > > + info->type = VIRTIO_PSTORE_TYPE_DMESG;
> > > + name += strlen("dmesg-");
> > > + } else if (g_str_has_prefix(name, "console-")) {
> > > + info->type = VIRTIO_PSTORE_TYPE_CONSOLE;
> > > + name += strlen("console-");
> > > + } else if (g_str_has_prefix(name, "unknown-")) {
> > > + info->type = VIRTIO_PSTORE_TYPE_UNKNOWN;
> > > + name += strlen("unknown-");
> > > + }
[snip]
> > > + struct virtio_pstore_fileinfo info;
> > > + size_t offset = sizeof(*res) + sizeof(info);
> > > +
> > > + if (s->dirp == NULL) {
> > > + return -1;
> > > + }
> > > +
> > > + dent = readdir(s->dirp);
> > > + while (dent) {
> > > + if (dent->d_name[0] != '.') {
> > > + break;
> > > + }
> > > + dent = readdir(s->dirp);
> > > + }
> > > +
> > > + if (dent == NULL) {
> > > + return 0;
> > > + }
> >
> > So this seems to just be picking the first filename reported by
> > readdir that isn't starting with '.'. Surely this can't the right
> > logic when your corresponding do_write method can pick several
> > different filenames, its potluck which do_read will give back.
>
> Do you mean that it'd be better to check a list of known filenames and
> fail if not?
No, I mean that you have several different VIRTIO_PSTORE_TYPE_nnn and
use a different file for each constant. When reading this directory
though you're not looking for the file corresponding to any given
VIRTIO_PSTORE_TYPE_nnn - you're simply reading whichever file is found
first. So you might have just read a TYPE_CONSOLE or have read a
TYPE_DMESG - it surely doesn't make sense to randonly read either
TYPE_CONSOLE or TYPE_DMESG based on whatever order readdir() lists
the files.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device
From: Daniel P. Berrange @ 2016-08-01 9:21 UTC (permalink / raw)
To: Namhyung Kim
Cc: Tony Luck, Kees Cook, kvm, Radim Kr??m????, Anton Vorontsov, LKML,
Steven Rostedt, qemu-devel, Minchan Kim, Michael S. Tsirkin,
Anthony Liguori, Colin Cross, Paolo Bonzini, virtualization,
Ingo Molnar
In-Reply-To: <20160730083827.GA26275@danjae.aot.lge.com>
On Sat, Jul 30, 2016 at 05:38:27PM +0900, Namhyung Kim wrote:
> Hello,
>
> On Thu, Jul 28, 2016 at 02:08:41PM +0100, Daniel P. Berrange wrote:
> > On Thu, Jul 28, 2016 at 01:56:07PM +0100, Stefan Hajnoczi wrote:
> > > On Thu, Jul 28, 2016 at 02:39:53PM +0900, Namhyung Kim wrote:
> > > > On Thu, Jul 28, 2016 at 03:02:54AM +0300, Michael S. Tsirkin wrote:
> > > > > On Thu, Jul 28, 2016 at 12:08:30AM +0900, Namhyung Kim wrote:
> > > > > > +static ssize_t virtio_pstore_do_write(VirtIOPstore *s, struct iovec *out_sg,
> > > > > > + unsigned int out_num,
> > > > > > + struct virtio_pstore_req *req)
> > > > > > +{
> > > > > > + char path[PATH_MAX];
> > > > > > + int fd;
> > > > > > + ssize_t len;
> > > > > > + unsigned short type;
> > > > > > + int flags = O_WRONLY | O_CREAT;
> > > > > > +
> > > > > > + /* we already consume the req */
> > > > > > + iov_discard_front(&out_sg, &out_num, sizeof(*req));
> > > > > > +
> > > > > > + virtio_pstore_to_filename(s, path, sizeof(path), req);
> > > > > > +
> > > > > > + type = le16_to_cpu(req->type);
> > > > > > +
> > > > > > + if (type == VIRTIO_PSTORE_TYPE_DMESG) {
> > > > > > + flags |= O_TRUNC;
> > > > > > + } else if (type == VIRTIO_PSTORE_TYPE_CONSOLE) {
> > > > > > + flags |= O_APPEND;
> > > > > > + }
> > > > > > +
> > > > > > + fd = open(path, flags, 0644);
> > > > > > + if (fd < 0) {
> > > > > > + error_report("cannot open %s", path);
> > > > > > + return -1;
> > > > > > + }
> > > > > > + len = writev(fd, out_sg, out_num);
> > > > > > + close(fd);
> > > > > > +
> > > > > > + return len;
> > > > >
> > > > > All this is blocking VM until host io completes.
> > > >
> > > > Hmm.. I don't know about the internals of qemu. So does it make guest
> > > > stop? If so, that's what I want to do for _DMESG. :) As it's called
> > > > only on kernel oops I think it's admittable. But for _CONSOLE, it
> > > > needs to do asynchronously. Maybe I can add a thread to do the work.
> > >
> > > Please look at include/io/channel.h. QEMU is event-driven and tends to
> > > use asynchronous I/O instead of spawning threads. The include/io/ APIs
> > > allow you to do asynchronous I/O in the event loop.
> >
> > That is true, except for I/O to/from plain files - the QIOChannelFile
> > impl doesn't do anything special (yet) to make that work correctly in
> > non-blocking mode. Of course that could be fixed...
>
> Yep, I don't know how I can use the QIOChannelFile for async IO.
> AFAICS it's just a wrapper for normal readv/writev. Who is
> responsible to do these IO when I use the IO channel API? Also does
> it guarantee that all IOs are processed in order?
I'd suggest just using QIOChannelFile regardless - we need to fix the
blocking behaviour already for sake of the qemu chardev code, and your
code just adds more pressure to fix it. I/O will be done in the order
in which you make the calls, as with regular POSIX I/O funcs you're
currently using.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply
* Re: ext4 error when testing virtio-scsi & vhost-scsi
From: Zhangfei Gao @ 2016-08-01 2:40 UTC (permalink / raw)
To: Jan Kara
Cc: Theodore Ts'o, kvm, Michael S. Tsirkin, qemu-devel,
virtualization, target-devel, linux-ext4
In-Reply-To: <CAMj5BkgqYq4feie3HVpp673JyFq+SEXRn2YiaO=v_v9RqW_pBw@mail.gmail.com>
Hi, Jan
On Thu, Jul 28, 2016 at 9:29 AM, Zhangfei Gao <zhangfei.gao@gmail.com> wrote:
> Hi, Jan
>
> On Wed, Jul 27, 2016 at 11:56 PM, Jan Kara <jack@suse.cz> wrote:
>> Hi!
>>
>> On Wed 27-07-16 15:58:55, Zhangfei Gao wrote:
>>> Hi, Michael
>>>
>>> I have met ext4 error when using vhost_scsi on arm64 platform, and
>>> suspect it is vhost_scsi issue.
>>>
>>> Ext4 error when testing virtio_scsi & vhost_scsi
>>>
>>>
>>> No issue:
>>> 1. virtio_scsi, ext4
>>> 2. vhost_scsi & virtio_scsi, ext2
>>> 3. Instead of vhost, also tried loopback and no problem.
>>> Using loopback, host can use the new block device, while vhost is used
>>> by guest (qemu).
>>> http://www.linux-iscsi.org/wiki/Tcm_loop
>>> Test directly in host, not find ext4 error.
>>>
>>>
>>>
>>> Have issue:
>>> 1. vhost_scsi & virtio_scsi, ext4
>>> a. iblock
>>> b, fileio, file located in /tmp (ram), no device based.
>>>
>>> 2, Have tried 4.7-r2 and 4.5-rc1 on D02 board, both have issue.
>>> Since I need kvm specific patch for D02, so it may not freely to switch
>>> to older version.
>>>
>>> 3. Also test with ext4, disabling journal
>>> mkfs.ext4 -O ^has_journal /dev/sda
>>>
>>>
>>> Do you have any suggestion?
>>
>> So can you mount the filesystem with errors=remount-ro to avoid clobbering
>> the fs after the problem happens? And then run e2fsck on the problematic
>> filesystem and send the output here?
>>
>
> Tested twice, log pasted.
> Both using fileio, located in host ramfs /tmp
> Before e2fsck, umount /dev/sda
>
> 1.
> root@(none)$ mount -o errors=remount-ro /dev/sda /mnt
> [ 22.812053] EXT4-fs (sda): mounted filesystem with ordered data
> mode. Opts: errors=remount-ro
> $ rm /mnt/test
> [ 108.388905] EXT4-fs error (device sda) in
> ext4_reserve_inode_write:5362: Corrupt filesystem
> [ 108.406930] Aborting journal on device sda-8.
> [ 108.414120] EXT4-fs (sda): Remounting filesystem read-only
> [ 108.414847] EXT4-fs error (device sda) in ext4_dirty_inode:5487: IO failure
> [ 108.423571] EXT4-fs error (device sda) in ext4_free_blocks:4904:
> Journal has aborted
> [ 108.431919] EXT4-fs error (device sda) in
> ext4_reserve_inode_write:5362: Corrupt filesystem
> [ 108.440269] EXT4-fs error (device sda) in
> ext4_reserve_inode_write:5362: Corrupt filesystem
> [ 108.448568] EXT4-fs error (device sda) in
> ext4_ext_remove_space:3058: IO failure
> [ 108.456917] EXT4-fs error (device sda) in ext4_ext_truncate:4657:
> Corrupt filesystem
> [ 108.465267] EXT4-fs error (device sda) in
> ext4_reserve_inode_write:5362: Corrupt filesystem
> [ 108.473567] EXT4-fs error (device sda) in ext4_truncate:4150: IO failure
> [ 108.481917] EXT4-fs error (device sda) in
> ext4_reserve_inode_write:5362: Corrupt filesystem
> root@(none)$ e2fsck /dev/sda
> e2fsck 1.42.9 (28-Dec-2013)
> /dev/sda is mounted.
> e2fsck: Cannot continue, aborting.
>
>
> root@(none)$ umount /mnt
> [ 260.756250] EXT4-fs error (device sda): ext4_put_super:837:
> Couldn't clean up the journal
> root@(none)$ umount /mnt e2fsck /dev/sda
> e2fsck 1.42.9 (28-Dec-2013)
> ext2fs_open2: Bad magic number in super-block
> e2fsck: Superblock invalid, trying backup blocks...
> Superblock needs_recovery flag is clear, but journal has data.
> Recovery flag not set in backup superblock, so running journal anyway.
> /dev/sda: recovering journal
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> Pass 3: Checking directory connectivity
> Pass 4: Checking reference counts
> Pass 5: Checking group summary information
> Free blocks count wrong for group #1 (32703, counted=8127).
> Fix<y>? yes
> Free blocks count wrong for group #2 (32768, counted=31744).
> Fix<y>? yes
> Free blocks count wrong (249509, counted=223909).
> Fix<y>? yes
> Free inodes count wrong for group #0 (8181, counted=8180).
> Fix<y>? yes
> Free inodes count wrong (65525, counted=65524).
> Fix<y>? yes
>
> /dev/sda: ***** FILE SYSTEM WAS MODIFIED *****
> /dev/sda: 12/65536 files (8.3% non-contiguous), 38235/262144 blocks
> root@(none)$
>
> 2.
>
> root@(none)$ rm /mnt/test
> [ 71.021484] EXT4-fs error (device sda) in
> ext4_reserve_inode_write:5362: Corrupt filesystem
> [ 71.044959] Aborting journal on device sda-8.
> [ 71.052152] EXT4-fs (sda): Remounting filesystem read-only
> [ 71.052833] EXT4-fs error (device sda) in ext4_dirty_inode:5487: IO failure
> [ 71.061600] EXT4-fs error (device sda) in ext4_free_blocks:4904:
> Journal has aborted
> [ 71.069948] EXT4-fs error (device sda) in
> ext4_reserve_inode_write:5362: Corrupt filesystem
> [ 71.078296] EXT4-fs error (device sda) in
> ext4_reserve_inode_write:5362: Corrupt filesystem
> [ 71.086597] EXT4-fs error (device sda) in
> ext4_ext_remove_space:3058: IO failure
> [ 71.094946] EXT4-fs error (device sda) in ext4_ext_truncate:4657:
> Corrupt filesystem
> [ 71.103296] EXT4-fs error (device sda) in
> ext4_reserve_inode_write:5362: Corrupt filesystem
> [ 71.111595] EXT4-fs error (device sda) in ext4_truncate:4150: IO failure
> [ 71.119946] EXT4-fs error (device sda) in
> ext4_reserve_inode_write:5362: Corrupt filesystem
> root@(none)$ e2fsck /dev/sda
> e2fsck 1.42.9 (28-Dec-2013)
> /dev/sda is mounted.
> e2fsck: Cannot continue, aborting.
>
>
> root@(none)$ umou nt /mnt/
> [ 92.103221] EXT4-fs error (device sda): ext4_put_super:837:
> Couldn't clean up the journal
> root@(none)$ umount /mnt/ e2fsck /dev/sda
> e2fsck 1.42.9 (28-Dec-2013)
> ext2fs_open2: Bad magic number in super-block
> e2fsck: Superblock invalid, trying backup blocks...
> Superblock needs_recovery flag is clear, but journal has data.
> Recovery flag not set in backup superblock, so running journal anyway.
> /dev/sda: recovering journal
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> Pass 3: Checking directory connectivity
> Pass 4: Checking reference counts
> Pass 5: Checking group summary information
> Free blocks count wrong for group #1 (32703, counted=8127).
> Fix<y>? yes
> Free blocks count wrong for group #2 (32768, counted=31744).
> Fix<y>? yes
> Free blocks count wrong (249509, counted=223909).
> Fix<y>? yes
> Free inodes count wrong for group #0 (8181, counted=8180).
> Fix<y>? yes
> Free inodes count wrong (65525, counted=65524).
> Fix<y>? yes
>
> /dev/sda: ***** FILE SYSTEM WAS MODIFIED *****
> /dev/sda: 12/65536 files (8.3% non-contiguous), 38235/262144 blocks
> root@(none)$
>
One more test on another different arm64 machine (apm-mustang).
root@(none)$ dd if=/dev/zero of=/mnt/test bs=1M count=100; sync;
[ 117.556265] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem
[ 117.570231] Aborting journal on device sda-8.
[ 117.582769] EXT4-fs (sda): Remounting filesystem read-only
[ 117.583739] EXT4-fs error (device sda) in ext4_dirty_inode:5297: IO failure
[ 117.596578] EXT4-fs error (device sda) in ext4_free_blocks:4897:
Journal has aborted
[ 117.609122] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem
[ 117.622970] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem
[ 117.635486] EXT4-fs error (device sda) in
ext4_ext_remove_space:3044: IO failure
[ 117.649351] EXT4-fs error (device sda) in ext4_ext_truncate:4661:
Corrupt filesystem
[ 117.661875] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem
[ 117.675717] EXT4-fs error (device sda) in ext4_orphan_del:2896:
Corrupt filesystem
[ 117.688235] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem
dd: writing '/mnt/test': Read-only file system
1+0 records in
0+0 records out
root@(none)$ umount /mnt
[ 126.637862] EXT4-fs error (device sda): ext4_put_super:838:
Couldn't clean up the journal
root@(none)$ e2fsck /dev/sda
e2fsck 1.42.9 (28-Dec-2013)
ext2fs_open2: Bad magic number in super-block
e2fsck: Superblock invalid, trying backup blocks...
Superblock needs_recovery flag is clear, but journal has data.
Recovery flag not set in backup superblock, so running journal anyway.
/dev/sda: recovering journal
Pass 1: Checking inodes, blocks, and sizes
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information
Block bitmap differences: -(86016--87039)
Fix<y>? yes
Free blocks count wrong for group #1 (32703, counted=7103).
Fix<y>? yes
Free blocks count wrong (249509, counted=223909).
Fix<y>? yes
Free inodes count wrong for group #0 (8181, counted=8180).
Fix<y>? yes
Free inodes count wrong (65525, counted=65524).
Fix<y>? yes
/dev/sda: ***** FILE SYSTEM WAS MODIFIED *****
/dev/sda: 12/65536 files (8.3% non-contiguous), 38235/262144 blocks
Do you know what's the possible reason of this error?
I got from your comments from other mail.
"
Hum, interesting. So 'Free blocks count wrong' and 'Free inodes count
wrong' messages are harmless - those entries and updated only
opportunistically and on mount and generally do not have to match on live
filesystem. The other three errors regarding inode and directory count are
a fallout from aborted inode deletion. Most importantly there is *no
problem* whatsoever with block bitmaps. So it was either some memory glitch
(bitflip in the counter or the bitmap) or there is some race and bb_free
can get out of sync with the bitmap and I don't see how that could happen
especially so early after mount... Strange.
"
there is such error:
Block bitmap differences: -(86016--87039)
Thanks
^ permalink raw reply
* Re: getrandom waits for a long time when /dev/random is insufficiently read from
From: Alex Xu via Virtualization @ 2016-07-31 1:53 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Linux Crypto Mailing List, virtualization
In-Reply-To: <20160730220922.GA12853@thunk.org>
On Sat, 30 Jul 2016 18:09:22 -0400
Theodore Ts'o <tytso@mit.edu> wrote as excerpted:
> On Fri, Jul 29, 2016 at 01:31:14PM -0400, Alex Xu wrote:
> > When qemu is started with -object rng-random,filename=/dev/urandom,
> > and immediately (i.e. with no initrd and as the first thing in
> > init):
> >
> > 1. the guest runs dd if=/dev/random, there is no blocking and tons
> > of data goes to the screen. the data appears to be random.
> >
> > 2. the guest runs getrandom with any requested amount (tested 1 byte
> > and 16 bytes) and no flags, it blocks for 90-110 seconds while the
> > "non-blocking pool is initialized". the returned data appears to be
> > random.
> >
> > 3. the guest runs getrandom with GRND_RANDOM with any requested
> > amount, it returns the desired amount or possibly less, but in my
> > experience at least 10 bytes. the returned data appears to be
> > random.
> >
> > I believe that the difference between cases 1 and 2 is a bug, since
> > based on my previous statement, in this scenario, getrandom should
> > never block.
>
> This is correct; and it has been fixed in the patches in v4.8-rc1.
> The patch which fixes this has been marked for backporting to stable
> kernels:
>
> commit 3371f3da08cff4b75c1f2dce742d460539d6566d
> Author: Theodore Ts'o <tytso@mit.edu>
> Date: Sun Jun 12 18:11:51 2016 -0400
>
> random: initialize the non-blocking pool via
> add_hwgenerator_randomness()
> If we have a hardware RNG and are using the in-kernel rngd, we
> should use this to initialize the non-blocking pool so that
> getrandom(2) doesn't block unnecessarily.
>
> Cc: stable@kernel.org
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
>
> Basically, the urandom pool (now CSRPNG) wasn't getting initialized
> from the hardware random number generator. Most people didn't notice
> because very few people actually *use* hardware random number
> generators (although it's much more common in VM's, which is how
> you're using it), and use of getrandom(2) is still relatively rare,
> given that glibc hasn't yet seen fit to support it yet.
>
> Cheers,
>
> - Ted
Dammit, the one time I track down an actual kernel bug someone's already
fixed it. I'd even bothered to check 4.6 so I figured nobody'd gotten
around to it yet.
Thanks for the excellent explanations though. :)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox