* CFP ICEIS 2019 - 21st Int.l Conf. on Enterprise Information Systems (Heraklion, Crete/Greece)
From: iceis @ 2018-10-16 9:32 UTC (permalink / raw)
To: virtualization
SUBMISSION DEADLINE
21st International Conference on Enterprise Information Systems
Submission Deadline: December 10, 2018
http://www.iceis.org/
May 3 - 5, 2019
Heraklion, Crete, Greece.
ICEIS is organized in 6 major tracks:
- Databases and Information Systems Integration
- Artificial Intelligence and Decision Support Systems
- Information Systems Analysis and Specification
- Software Agents and Internet Computing
- Human-Computer Interaction
- Enterprise Architecture
In Cooperation with: ACM SIGCAS, EDSO, Cluster Habitat Sustentável, OSGP Alliance and Siemens. <br/>
Proceedings will be submitted for indexation by: DBLP, Thomson Reuters, EI, SCOPUS and Semantic Scholar. <br/>
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
Should you have any question please don't hesitate contacting me.
Kind regards,
ICEIS Secretariat
Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 184
Fax: +351 265 520 186
Web: http://www.iceis.org/
e-mail: iceis.secretariat@insticc.org
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* CFP IoTBDS 2019 - 4th Int.l Conf. on Internet of Things, Big Data and Security (Heraklion, Crete/Greece)
From: iotbds @ 2018-10-16 9:32 UTC (permalink / raw)
To: virtualization
SUBMISSION DEADLINE
4th International Conference on Internet of Things, Big Data and Security
Submission Deadline: December 10, 2018
http://iotbds.org/
May 2 - 4, 2019
Heraklion, Crete, Greece.
IoTBDS is organized in 7 major tracks:
- Big Data Research
- Emerging Services and Analytics
- Internet of Things (IoT) Fundamentals
- Internet of Things (IoT) Applications
- Big Data for Multi-discipline Services
- Security, Privacy and Trust
- IoT Technologies
In Cooperation with: ACM SIGCAS, EDSO, Cluster Habitat Sustentável, OSGP Alliance and Siemens. <br/>
Proceedings will be submitted for indexation by: DBLP, Thomson Reuters, EI, SCOPUS and Semantic Scholar. <br/>
With the presence of internationally distinguished keynote speakers:
Francisco Herrera, University of Granada, Spain
Bonghee Hong, Pusan National University, Korea, Republic of
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
Should you have any question please don't hesitate contacting me.
Kind regards,
IoTBDS Secretariat
Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 184
Fax: +351 265 520 186
Web: http://iotbds.org/
e-mail: iotbds.secretariat@insticc.org
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* CFP CLOSER 2019 - 9th Int.l Conf. on Cloud Computing and Services Science (Heraklion, Crete/Greece)
From: closer @ 2018-10-16 9:32 UTC (permalink / raw)
To: virtualization
SUBMISSION DEADLINE
9th International Conference on Cloud Computing and Services Science
Submission Deadline: December 10, 2018
http://closer.scitevents.org
May 2 - 4, 2019
Heraklion, Crete, Greece.
CLOSER is organized in 9 major tracks:
- Services Science
- Data as a Service
- Cloud Operations
- Edge Cloud and Fog Computing
- Service Modelling and Analytics
- Mobile Cloud Computing
- Cloud Computing Fundamentals
- Cloud Computing Platforms and Applications
- Cloud Computing Enabling Technology
In Cooperation with: ACM SIGCAS, EDSO, Cluster Habitat Sustentável, OSGP Alliance and Siemens. <br/>
Proceedings will be submitted for indexation by: DBLP, Thomson Reuters, EI, SCOPUS and Semantic Scholar. <br/>
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
Should you have any question please don't hesitate contacting me.
Kind regards,
CLOSER Secretariat
Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 184
Fax: +351 265 520 186
Web: http://closer.scitevents.org
e-mail: closer.secretariat@insticc.org
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 1/2] drm/ttm: Rename ttm_bo_global_{init,release}() to ttm_bo_global_ref_{,}()
From: Koenig, Christian @ 2018-10-16 10:43 UTC (permalink / raw)
To: Thomas Zimmermann, Huang, Ray, Zhang, Jerry, airlied@linux.ie,
daniel.vetter@ffwll.ch, maarten.lankhorst@linux.intel.com,
maxime.ripard@bootlin.com, sean@poorly.run, Deucher, Alexander,
corbet@lwn.net, Zhou, David(ChunMing), kraxel@redhat.com,
z.liuxinliang@hisilicon.com, zourongrong@gmail.com,
kong.kongxinwei@hisilicon.com, puck.chen@hisilicon.com,
bskeggs@redhat.com, linux-graphics-maintainer@vmware.com
Cc: nouveau@lists.freedesktop.org,
virtualization@lists.linux-foundation.org
In-Reply-To: <20181016080409.18838-2-tzimmermann@suse.de>
Am 16.10.2018 um 10:04 schrieb Thomas Zimmermann:
> The functions ttm_bo_global_init() and ttm_bo_global_release() do not
> receive an argument of type struct ttm_bo_global. Both take a struct
> drm_global_reference that contains points to a struct ttm_bo_global_ref.
> Renaming them reflects this.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed and pushed upstream.
Christian.
> ---
> Documentation/gpu/drm-mm.rst | 4 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++--
> drivers/gpu/drm/ast/ast_ttm.c | 4 ++--
> drivers/gpu/drm/bochs/bochs_mm.c | 4 ++--
> drivers/gpu/drm/cirrus/cirrus_ttm.c | 4 ++--
> drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 4 ++--
> drivers/gpu/drm/mgag200/mgag200_ttm.c | 4 ++--
> drivers/gpu/drm/nouveau/nouveau_ttm.c | 4 ++--
> drivers/gpu/drm/qxl/qxl_ttm.c | 4 ++--
> drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++--
> drivers/gpu/drm/ttm/ttm_bo.c | 8 ++++----
> drivers/gpu/drm/virtio/virtgpu_ttm.c | 4 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 4 ++--
> drivers/staging/vboxvideo/vbox_ttm.c | 4 ++--
> include/drm/ttm/ttm_bo_driver.h | 4 ++--
> 15 files changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index e725e8449e72..d0f3c6b03200 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -72,8 +72,8 @@ object TTM to provide a pool for buffer object allocation by clients and
> the kernel itself. The type of this object should be
> TTM_GLOBAL_TTM_BO, and its size should be sizeof(struct
> ttm_bo_global). Again, driver-specific init and release functions may
> -be provided, likely eventually calling ttm_bo_global_init() and
> -ttm_bo_global_release(), respectively. Also, like the previous
> +be provided, likely eventually calling ttm_bo_global_ref_init() and
> +ttm_bo_global_ref_release(), respectively. Also, like the previous
> object, ttm_global_item_ref() is used to create an initial reference
> count for the TTM, which will call your initialization function.
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index a44fc12ae1f9..3a6802846698 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -125,8 +125,8 @@ static int amdgpu_ttm_global_init(struct amdgpu_device *adev)
> global_ref = &adev->mman.bo_global_ref.ref;
> global_ref->global_type = DRM_GLOBAL_TTM_BO;
> global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_init;
> - global_ref->release = &ttm_bo_global_release;
> + global_ref->init = &ttm_bo_global_ref_init;
> + global_ref->release = &ttm_bo_global_ref_release;
> r = drm_global_item_ref(global_ref);
> if (r) {
> DRM_ERROR("Failed setting up TTM BO subsystem.\n");
> diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
> index fe354ebf374d..d21fbd26785a 100644
> --- a/drivers/gpu/drm/ast/ast_ttm.c
> +++ b/drivers/gpu/drm/ast/ast_ttm.c
> @@ -70,8 +70,8 @@ static int ast_ttm_global_init(struct ast_private *ast)
> global_ref = &ast->ttm.bo_global_ref.ref;
> global_ref->global_type = DRM_GLOBAL_TTM_BO;
> global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_init;
> - global_ref->release = &ttm_bo_global_release;
> + global_ref->init = &ttm_bo_global_ref_init;
> + global_ref->release = &ttm_bo_global_ref_release;
> r = drm_global_item_ref(global_ref);
> if (r != 0) {
> DRM_ERROR("Failed setting up TTM BO subsystem.\n");
> diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
> index e6ccf7fa92d4..ff4f41dec228 100644
> --- a/drivers/gpu/drm/bochs/bochs_mm.c
> +++ b/drivers/gpu/drm/bochs/bochs_mm.c
> @@ -48,8 +48,8 @@ static int bochs_ttm_global_init(struct bochs_device *bochs)
> global_ref = &bochs->ttm.bo_global_ref.ref;
> global_ref->global_type = DRM_GLOBAL_TTM_BO;
> global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_init;
> - global_ref->release = &ttm_bo_global_release;
> + global_ref->init = &ttm_bo_global_ref_init;
> + global_ref->release = &ttm_bo_global_ref_release;
> r = drm_global_item_ref(global_ref);
> if (r != 0) {
> DRM_ERROR("Failed setting up TTM BO subsystem.\n");
> diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c b/drivers/gpu/drm/cirrus/cirrus_ttm.c
> index f21953243790..2e2141f26c5b 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
> @@ -70,8 +70,8 @@ static int cirrus_ttm_global_init(struct cirrus_device *cirrus)
> global_ref = &cirrus->ttm.bo_global_ref.ref;
> global_ref->global_type = DRM_GLOBAL_TTM_BO;
> global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_init;
> - global_ref->release = &ttm_bo_global_release;
> + global_ref->init = &ttm_bo_global_ref_init;
> + global_ref->release = &ttm_bo_global_ref_release;
> r = drm_global_item_ref(global_ref);
> if (r != 0) {
> DRM_ERROR("Failed setting up TTM BO subsystem.\n");
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> index 2e3e0bdb8932..0454aa43ffc6 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> @@ -59,8 +59,8 @@ static int hibmc_ttm_global_init(struct hibmc_drm_private *hibmc)
> hibmc->mem_global_ref.object;
> hibmc->bo_global_ref.ref.global_type = DRM_GLOBAL_TTM_BO;
> hibmc->bo_global_ref.ref.size = sizeof(struct ttm_bo_global);
> - hibmc->bo_global_ref.ref.init = &ttm_bo_global_init;
> - hibmc->bo_global_ref.ref.release = &ttm_bo_global_release;
> + hibmc->bo_global_ref.ref.init = &ttm_bo_global_ref_init;
> + hibmc->bo_global_ref.ref.release = &ttm_bo_global_ref_release;
> ret = drm_global_item_ref(&hibmc->bo_global_ref.ref);
> if (ret) {
> DRM_ERROR("failed setting up TTM BO subsystem: %d\n", ret);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c
> index 05570f0de4d7..3444b539e7f4 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
> @@ -70,8 +70,8 @@ static int mgag200_ttm_global_init(struct mga_device *ast)
> global_ref = &ast->ttm.bo_global_ref.ref;
> global_ref->global_type = DRM_GLOBAL_TTM_BO;
> global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_init;
> - global_ref->release = &ttm_bo_global_release;
> + global_ref->init = &ttm_bo_global_ref_init;
> + global_ref->release = &ttm_bo_global_ref_release;
> r = drm_global_item_ref(global_ref);
> if (r != 0) {
> DRM_ERROR("Failed setting up TTM BO subsystem.\n");
> diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
> index 8edb9f2a4269..a293383c8654 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
> @@ -209,8 +209,8 @@ nouveau_ttm_global_init(struct nouveau_drm *drm)
> global_ref = &drm->ttm.bo_global_ref.ref;
> global_ref->global_type = DRM_GLOBAL_TTM_BO;
> global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_init;
> - global_ref->release = &ttm_bo_global_release;
> + global_ref->init = &ttm_bo_global_ref_init;
> + global_ref->release = &ttm_bo_global_ref_release;
>
> ret = drm_global_item_ref(global_ref);
> if (unlikely(ret != 0)) {
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index 86a1fb32f6db..db2a0036e9c4 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -80,8 +80,8 @@ static int qxl_ttm_global_init(struct qxl_device *qdev)
> global_ref = &qdev->mman.bo_global_ref.ref;
> global_ref->global_type = DRM_GLOBAL_TTM_BO;
> global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_init;
> - global_ref->release = &ttm_bo_global_release;
> + global_ref->init = &ttm_bo_global_ref_init;
> + global_ref->release = &ttm_bo_global_ref_release;
> r = drm_global_item_ref(global_ref);
> if (r != 0) {
> DRM_ERROR("Failed setting up TTM BO subsystem.\n");
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index cbb67e9ffb3a..dac4ec5a120b 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -97,8 +97,8 @@ static int radeon_ttm_global_init(struct radeon_device *rdev)
> global_ref = &rdev->mman.bo_global_ref.ref;
> global_ref->global_type = DRM_GLOBAL_TTM_BO;
> global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_init;
> - global_ref->release = &ttm_bo_global_release;
> + global_ref->init = &ttm_bo_global_ref_init;
> + global_ref->release = &ttm_bo_global_ref_release;
> r = drm_global_item_ref(global_ref);
> if (r != 0) {
> DRM_ERROR("Failed setting up TTM BO subsystem.\n");
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 26b889f86670..9c2bb880491e 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1522,16 +1522,16 @@ static void ttm_bo_global_kobj_release(struct kobject *kobj)
> kfree(glob);
> }
>
> -void ttm_bo_global_release(struct drm_global_reference *ref)
> +void ttm_bo_global_ref_release(struct drm_global_reference *ref)
> {
> struct ttm_bo_global *glob = ref->object;
>
> kobject_del(&glob->kobj);
> kobject_put(&glob->kobj);
> }
> -EXPORT_SYMBOL(ttm_bo_global_release);
> +EXPORT_SYMBOL(ttm_bo_global_ref_release);
>
> -int ttm_bo_global_init(struct drm_global_reference *ref)
> +int ttm_bo_global_ref_init(struct drm_global_reference *ref)
> {
> struct ttm_bo_global_ref *bo_ref =
> container_of(ref, struct ttm_bo_global_ref, ref);
> @@ -1564,7 +1564,7 @@ int ttm_bo_global_init(struct drm_global_reference *ref)
> kfree(glob);
> return ret;
> }
> -EXPORT_SYMBOL(ttm_bo_global_init);
> +EXPORT_SYMBOL(ttm_bo_global_ref_init);
>
>
> int ttm_bo_device_release(struct ttm_bo_device *bdev)
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ttm.c b/drivers/gpu/drm/virtio/virtgpu_ttm.c
> index e3152d45c5f1..526a5e48dc3b 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ttm.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ttm.c
> @@ -84,8 +84,8 @@ static int virtio_gpu_ttm_global_init(struct virtio_gpu_device *vgdev)
> global_ref = &vgdev->mman.bo_global_ref.ref;
> global_ref->global_type = DRM_GLOBAL_TTM_BO;
> global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_init;
> - global_ref->release = &ttm_bo_global_release;
> + global_ref->init = &ttm_bo_global_ref_init;
> + global_ref->release = &ttm_bo_global_ref_release;
> r = drm_global_item_ref(global_ref);
> if (r != 0) {
> DRM_ERROR("Failed setting up TTM BO subsystem.\n");
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> index 7b1e5a5cbd2c..f3ce43c41978 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> @@ -76,8 +76,8 @@ int vmw_ttm_global_init(struct vmw_private *dev_priv)
> global_ref = &dev_priv->bo_global_ref.ref;
> global_ref->global_type = DRM_GLOBAL_TTM_BO;
> global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_init;
> - global_ref->release = &ttm_bo_global_release;
> + global_ref->init = &ttm_bo_global_ref_init;
> + global_ref->release = &ttm_bo_global_ref_release;
> ret = drm_global_item_ref(global_ref);
>
> if (unlikely(ret != 0)) {
> diff --git a/drivers/staging/vboxvideo/vbox_ttm.c b/drivers/staging/vboxvideo/vbox_ttm.c
> index 548edb7c494b..2329a55d4636 100644
> --- a/drivers/staging/vboxvideo/vbox_ttm.c
> +++ b/drivers/staging/vboxvideo/vbox_ttm.c
> @@ -68,8 +68,8 @@ static int vbox_ttm_global_init(struct vbox_private *vbox)
> global_ref = &vbox->ttm.bo_global_ref.ref;
> global_ref->global_type = DRM_GLOBAL_TTM_BO;
> global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_init;
> - global_ref->release = &ttm_bo_global_release;
> + global_ref->init = &ttm_bo_global_ref_init;
> + global_ref->release = &ttm_bo_global_ref_release;
>
> ret = drm_global_item_ref(global_ref);
> if (ret) {
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index e4fee8e02559..c3c0751dec63 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -578,8 +578,8 @@ void ttm_bo_mem_put(struct ttm_buffer_object *bo, struct ttm_mem_reg *mem);
> void ttm_bo_mem_put_locked(struct ttm_buffer_object *bo,
> struct ttm_mem_reg *mem);
>
> -void ttm_bo_global_release(struct drm_global_reference *ref);
> -int ttm_bo_global_init(struct drm_global_reference *ref);
> +void ttm_bo_global_ref_release(struct drm_global_reference *ref);
> +int ttm_bo_global_ref_init(struct drm_global_reference *ref);
>
> int ttm_bo_device_release(struct ttm_bo_device *bdev);
>
^ permalink raw reply
* [RFC] virtio_net: add local_bh_disable() around u64_stats_update_begin
From: Sebastian Andrzej Siewior @ 2018-10-16 16:55 UTC (permalink / raw)
To: netdev, virtualization; +Cc: David S. Miller, tglx, Michael S. Tsirkin
on 32bit, lockdep notices:
| ================================
| WARNING: inconsistent lock state
| 4.19.0-rc8+ #9 Tainted: G W
| --------------------------------
| inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
| ip/1106 [HC0[0]:SC1[1]:HE1:SE0] takes:
| (ptrval) (&syncp->seq#2){+.?.}, at: net_rx_action+0xc8/0x380
| {SOFTIRQ-ON-W} state was registered at:
| lock_acquire+0x7e/0x170
| try_fill_recv+0x5fa/0x700
| virtnet_open+0xe0/0x180
| __dev_open+0xae/0x130
| __dev_change_flags+0x17f/0x200
| dev_change_flags+0x23/0x60
| do_setlink+0x2bb/0xa20
| rtnl_newlink+0x523/0x830
| rtnetlink_rcv_msg+0x14b/0x470
| netlink_rcv_skb+0x6e/0xf0
| rtnetlink_rcv+0xd/0x10
| netlink_unicast+0x16e/0x1f0
| netlink_sendmsg+0x1af/0x3a0
| ___sys_sendmsg+0x20f/0x240
| __sys_sendmsg+0x39/0x80
| sys_socketcall+0x13a/0x2a0
| do_int80_syscall_32+0x50/0x180
| restore_all+0x0/0xb2
| irq event stamp: 3326
| hardirqs last enabled at (3326): [<c159e6d0>] net_rx_action+0x80/0x380
| hardirqs last disabled at (3325): [<c159e6aa>] net_rx_action+0x5a/0x380
| softirqs last enabled at (3322): [<c14b440d>] virtnet_napi_enable+0xd/0x60
| softirqs last disabled at (3323): [<c101d63d>] call_on_stack+0xd/0x50
|
| other info that might help us debug this:
| Possible unsafe locking scenario:
|
| CPU0
| ----
| lock(&syncp->seq#2);
| <Interrupt>
| lock(&syncp->seq#2);
|
| *** DEADLOCK ***
This is the "up" path which is not a hotpath. There is also
refill_work().
It might be unwise to add the local_bh_disable() to try_fill_recv()
because if it is used mostly in BH so that local_bh_en+dis might be a
waste of cycles.
Adding local_bh_disable() around try_fill_recv() for the non-BH call
sites would render GFP_KERNEL pointless.
Also, ptr->var++ is not an atomic operation even on 64bit CPUs. Which
means if try_fill_recv() runs on CPU0 (via virtnet_receive()) then the
worker might run on CPU1.
Do we care or is this just stupid stats? Any suggestions?
This warning appears since commit 461f03dc99cf6 ("virtio_net: Add kick stats").
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
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 dab504ec5e502..d782160cfa882 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1206,9 +1206,11 @@ static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
break;
} while (rq->vq->num_free);
if (virtqueue_kick_prepare(rq->vq) && virtqueue_notify(rq->vq)) {
+ local_bh_disable();
u64_stats_update_begin(&rq->stats.syncp);
rq->stats.kicks++;
u64_stats_update_end(&rq->stats.syncp);
+ local_bh_enable();
}
return !oom;
--
2.19.1
^ permalink raw reply related
* Re: [RFC] virtio_net: add local_bh_disable() around u64_stats_update_begin
From: Stephen Hemminger @ 2018-10-16 17:59 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Michael S. Tsirkin, netdev, virtualization, tglx, David S. Miller
In-Reply-To: <20181016165545.guksrl23ulcudxrk@linutronix.de>
On Tue, 16 Oct 2018 18:55:45 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> on 32bit, lockdep notices:
> | ================================
> | WARNING: inconsistent lock state
> | 4.19.0-rc8+ #9 Tainted: G W
> | --------------------------------
> | inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
> | ip/1106 [HC0[0]:SC1[1]:HE1:SE0] takes:
> | (ptrval) (&syncp->seq#2){+.?.}, at: net_rx_action+0xc8/0x380
> | {SOFTIRQ-ON-W} state was registered at:
> | lock_acquire+0x7e/0x170
> | try_fill_recv+0x5fa/0x700
> | virtnet_open+0xe0/0x180
> | __dev_open+0xae/0x130
> | __dev_change_flags+0x17f/0x200
> | dev_change_flags+0x23/0x60
> | do_setlink+0x2bb/0xa20
> | rtnl_newlink+0x523/0x830
> | rtnetlink_rcv_msg+0x14b/0x470
> | netlink_rcv_skb+0x6e/0xf0
> | rtnetlink_rcv+0xd/0x10
> | netlink_unicast+0x16e/0x1f0
> | netlink_sendmsg+0x1af/0x3a0
> | ___sys_sendmsg+0x20f/0x240
> | __sys_sendmsg+0x39/0x80
> | sys_socketcall+0x13a/0x2a0
> | do_int80_syscall_32+0x50/0x180
> | restore_all+0x0/0xb2
> | irq event stamp: 3326
> | hardirqs last enabled at (3326): [<c159e6d0>] net_rx_action+0x80/0x380
> | hardirqs last disabled at (3325): [<c159e6aa>] net_rx_action+0x5a/0x380
> | softirqs last enabled at (3322): [<c14b440d>] virtnet_napi_enable+0xd/0x60
> | softirqs last disabled at (3323): [<c101d63d>] call_on_stack+0xd/0x50
> |
> | other info that might help us debug this:
> | Possible unsafe locking scenario:
> |
> | CPU0
> | ----
> | lock(&syncp->seq#2);
> | <Interrupt>
> | lock(&syncp->seq#2);
> |
> | *** DEADLOCK ***
>
> This is the "up" path which is not a hotpath. There is also
> refill_work().
> It might be unwise to add the local_bh_disable() to try_fill_recv()
> because if it is used mostly in BH so that local_bh_en+dis might be a
> waste of cycles.
>
> Adding local_bh_disable() around try_fill_recv() for the non-BH call
> sites would render GFP_KERNEL pointless.
>
> Also, ptr->var++ is not an atomic operation even on 64bit CPUs. Which
> means if try_fill_recv() runs on CPU0 (via virtnet_receive()) then the
> worker might run on CPU1.
>
> Do we care or is this just stupid stats? Any suggestions?
>
> This warning appears since commit 461f03dc99cf6 ("virtio_net: Add kick stats").
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> 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 dab504ec5e502..d782160cfa882 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1206,9 +1206,11 @@ static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
> break;
> } while (rq->vq->num_free);
> if (virtqueue_kick_prepare(rq->vq) && virtqueue_notify(rq->vq)) {
> + local_bh_disable();
> u64_stats_update_begin(&rq->stats.syncp);
> rq->stats.kicks++;
> u64_stats_update_end(&rq->stats.syncp);
> + local_bh_enable();
> }
>
> return !oom;
Since there already is u64_stats_update_begin_irqsave inline, why not introduce
u64_stats_update_begin_bh which encapsulates the local_bh_disable
^ permalink raw reply
* Re: [RFC] virtio_net: add local_bh_disable() around u64_stats_update_begin
From: Stephen Hemminger @ 2018-10-16 18:01 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Michael S. Tsirkin, netdev, virtualization, tglx, David S. Miller
In-Reply-To: <20181016165545.guksrl23ulcudxrk@linutronix.de>
On Tue, 16 Oct 2018 18:55:45 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> Also, ptr->var++ is not an atomic operation even on 64bit CPUs. Which
> means if try_fill_recv() runs on CPU0 (via virtnet_receive()) then the
> worker might run on CPU1.
On modern CPU's increment of native types is atomic but not locked.
u64_stats_update_begin is a no-op on UP and also if BIT_PER_LONG != 32
^ permalink raw reply
* Re: [RFC] virtio_net: add local_bh_disable() around u64_stats_update_begin
From: Sebastian Andrzej Siewior @ 2018-10-16 18:21 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Michael S. Tsirkin, netdev, virtualization, tglx, David S. Miller
In-Reply-To: <20181016105930.55e8fe5d@xeon-e3>
On 2018-10-16 10:59:30 [-0700], Stephen Hemminger wrote:
> Since there already is u64_stats_update_begin_irqsave inline, why not introduce
> u64_stats_update_begin_bh which encapsulates the local_bh_disable
CPU0 CPU1
refill_work() virtnet_receive()
try_fill_recv() try_fill_recv()
u64_stats_update_begin_bh() u64_stats_update_begin_bh()
both CPUs may operate on the `rq'.
Sebastian
^ permalink raw reply
* Re: [RFC] virtio_net: add local_bh_disable() around u64_stats_update_begin
From: Sebastian Andrzej Siewior @ 2018-10-16 18:42 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Michael S. Tsirkin, netdev, virtualization, tglx, David S. Miller
In-Reply-To: <20181016110114.73e2b248@xeon-e3>
On 2018-10-16 11:01:14 [-0700], Stephen Hemminger wrote:
> On Tue, 16 Oct 2018 18:55:45 +0200
> Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
>
> > Also, ptr->var++ is not an atomic operation even on 64bit CPUs. Which
> > means if try_fill_recv() runs on CPU0 (via virtnet_receive()) then the
> > worker might run on CPU1.
>
> On modern CPU's increment of native types is atomic but not locked.
> u64_stats_update_begin is a no-op on UP and also if BIT_PER_LONG != 32
On ARM64 you have load, inc, store. So if two CPUs increment the counter
simultaneously we might lose one increment. That is why I asked if we
care or not.
Sebastian
^ permalink raw reply
* Re: [PATCH v3 0/7] Add virtio-iommu driver
From: Jean-Philippe Brucker @ 2018-10-16 18:44 UTC (permalink / raw)
To: Auger Eric, iommu@lists.linux-foundation.org,
virtualization@lists.linux-foundation.org,
devicetree@vger.kernel.org
Cc: Mark Rutland, peter.maydell@linaro.org, Lorenzo Pieralisi,
tnowicki@caviumnetworks.com, mst@redhat.com, Marc Zyngier,
linux-pci@vger.kernel.org, Will Deacon,
kvmarm@lists.cs.columbia.edu, robh+dt@kernel.org, Robin Murphy,
joro@8bytes.org
In-Reply-To: <7874471b-3711-ca44-d220-5e756ac7db8c@redhat.com>
On 16/10/2018 10:25, Auger Eric wrote:
> Hi Jean,
>
> On 10/12/18 4:59 PM, Jean-Philippe Brucker wrote:
>> Implement the virtio-iommu driver, following specification v0.8 [1].
>> Changes since v2 [2]:
>>
>> * Patches 2-4 allow virtio-iommu to use the PCI transport, since QEMU
>> would like to phase out the MMIO transport. This produces a complex
>> topology where the programming interface of the IOMMU could appear
>> lower than the endpoints that it translates. It's not unheard of (e.g.
>> AMD IOMMU), and the guest easily copes with this.
>>
>> The "Firmware description" section of the specification has been
>> updated with all combinations of PCI, MMIO and DT, ACPI.
>
> I have a question wrt the FW specification. The IOMMU consumes 1 slot in
> the PCI domain and one needs to leave a RID hole in the iommu-map. It
> is not obvious to me that this RID always is predictable given the pcie
> enumeration mechanism. Generally we have a coarse grain mapping of RID
> onto iommu phandles/STREAMIDs. Here, if I understand correctly we need
> to precisely identify the RID granted to the iommu. On QEMU this may
> depend on the instantiation order of the virtio-pci device right?
Yes, although it should all happen before you boot the guest, since
there is no hotplugging an IOMMU. Could you reserve a PCI slot upfront
and use it for virtio-iommu later? Or generate the iommu-map at the same
time as generating the child node of the PCI RC?
> So
> this does not look trivial to build this info. Isn't it possible to do
> this exclusion at kernel level instead?
So in theory VIRTIO_F_IOMMU_PLATFORM already does that:
VIRTIO_F_IOMMU_PLATFORM(33)
This feature indicates that the device is behind an IOMMU that
translates bus addresses from the device into physical addresses in
memory. If this feature bit is set to 0, then the device emits
physical addresses which are not translated further, even though an
IOMMU may be present.
For better or for worse, the guest has to implement it. If this feature
bit is unset for virtio-iommu, it does DMA on the physical address
space, regardless of what the static topology description says.
In practice it doesn't quite work. If your iommu-map describes the IOMMU
as translating itself, Linux' OF code will wait for the IOMMU to be
probed before probing the IOMMU. Working around this with hacks is
possible, but I don't want to introduce more questionable code to OF and
device tree bindings if there is any other way.
Thanks,
Jean
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] virtio_net: add local_bh_disable() around u64_stats_update_begin
From: Stephen Hemminger @ 2018-10-16 18:44 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Michael S. Tsirkin, netdev, virtualization, tglx, David S. Miller
In-Reply-To: <20181016184206.coukhtgmlr32hyl7@linutronix.de>
On Tue, 16 Oct 2018 20:42:07 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> On 2018-10-16 11:01:14 [-0700], Stephen Hemminger wrote:
> > On Tue, 16 Oct 2018 18:55:45 +0200
> > Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> >
> > > Also, ptr->var++ is not an atomic operation even on 64bit CPUs. Which
> > > means if try_fill_recv() runs on CPU0 (via virtnet_receive()) then the
> > > worker might run on CPU1.
> >
> > On modern CPU's increment of native types is atomic but not locked.
> > u64_stats_update_begin is a no-op on UP and also if BIT_PER_LONG != 32
>
> On ARM64 you have load, inc, store. So if two CPUs increment the counter
> simultaneously we might lose one increment. That is why I asked if we
> care or not.
>
> Sebastian
The point is that kicks is just a counter, not important as part of the
device operation. The point of the u64_stats is to avoid problems with
high/low 32 bit wrap on increment. So this is ok on ARM64.
^ permalink raw reply
* Re: [PATCH v3 0/7] Add virtio-iommu driver
From: Auger Eric @ 2018-10-16 20:31 UTC (permalink / raw)
To: Jean-Philippe Brucker, iommu@lists.linux-foundation.org,
virtualization@lists.linux-foundation.org,
devicetree@vger.kernel.org
Cc: Mark Rutland, peter.maydell@linaro.org, Lorenzo Pieralisi,
tnowicki@caviumnetworks.com, mst@redhat.com, Marc Zyngier,
linux-pci@vger.kernel.org, Will Deacon,
kvmarm@lists.cs.columbia.edu, robh+dt@kernel.org, Robin Murphy,
joro@8bytes.org
In-Reply-To: <b2f929c3-b0df-acdc-c5e8-0d7ddf7ed964@arm.com>
Hi Jean,
On 10/16/18 8:44 PM, Jean-Philippe Brucker wrote:
> On 16/10/2018 10:25, Auger Eric wrote:
>> Hi Jean,
>>
>> On 10/12/18 4:59 PM, Jean-Philippe Brucker wrote:
>>> Implement the virtio-iommu driver, following specification v0.8 [1].
>>> Changes since v2 [2]:
>>>
>>> * Patches 2-4 allow virtio-iommu to use the PCI transport, since QEMU
>>> would like to phase out the MMIO transport. This produces a complex
>>> topology where the programming interface of the IOMMU could appear
>>> lower than the endpoints that it translates. It's not unheard of (e.g.
>>> AMD IOMMU), and the guest easily copes with this.
>>>
>>> The "Firmware description" section of the specification has been
>>> updated with all combinations of PCI, MMIO and DT, ACPI.
>>
>> I have a question wrt the FW specification. The IOMMU consumes 1 slot in
>> the PCI domain and one needs to leave a RID hole in the iommu-map. It
>> is not obvious to me that this RID always is predictable given the pcie
>> enumeration mechanism. Generally we have a coarse grain mapping of RID
>> onto iommu phandles/STREAMIDs. Here, if I understand correctly we need
>> to precisely identify the RID granted to the iommu. On QEMU this may
>> depend on the instantiation order of the virtio-pci device right?
>
> Yes, although it should all happen before you boot the guest, since
> there is no hotplugging an IOMMU. Could you reserve a PCI slot upfront
> and use it for virtio-iommu later? Or generate the iommu-map at the same
> time as generating the child node of the PCI RC?
Even when cold-plugging the PCIe devices through qemu CLI, this depends
on the order of the pcie devices in the list I guess. I need to further
experiment.
>
>> So
>> this does not look trivial to build this info. Isn't it possible to do
>> this exclusion at kernel level instead?
>
> So in theory VIRTIO_F_IOMMU_PLATFORM already does that:
>
> VIRTIO_F_IOMMU_PLATFORM(33)
> This feature indicates that the device is behind an IOMMU that
> translates bus addresses from the device into physical addresses in
> memory. If this feature bit is set to 0, then the device emits
> physical addresses which are not translated further, even though an
> IOMMU may be present.
This tells the driver to use the dma api, right? Effectively this
explicitly says whether the device is supposed to be upfront an IOMMU.
>
> For better or for worse, the guest has to implement it. If this feature
> bit is unset for virtio-iommu, it does DMA on the physical address
> space, regardless of what the static topology description says.
>
> In practice it doesn't quite work. If your iommu-map describes the IOMMU
> as translating itself, Linux' OF code will wait for the IOMMU to be
> probed before probing the IOMMU. Working around this with hacks is
> possible, but I don't want to introduce more questionable code to OF and
> device tree bindings if there is any other way.
Hum ok. I cannot really comment on this.
I just wanted to raise this concern about RID identfication.
Thanks
Eric
>
> Thanks,
> Jean
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] virtio_net: add local_bh_disable() around u64_stats_update_begin
From: Toshiaki Makita @ 2018-10-17 1:13 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, netdev, virtualization
Cc: tglx, David S. Miller, Michael S. Tsirkin
In-Reply-To: <20181016165545.guksrl23ulcudxrk@linutronix.de>
On 2018/10/17 1:55, Sebastian Andrzej Siewior wrote:
> on 32bit, lockdep notices:
> | ================================
> | WARNING: inconsistent lock state
> | 4.19.0-rc8+ #9 Tainted: G W
> | --------------------------------
> | inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
> | ip/1106 [HC0[0]:SC1[1]:HE1:SE0] takes:
> | (ptrval) (&syncp->seq#2){+.?.}, at: net_rx_action+0xc8/0x380
> | {SOFTIRQ-ON-W} state was registered at:
> | lock_acquire+0x7e/0x170
> | try_fill_recv+0x5fa/0x700
> | virtnet_open+0xe0/0x180
> | __dev_open+0xae/0x130
> | __dev_change_flags+0x17f/0x200
> | dev_change_flags+0x23/0x60
> | do_setlink+0x2bb/0xa20
> | rtnl_newlink+0x523/0x830
> | rtnetlink_rcv_msg+0x14b/0x470
> | netlink_rcv_skb+0x6e/0xf0
> | rtnetlink_rcv+0xd/0x10
> | netlink_unicast+0x16e/0x1f0
> | netlink_sendmsg+0x1af/0x3a0
> | ___sys_sendmsg+0x20f/0x240
> | __sys_sendmsg+0x39/0x80
> | sys_socketcall+0x13a/0x2a0
> | do_int80_syscall_32+0x50/0x180
> | restore_all+0x0/0xb2
> | irq event stamp: 3326
> | hardirqs last enabled at (3326): [<c159e6d0>] net_rx_action+0x80/0x380
> | hardirqs last disabled at (3325): [<c159e6aa>] net_rx_action+0x5a/0x380
> | softirqs last enabled at (3322): [<c14b440d>] virtnet_napi_enable+0xd/0x60
> | softirqs last disabled at (3323): [<c101d63d>] call_on_stack+0xd/0x50
> |
> | other info that might help us debug this:
> | Possible unsafe locking scenario:
> |
> | CPU0
> | ----
> | lock(&syncp->seq#2);
> | <Interrupt>
> | lock(&syncp->seq#2);
> |
> | *** DEADLOCK ***
IIUC try_fill_recv is called only when NAPI is disabled from process
context, so there should be no point to race with virtnet_receive which
is called from NAPI handler.
I'm not sure what condition triggered this warning.
Toshiaki Makita
>
> This is the "up" path which is not a hotpath. There is also
> refill_work().
> It might be unwise to add the local_bh_disable() to try_fill_recv()
> because if it is used mostly in BH so that local_bh_en+dis might be a
> waste of cycles.
>
> Adding local_bh_disable() around try_fill_recv() for the non-BH call
> sites would render GFP_KERNEL pointless.
>
> Also, ptr->var++ is not an atomic operation even on 64bit CPUs. Which
> means if try_fill_recv() runs on CPU0 (via virtnet_receive()) then the
> worker might run on CPU1.
>
> Do we care or is this just stupid stats? Any suggestions?
>
> This warning appears since commit 461f03dc99cf6 ("virtio_net: Add kick stats").
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> 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 dab504ec5e502..d782160cfa882 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1206,9 +1206,11 @@ static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
> break;
> } while (rq->vq->num_free);
> if (virtqueue_kick_prepare(rq->vq) && virtqueue_notify(rq->vq)) {
> + local_bh_disable();
> u64_stats_update_begin(&rq->stats.syncp);
> rq->stats.kicks++;
> u64_stats_update_end(&rq->stats.syncp);
> + local_bh_enable();
> }
>
> return !oom;
>
^ permalink raw reply
* Re: [PATCH] virtio_net: enable tx after resuming from suspend
From: Jason Wang @ 2018-10-17 6:18 UTC (permalink / raw)
To: ake
Cc: netdev, virtualization, David S. Miller, linux-kernel,
Michael S. Tsirkin
In-Reply-To: <0e8b0747-cc9f-8863-2ee1-2b032d6c0fae@igel.co.jp>
On 2018/10/16 下午6:15, ake wrote:
>
> On 2018年10月16日 17:53, Jason Wang wrote:
>> On 2018/10/15 下午6:08, ake wrote:
>>> On 2018年10月12日 18:18, ake wrote:
>>>> On 2018年10月12日 17:23, Jason Wang wrote:
>>>>> On 2018年10月12日 12:30, ake wrote:
>>>>>> On 2018年10月11日 22:06, Jason Wang wrote:
>>>>>>> On 2018年10月11日 18:22, ake wrote:
>>>>>>>> On 2018年10月11日 18:44, Jason Wang wrote:
>>>>>>>>> On 2018年10月11日 15:51, Ake Koomsin wrote:
>>>>>>>>>> commit 713a98d90c5e ("virtio-net: serialize tx routine during
>>>>>>>>>> reset")
>>>>>>>>>> disabled the virtio tx before going to suspend to avoid a use
>>>>>>>>>> after
>>>>>>>>>> free.
>>>>>>>>>> However, after resuming, it causes the virtio_net device to
>>>>>>>>>> lose its
>>>>>>>>>> network connectivity.
>>>>>>>>>>
>>>>>>>>>> To solve the issue, we need to enable tx after resuming.
>>>>>>>>>>
>>>>>>>>>> Fixes commit 713a98d90c5e ("virtio-net: serialize tx routine
>>>>>>>>>> during
>>>>>>>>>> reset")
>>>>>>>>>> Signed-off-by: Ake Koomsin <ake@igel.co.jp>
>>>>>>>>>> ---
>>>>>>>>>> drivers/net/virtio_net.c | 1 +
>>>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>>>>>>>>> index dab504ec5e50..3453d80f5f81 100644
>>>>>>>>>> --- a/drivers/net/virtio_net.c
>>>>>>>>>> +++ b/drivers/net/virtio_net.c
>>>>>>>>>> @@ -2256,6 +2256,7 @@ static int virtnet_restore_up(struct
>>>>>>>>>> virtio_device *vdev)
>>>>>>>>>> }
>>>>>>>>>> netif_device_attach(vi->dev);
>>>>>>>>>> + netif_start_queue(vi->dev);
>>>>>>>>> I believe this is duplicated with netif_tx_wake_all_queues() in
>>>>>>>>> netif_device_attach() above?
>>>>>>>> Thank you for your review.
>>>>>>>>
>>>>>>>> If both netif_tx_wake_all_queues() and netif_start_queue() result in
>>>>>>>> clearing __QUEUE_STATE_DRV_XOFF, then is it possible that some
>>>>>>>> conditions in netif_device_attach() is not satisfied?
>>>>>>> Yes, maybe. One case I can see now is when the device is down, in
>>>>>>> this
>>>>>>> case netif_device_attach() won't try to wakeup the queue.
>>>>>>>
>>>>>>>> Without
>>>>>>>> netif_start_queue(), the virtio_net device does not resume properly
>>>>>>>> after waking up.
>>>>>>> How do you trigger the issue? Just do suspend/resume?
>>>>>> Yes, simply suspend and resume.
>>>>>>
>>>>>> Here is how I trigger the issue:
>>>>>>
>>>>>> 1) Start the Virtual Machine Manager GUI program.
>>>>>> 2) Create a guest Linux OS. Make sure that the guest OS kernel is
>>>>>> >= 4.12. Make sure that it uses virtio_net as its network device.
>>>>>> In addition, make sure that the video adapter is VGA. Otherwise,
>>>>>> waking up with the virtual power button does not work.
>>>>>> 3) After installing the guest OS, log in, and test the network
>>>>>> connectivity by ping the host machine.
>>>>>> 4) Suspend. After this, the screen is blank.
>>>>>> 5) Resume by hitting the virtual power button. The login screen
>>>>>> appears again.
>>>>>> 6) Log in again. The guest loses its network connection.
>>>>>>
>>>>>> In my test:
>>>>>> Guest: Ubuntu 16.04/18.04 with kernel 4.15.0-36-generic
>>>>>> Host: Ubuntu 16.04 with kernel 4.15.0-36-generic/4.4.0-137-generic
>>>>> I can not reproduce this issue if virtio-net interface is up in guest
>>>>> before the suspend. I'm using net-next.git and qemu master. But I do
>>>>> reproduce when virtio-net interface is down in guest before suspend,
>>>>> after resume, even if I make it up, the network is still lost.
>>>>>
>>>>> I think the interface is up in your case, but please confirm this.
>>>> If you mean the interface state before I hit the suspend button,
>>>> the answer is yes. The interface is up before I suspend the guest
>>>> machine.
>>>>
>>>> Note that my current QEMU version is QEMU emulator version 2.5.0
>>>> (Debian 1:2.5+dfsg-5ubuntu10.32).
>>>>
>>>> I will try with net-next.git and qemu master later and see if I can
>>>> reproduce the issue.
>>> Update. I tried with net-next and qemu master. Interestingly, the result
>>> is different from yours. The network is lost even if the virtio_net
>>> interface is up before suspending.
>>>
>>> Host: Ubuntu 16.04 with net-next kernel (default configuration)
>>> Guest: Ubuntu 18.04 with net-next kernel (default configuration)
>>> Qemu: master
>>> Qemu command:
>>> qemu-system-x86_64 -cpu host -m 2048 -enable-kvm \
>>> -bios /usr/share/OVMF/OVMF_CODE.fd \
>>> -drive file=/var/lib/libvirt/images/virtio_test.qcow2,if=virtio \
>>> -netdev user,id=hostnet0 \
>>> -device virtio-net-pci,netdev=hostnet0 \
>>> -device VGA,id=video0,vgamem_mb=16 \
>>> -global PIIX4_PM.disable_s3=1 \
>>> -global PIIX4_PM.disable_s4=1 -monitor stdio
>>
>> Interesting, just notice you're using userspace network. To isolate the
>> issue, can you retry with e.g tap or e1000 to make sure it's not a fault
>> of slirp or virito-net?
> I will try.
>
>> Thanks
>>
> There is another thing that I want to discuss. I notice that
> netif_device_detach() should result in setting __QUEUE_STATE_DRV_XOFF if
> the network interface is running. By calling netif_tx_disable() after
> netif_device_detach(), isn't it redundant in case of the network
> interface is running? If the goal is to serialize tx routine, would
> netif_tx_lock() and net_tx_unlock() are more appropriate? Like this:
>
> netif_tx_lock(vi->dev);
> netif_device_detach(vi->dev);
> netif_tx_unlock(vi->dev);
>
> Currently, netif_tx_disable() seems to disturb the symmetry of
> netif_device_detach() and netif_device_attach(). That is the reason
> why you can reproduce the problem when the interface is down before
> suspending.
Yes I agree.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] virtio_net: add local_bh_disable() around u64_stats_update_begin
From: Jason Wang @ 2018-10-17 6:48 UTC (permalink / raw)
To: Toshiaki Makita, Sebastian Andrzej Siewior, netdev,
virtualization
Cc: tglx, David S. Miller, Michael S. Tsirkin
In-Reply-To: <68582c17-27bf-43f4-cbe5-96ec712a704c@lab.ntt.co.jp>
On 2018/10/17 上午9:13, Toshiaki Makita wrote:
> On 2018/10/17 1:55, Sebastian Andrzej Siewior wrote:
>> on 32bit, lockdep notices:
>> | ================================
>> | WARNING: inconsistent lock state
>> | 4.19.0-rc8+ #9 Tainted: G W
>> | --------------------------------
>> | inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
>> | ip/1106 [HC0[0]:SC1[1]:HE1:SE0] takes:
>> | (ptrval) (&syncp->seq#2){+.?.}, at: net_rx_action+0xc8/0x380
>> | {SOFTIRQ-ON-W} state was registered at:
>> | lock_acquire+0x7e/0x170
>> | try_fill_recv+0x5fa/0x700
>> | virtnet_open+0xe0/0x180
>> | __dev_open+0xae/0x130
>> | __dev_change_flags+0x17f/0x200
>> | dev_change_flags+0x23/0x60
>> | do_setlink+0x2bb/0xa20
>> | rtnl_newlink+0x523/0x830
>> | rtnetlink_rcv_msg+0x14b/0x470
>> | netlink_rcv_skb+0x6e/0xf0
>> | rtnetlink_rcv+0xd/0x10
>> | netlink_unicast+0x16e/0x1f0
>> | netlink_sendmsg+0x1af/0x3a0
>> | ___sys_sendmsg+0x20f/0x240
>> | __sys_sendmsg+0x39/0x80
>> | sys_socketcall+0x13a/0x2a0
>> | do_int80_syscall_32+0x50/0x180
>> | restore_all+0x0/0xb2
>> | irq event stamp: 3326
>> | hardirqs last enabled at (3326): [<c159e6d0>] net_rx_action+0x80/0x380
>> | hardirqs last disabled at (3325): [<c159e6aa>] net_rx_action+0x5a/0x380
>> | softirqs last enabled at (3322): [<c14b440d>] virtnet_napi_enable+0xd/0x60
>> | softirqs last disabled at (3323): [<c101d63d>] call_on_stack+0xd/0x50
>> |
>> | other info that might help us debug this:
>> | Possible unsafe locking scenario:
>> |
>> | CPU0
>> | ----
>> | lock(&syncp->seq#2);
>> | <Interrupt>
>> | lock(&syncp->seq#2);
>> |
>> | *** DEADLOCK ***
> IIUC try_fill_recv is called only when NAPI is disabled from process
> context, so there should be no point to race with virtnet_receive which
> is called from NAPI handler.
>
> I'm not sure what condition triggered this warning.
>
>
> Toshiaki Makita
Or maybe NAPI is enabled unexpectedly somewhere?
Btw, the schedule_delayed_work() in virtnet_open() is also suspicious,
if the work is executed before virtnet_napi_enable(), there will be a
deadloop for napi_disable().
Thanks
>
>
>> This is the "up" path which is not a hotpath. There is also
>> refill_work().
>> It might be unwise to add the local_bh_disable() to try_fill_recv()
>> because if it is used mostly in BH so that local_bh_en+dis might be a
>> waste of cycles.
>>
>> Adding local_bh_disable() around try_fill_recv() for the non-BH call
>> sites would render GFP_KERNEL pointless.
>>
>> Also, ptr->var++ is not an atomic operation even on 64bit CPUs. Which
>> means if try_fill_recv() runs on CPU0 (via virtnet_receive()) then the
>> worker might run on CPU1.
>>
>> Do we care or is this just stupid stats? Any suggestions?
>>
>> This warning appears since commit 461f03dc99cf6 ("virtio_net: Add kick stats").
>>
>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> ---
>> 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 dab504ec5e502..d782160cfa882 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1206,9 +1206,11 @@ static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
>> break;
>> } while (rq->vq->num_free);
>> if (virtqueue_kick_prepare(rq->vq) && virtqueue_notify(rq->vq)) {
>> + local_bh_disable();
>> u64_stats_update_begin(&rq->stats.syncp);
>> rq->stats.kicks++;
>> u64_stats_update_end(&rq->stats.syncp);
>> + local_bh_enable();
>> }
>>
>> return !oom;
>>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next V2 6/8] vhost: packed ring support
From: Jason Wang @ 2018-10-17 6:54 UTC (permalink / raw)
To: Maxime Coquelin, Michael S. Tsirkin, Tiwei Bie
Cc: kvm, netdev, linux-kernel, virtualization, wexu
In-Reply-To: <1df62bd3-3cc9-d04a-2939-4570d37faa68@redhat.com>
On 2018/10/16 下午9:58, Maxime Coquelin wrote:
>
> On 10/15/2018 04:22 AM, Jason Wang wrote:
>>
>>
>> On 2018年10月13日 01:23, Michael S. Tsirkin wrote:
>>> On Fri, Oct 12, 2018 at 10:32:44PM +0800, Tiwei Bie wrote:
>>>> On Mon, Jul 16, 2018 at 11:28:09AM +0800, Jason Wang wrote:
>>>> [...]
>>>>> @@ -1367,10 +1397,48 @@ long vhost_vring_ioctl(struct vhost_dev
>>>>> *d, unsigned int ioctl, void __user *arg
>>>>> vq->last_avail_idx = s.num;
>>>>> /* Forget the cached index value. */
>>>>> vq->avail_idx = vq->last_avail_idx;
>>>>> + if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED)) {
>>>>> + vq->last_avail_wrap_counter = wrap_counter;
>>>>> + vq->avail_wrap_counter = vq->last_avail_wrap_counter;
>>>>> + }
>>>>> break;
>>>>> case VHOST_GET_VRING_BASE:
>>>>> s.index = idx;
>>>>> s.num = vq->last_avail_idx;
>>>>> + if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
>>>>> + s.num |= vq->last_avail_wrap_counter << 31;
>>>>> + if (copy_to_user(argp, &s, sizeof(s)))
>>>>> + r = -EFAULT;
>>>>> + break;
>>>>> + case VHOST_SET_VRING_USED_BASE:
>>>>> + /* Moving base with an active backend?
>>>>> + * You don't want to do that.
>>>>> + */
>>>>> + if (vq->private_data) {
>>>>> + r = -EBUSY;
>>>>> + break;
>>>>> + }
>>>>> + if (copy_from_user(&s, argp, sizeof(s))) {
>>>>> + r = -EFAULT;
>>>>> + break;
>>>>> + }
>>>>> + if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED)) {
>>>>> + wrap_counter = s.num >> 31;
>>>>> + s.num &= ~(1 << 31);
>>>>> + }
>>>>> + if (s.num > 0xffff) {
>>>>> + r = -EINVAL;
>>>>> + break;
>>>>> + }
>>>> Do we want to put wrap_counter at bit 15?
>>> I think I second that - seems to be consistent with
>>> e.g. event suppression structure and the proposed
>>> extension to driver notifications.
>>
>> Ok, I assumes packed virtqueue support 64K but looks not. I can
>> change it to bit 15 and GET_VRING_BASE need to be changed as well.
>>
>>>
>>>
>>>> If put wrap_counter at bit 31, the check (s.num > 0xffff)
>>>> won't be able to catch the illegal index 0x8000~0xffff for
>>>> packed ring.
>>>>
>>
>> Do we need to clarify this in the spec?
>>
>>>>> + vq->last_used_idx = s.num;
>>>>> + if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
>>>>> + vq->last_used_wrap_counter = wrap_counter;
>>>>> + break;
>>>>> + case VHOST_GET_VRING_USED_BASE:
>>>> Do we need the new VHOST_GET_VRING_USED_BASE and
>>>> VHOST_SET_VRING_USED_BASE ops?
>>>>
>>>> We are going to merge below series in DPDK:
>>>>
>>>> http://patches.dpdk.org/patch/45874/
>>>>
>>>> We may need to reach an agreement first.
>>
>> If we agree that 64K virtqueue won't be supported, I'm ok with either.
>
> I'm fine to put wrap_counter at bit 15.
> I will post a new version of the DPDK series soon.
>
>> Btw the code assumes used_wrap_counter is equal to avail_wrap_counter
>> which looks wrong?
>
> For split ring, we used to set the last_used_idx to the same value as
> last_avail_idx as VHOST_USER_GET_VRING_BASE cannot be called while the
> ring is being processed, so their value is always the same at the time
> the request is handled.
I may miss something, but it looks to me we should sync last_used_idx
from used_idx.
Thanks
>
>
> I kept the same behavior for packed ring, and so the wrap counter have
> to be the same.
>
> Regards,
> Maxime
>
>> Thanks
>>
>>>>
>>>>> + s.index = idx;
>>>>> + s.num = vq->last_used_idx;
>>>>> + if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
>>>>> + s.num |= vq->last_used_wrap_counter << 31;
>>>>> if (copy_to_user(argp, &s, sizeof s))
>>>>> r = -EFAULT;
>>>>> break;
>>>> [...]
>>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v2] virtio_net: avoid using netif_tx_disable() for serializing tx routine
From: Jason Wang @ 2018-10-17 9:02 UTC (permalink / raw)
To: Ake Koomsin
Cc: netdev, virtualization, David S. Miller, linux-kernel,
Michael S. Tsirkin
In-Reply-To: <20181017075918.7043-1-ake@igel.co.jp>
On 2018/10/17 下午3:59, Ake Koomsin wrote:
> Commit 713a98d90c5e ("virtio-net: serialize tx routine during reset")
> introduces netif_tx_disable() after netif_device_detach() in order to
> avoid use-after-free of tx queues. However, there are two issues.
>
> 1) Its operation is redundant with netif_device_detach() if the interface
> is running.
> 2) In case of the interface is not running before suspending and
> resuming, the tx does not get resumed by netif_device_attach().
> This results in losing network connectivity.
>
> It is better to use netif_tx_lock()/netif_tx_unlock() instead for
> serializing tx routine during reset. This also preserves the symmetry
> of netif_device_detach() and netif_device_attach().
>
> Fixes commit 713a98d90c5e ("virtio-net: serialize tx routine during reset")
> Signed-off-by: Ake Koomsin <ake@igel.co.jp>
> ---
> drivers/net/virtio_net.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 3f5aa59c37b7..41ccf9c994a4 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2267,8 +2267,9 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
> /* Make sure no work handler is accessing the device */
> flush_work(&vi->config_work);
>
> + netif_tx_lock(vi->dev);
> netif_device_detach(vi->dev);
> - netif_tx_disable(vi->dev);
> + netif_tx_unlock(vi->dev);
Sorry for not finding this earlier. I think we should use
netif_tx_lock_bh() to prevent start_xmit() to run under bh.
Thanks
> cancel_delayed_work_sync(&vi->refill);
>
> if (netif_running(vi->dev)) {
> @@ -2304,7 +2305,9 @@ static int virtnet_restore_up(struct virtio_device *vdev)
> }
> }
>
> + netif_tx_lock(vi->dev);
> netif_device_attach(vi->dev);
> + netif_tx_unlock(vi->dev);
> return err;
> }
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] VSOCK: The performance problem of vhost_vsock.
From: jiangyiwen @ 2018-10-17 9:27 UTC (permalink / raw)
To: Jason Wang, stefanha; +Cc: netdev, kvm, virtualization
In-Reply-To: <5BC42FD8.2070104@huawei.com>
On 2018/10/15 14:12, jiangyiwen wrote:
> On 2018/10/15 10:33, Jason Wang wrote:
>>
>>
>> On 2018年10月15日 09:43, jiangyiwen wrote:
>>> Hi Stefan & All:
>>>
>>> Now I find vhost-vsock has two performance problems even if it
>>> is not designed for performance.
>>>
>>> First, I think vhost-vsock should faster than vhost-net because it
>>> is no TCP/IP stack, but the real test result vhost-net is 5~10
>>> times than vhost-vsock, currently I am looking for the reason.
>>
>> TCP/IP is not a must for vhost-net.
>>
>> How do you test and compare the performance?
>>
>> Thanks
>>
>
> I test the performance used my test tool, like follows:
>
> Server Client
> socket()
> bind()
> listen()
>
> socket(AF_VSOCK) or socket(AF_INET)
> Accept() <-------------->connect()
> *======Start Record Time======*
> Call syscall sendfile()
> Recv()
> Send end
> Receive end
> Send(file_size)
> Recv(file_size)
> *======End Record Time======*
>
> The test result, vhost-vsock is about 500MB/s, and vhost-net is about 2500MB/s.
>
> By the way, vhost-net use single queue.
>
> Thanks.
>
>>> Second, vhost-vsock only supports two vqs(tx and rx), that means
>>> if multiple sockets in the guest will use the same vq to transmit
>>> the message and get the response. So if there are multiple applications
>>> in the guest, we should support "Multiqueue" feature for Virtio-vsock.
>>>
>>> Stefan, have you encountered these problems?
>>>
>>> Thanks,
>>> Yiwen.
>>>
>>
>>
>> .
>>
>
>
Hi Jason and Stefan,
Maybe I find the reason of bad performance.
I found pkt_len is limited to VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE(4K),
it will cause the bandwidth is limited to 500~600MB/s. And once I
increase to 64k, it can improve about 3 times(~1500MB/s).
By the way, I send to 64K in application once, and I don't use
sg_init_one and rewrite function to packet sg list because pkt_len
include multiple pages.
Thanks,
Yiwen.
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] VSOCK: The performance problem of vhost_vsock.
From: Jason Wang @ 2018-10-17 9:39 UTC (permalink / raw)
To: jiangyiwen, stefanha; +Cc: netdev, kvm, virtualization
In-Reply-To: <5BC70069.4000600@huawei.com>
On 2018/10/17 下午5:27, jiangyiwen wrote:
> On 2018/10/15 14:12, jiangyiwen wrote:
>> On 2018/10/15 10:33, Jason Wang wrote:
>>>
>>> On 2018年10月15日 09:43, jiangyiwen wrote:
>>>> Hi Stefan & All:
>>>>
>>>> Now I find vhost-vsock has two performance problems even if it
>>>> is not designed for performance.
>>>>
>>>> First, I think vhost-vsock should faster than vhost-net because it
>>>> is no TCP/IP stack, but the real test result vhost-net is 5~10
>>>> times than vhost-vsock, currently I am looking for the reason.
>>> TCP/IP is not a must for vhost-net.
>>>
>>> How do you test and compare the performance?
>>>
>>> Thanks
>>>
>> I test the performance used my test tool, like follows:
>>
>> Server Client
>> socket()
>> bind()
>> listen()
>>
>> socket(AF_VSOCK) or socket(AF_INET)
>> Accept() <-------------->connect()
>> *======Start Record Time======*
>> Call syscall sendfile()
>> Recv()
>> Send end
>> Receive end
>> Send(file_size)
>> Recv(file_size)
>> *======End Record Time======*
>>
>> The test result, vhost-vsock is about 500MB/s, and vhost-net is about 2500MB/s.
>>
>> By the way, vhost-net use single queue.
>>
>> Thanks.
>>
>>>> Second, vhost-vsock only supports two vqs(tx and rx), that means
>>>> if multiple sockets in the guest will use the same vq to transmit
>>>> the message and get the response. So if there are multiple applications
>>>> in the guest, we should support "Multiqueue" feature for Virtio-vsock.
>>>>
>>>> Stefan, have you encountered these problems?
>>>>
>>>> Thanks,
>>>> Yiwen.
>>>>
>>>
>>> .
>>>
>>
> Hi Jason and Stefan,
>
> Maybe I find the reason of bad performance.
>
> I found pkt_len is limited to VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE(4K),
> it will cause the bandwidth is limited to 500~600MB/s. And once I
> increase to 64k, it can improve about 3 times(~1500MB/s).
Looks like the value was chosen for a balance between rx buffer size and
performance. Allocating 64K always even for small packet is kind of
waste and stress for guest memory. Virito-net try to avoid this by
inventing the merge able rx buffer which allows big packet to be
scattered in into different buffers. We can reuse this idea or revisit
the idea of using virtio-net/vhost-net as a transport of vsock.
What interesting is the performance is still behind vhost-net.
Thanks
>
> By the way, I send to 64K in application once, and I don't use
> sg_init_one and rewrite function to packet sg list because pkt_len
> include multiple pages.
>
> Thanks,
> Yiwen.
>
>> _______________________________________________
>> Virtualization mailing list
>> Virtualization@lists.linux-foundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
>>
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] VSOCK: The performance problem of vhost_vsock.
From: Jason Wang @ 2018-10-17 9:51 UTC (permalink / raw)
To: jiangyiwen, stefanha; +Cc: netdev, kvm, virtualization
In-Reply-To: <d16d2052-bfb1-7861-e210-b53b4ea3260c@redhat.com>
On 2018/10/17 下午5:39, Jason Wang wrote:
>>>
>> Hi Jason and Stefan,
>>
>> Maybe I find the reason of bad performance.
>>
>> I found pkt_len is limited to VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE(4K),
>> it will cause the bandwidth is limited to 500~600MB/s. And once I
>> increase to 64k, it can improve about 3 times(~1500MB/s).
>
>
> Looks like the value was chosen for a balance between rx buffer size
> and performance. Allocating 64K always even for small packet is kind
> of waste and stress for guest memory. Virito-net try to avoid this by
> inventing the merge able rx buffer which allows big packet to be
> scattered in into different buffers. We can reuse this idea or revisit
> the idea of using virtio-net/vhost-net as a transport of vsock.
>
> What interesting is the performance is still behind vhost-net.
>
> Thanks
>
>>
>> By the way, I send to 64K in application once, and I don't use
>> sg_init_one and rewrite function to packet sg list because pkt_len
>> include multiple pages.
>>
>> Thanks,
>> Yiwen.
Btw, if you're using vsock for transferring large files, maybe it's more
efficient to implement sendpage() for vsock to allow sendfile()/splice()
work.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] VSOCK: The performance problem of vhost_vsock.
From: jiangyiwen @ 2018-10-17 11:32 UTC (permalink / raw)
To: Jason Wang, stefanha; +Cc: netdev, kvm, virtualization
In-Reply-To: <d16d2052-bfb1-7861-e210-b53b4ea3260c@redhat.com>
On 2018/10/17 17:39, Jason Wang wrote:
>
> On 2018/10/17 下午5:27, jiangyiwen wrote:
>> On 2018/10/15 14:12, jiangyiwen wrote:
>>> On 2018/10/15 10:33, Jason Wang wrote:
>>>>
>>>> On 2018年10月15日 09:43, jiangyiwen wrote:
>>>>> Hi Stefan & All:
>>>>>
>>>>> Now I find vhost-vsock has two performance problems even if it
>>>>> is not designed for performance.
>>>>>
>>>>> First, I think vhost-vsock should faster than vhost-net because it
>>>>> is no TCP/IP stack, but the real test result vhost-net is 5~10
>>>>> times than vhost-vsock, currently I am looking for the reason.
>>>> TCP/IP is not a must for vhost-net.
>>>>
>>>> How do you test and compare the performance?
>>>>
>>>> Thanks
>>>>
>>> I test the performance used my test tool, like follows:
>>>
>>> Server Client
>>> socket()
>>> bind()
>>> listen()
>>>
>>> socket(AF_VSOCK) or socket(AF_INET)
>>> Accept() <-------------->connect()
>>> *======Start Record Time======*
>>> Call syscall sendfile()
>>> Recv()
>>> Send end
>>> Receive end
>>> Send(file_size)
>>> Recv(file_size)
>>> *======End Record Time======*
>>>
>>> The test result, vhost-vsock is about 500MB/s, and vhost-net is about 2500MB/s.
>>>
>>> By the way, vhost-net use single queue.
>>>
>>> Thanks.
>>>
>>>>> Second, vhost-vsock only supports two vqs(tx and rx), that means
>>>>> if multiple sockets in the guest will use the same vq to transmit
>>>>> the message and get the response. So if there are multiple applications
>>>>> in the guest, we should support "Multiqueue" feature for Virtio-vsock.
>>>>>
>>>>> Stefan, have you encountered these problems?
>>>>>
>>>>> Thanks,
>>>>> Yiwen.
>>>>>
>>>>
>>>> .
>>>>
>>>
>> Hi Jason and Stefan,
>>
>> Maybe I find the reason of bad performance.
>>
>> I found pkt_len is limited to VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE(4K),
>> it will cause the bandwidth is limited to 500~600MB/s. And once I
>> increase to 64k, it can improve about 3 times(~1500MB/s).
>
>
> Looks like the value was chosen for a balance between rx buffer size and performance. Allocating 64K always even for small packet is kind of waste and stress for guest memory. Virito-net try to avoid this by inventing the merge able rx buffer which allows big packet to be scattered in into different buffers. We can reuse this idea or revisit the idea of using virtio-net/vhost-net as a transport of vsock.
>
> What interesting is the performance is still behind vhost-net.
>
> Thanks
>
Actually I don't understand why pkt_len is limited to
VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE in virtio_transport_send_pkt_info(),
while I think it should used VIRTIO_VSOCK_MAX_PKT_BUF_SIZE instead.
Thanks.
>>
>> By the way, I send to 64K in application once, and I don't use
>> sg_init_one and rewrite function to packet sg list because pkt_len
>> include multiple pages.
>>
>> Thanks,
>> Yiwen.
>>
>>> _______________________________________________
>>> Virtualization mailing list
>>> Virtualization@lists.linux-foundation.org
>>> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
>>>
>>
>
> .
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] VSOCK: The performance problem of vhost_vsock.
From: jiangyiwen @ 2018-10-17 11:41 UTC (permalink / raw)
To: Jason Wang, stefanha; +Cc: netdev, kvm, virtualization
In-Reply-To: <8edff784-b311-bfb6-1bf4-1970d564279d@redhat.com>
On 2018/10/17 17:51, Jason Wang wrote:
>
> On 2018/10/17 下午5:39, Jason Wang wrote:
>>>>
>>> Hi Jason and Stefan,
>>>
>>> Maybe I find the reason of bad performance.
>>>
>>> I found pkt_len is limited to VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE(4K),
>>> it will cause the bandwidth is limited to 500~600MB/s. And once I
>>> increase to 64k, it can improve about 3 times(~1500MB/s).
>>
>>
>> Looks like the value was chosen for a balance between rx buffer size and performance. Allocating 64K always even for small packet is kind of waste and stress for guest memory. Virito-net try to avoid this by inventing the merge able rx buffer which allows big packet to be scattered in into different buffers. We can reuse this idea or revisit the idea of using virtio-net/vhost-net as a transport of vsock.
>>
>> What interesting is the performance is still behind vhost-net.
>>
>> Thanks
>>
>>>
>>> By the way, I send to 64K in application once, and I don't use
>>> sg_init_one and rewrite function to packet sg list because pkt_len
>>> include multiple pages.
>>>
>>> Thanks,
>>> Yiwen.
>
>
> Btw, if you're using vsock for transferring large files, maybe it's more efficient to implement sendpage() for vsock to allow sendfile()/splice() work.
>
> Thanks
>
I can't agree more.
why vhost_vsock is still behind vhost_net?
Because I use sendfile() to test performance at first, and then
I found vsock don't implement sendpage() and cause the bandwidth
can't be increased. So I use read() and send() to replace sendfile(),
it will increase some switch between kernel and user mode, and sendfile()
can support zero copy. I think this is main reason.
Thanks.
>
> .
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 0/7] Add virtio-iommu driver
From: Jean-Philippe Brucker @ 2018-10-17 11:54 UTC (permalink / raw)
To: Auger Eric, iommu@lists.linux-foundation.org,
virtualization@lists.linux-foundation.org,
devicetree@vger.kernel.org
Cc: Mark Rutland, peter.maydell@linaro.org, Lorenzo Pieralisi,
tnowicki@caviumnetworks.com, mst@redhat.com, Marc Zyngier,
linux-pci@vger.kernel.org, Will Deacon,
kvmarm@lists.cs.columbia.edu, robh+dt@kernel.org, Robin Murphy,
joro@8bytes.org
In-Reply-To: <c7167c51-d864-08f5-c128-f4b3257840ce@redhat.com>
On 16/10/2018 21:31, Auger Eric wrote:
> Hi Jean,
>
> On 10/16/18 8:44 PM, Jean-Philippe Brucker wrote:
>> On 16/10/2018 10:25, Auger Eric wrote:
>>> Hi Jean,
>>>
>>> On 10/12/18 4:59 PM, Jean-Philippe Brucker wrote:
>>>> Implement the virtio-iommu driver, following specification v0.8 [1].
>>>> Changes since v2 [2]:
>>>>
>>>> * Patches 2-4 allow virtio-iommu to use the PCI transport, since QEMU
>>>> would like to phase out the MMIO transport. This produces a complex
>>>> topology where the programming interface of the IOMMU could appear
>>>> lower than the endpoints that it translates. It's not unheard of (e.g.
>>>> AMD IOMMU), and the guest easily copes with this.
>>>>
>>>> The "Firmware description" section of the specification has been
>>>> updated with all combinations of PCI, MMIO and DT, ACPI.
>>>
>>> I have a question wrt the FW specification. The IOMMU consumes 1 slot in
>>> the PCI domain and one needs to leave a RID hole in the iommu-map. It
>>> is not obvious to me that this RID always is predictable given the pcie
>>> enumeration mechanism. Generally we have a coarse grain mapping of RID
>>> onto iommu phandles/STREAMIDs. Here, if I understand correctly we need
>>> to precisely identify the RID granted to the iommu. On QEMU this may
>>> depend on the instantiation order of the virtio-pci device right?
>>
>> Yes, although it should all happen before you boot the guest, since
>> there is no hotplugging an IOMMU. Could you reserve a PCI slot upfront
>> and use it for virtio-iommu later? Or generate the iommu-map at the same
>> time as generating the child node of the PCI RC?
>
> Even when cold-plugging the PCIe devices through qemu CLI, this depends
> on the order of the pcie devices in the list I guess. I need to further
> experiment.
Please let me know how it goes. I guess the problem will be the same for
building IORT tables? You're also going to need a hole in the ID
mappings of the PCI root complex node.
>>> So
>>> this does not look trivial to build this info. Isn't it possible to do
>>> this exclusion at kernel level instead?
>>
>> So in theory VIRTIO_F_IOMMU_PLATFORM already does that:
>>
>> VIRTIO_F_IOMMU_PLATFORM(33)
>> This feature indicates that the device is behind an IOMMU that
>> translates bus addresses from the device into physical addresses in
>> memory. If this feature bit is set to 0, then the device emits
>> physical addresses which are not translated further, even though an
>> IOMMU may be present.
>
> This tells the driver to use the dma api, right?
That's how Linux implements the bit, install custom DMA ops when the bit
is absent. But it doesn't work for everyone and has caused a lot of
debate (https://patchwork.ozlabs.org/cover/946708/)
> Effectively this
> explicitly says whether the device is supposed to be upfront an IOMMU.
Yes. It's quite strange if you consider hotpluggable hardware, since
those devices shouldn't get to choose whether they are managed by an
IOMMU. For the IOMMU itself, it should be fine
>> For better or for worse, the guest has to implement it. If this feature
>> bit is unset for virtio-iommu, it does DMA on the physical address
>> space, regardless of what the static topology description says.
>>
>> In practice it doesn't quite work. If your iommu-map describes the IOMMU
>> as translating itself, Linux' OF code will wait for the IOMMU to be
>> probed before probing the IOMMU. Working around this with hacks is
>> possible, but I don't want to introduce more questionable code to OF and
>> device tree bindings if there is any other way.
> Hum ok. I cannot really comment on this.
>
> I just wanted to raise this concern about RID identfication.
We can always try. Relaxing iommu-map further would be one additional
patch to Documentation/devicetree/bindings/pci/pci-iommu.txt, and one to
drivers/iommu/of-iommu.c. I'd rather make it a separate RFC.
Since we need acks from an OF maintainer and I'd also like Joerg's
approval for adding a new driver to the IOMMU tree, I think it's too
late for this iteration. I wasn't intending for this to go into 4.20,
just have something to discuss at KVM forum next week.
Thanks,
Jean
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3] virtio_net: avoid using netif_tx_disable() for serializing tx routine
From: Jason Wang @ 2018-10-17 12:30 UTC (permalink / raw)
To: Ake Koomsin
Cc: netdev, virtualization, David S. Miller, linux-kernel,
Michael S. Tsirkin
In-Reply-To: <20181017104419.13003-1-ake@igel.co.jp>
On 2018/10/17 下午6:44, Ake Koomsin wrote:
> Commit 713a98d90c5e ("virtio-net: serialize tx routine during reset")
> introduces netif_tx_disable() after netif_device_detach() in order to
> avoid use-after-free of tx queues. However, there are two issues.
>
> 1) Its operation is redundant with netif_device_detach() in case the
> interface is running.
> 2) In case of the interface is not running before suspending and
> resuming, the tx does not get resumed by netif_device_attach().
> This results in losing network connectivity.
>
> It is better to use netif_tx_lock_bh()/netif_tx_unlock_bh() instead for
> serializing tx routine during reset. This also preserves the symmetry
> of netif_device_detach() and netif_device_attach().
>
> Fixes commit 713a98d90c5e ("virtio-net: serialize tx routine during reset")
> Signed-off-by: Ake Koomsin <ake@igel.co.jp>
> ---
> drivers/net/virtio_net.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 3f5aa59c37b7..3e2c041d76ac 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2267,8 +2267,9 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
> /* Make sure no work handler is accessing the device */
> flush_work(&vi->config_work);
>
> + netif_tx_lock_bh(vi->dev);
> netif_device_detach(vi->dev);
> - netif_tx_disable(vi->dev);
> + netif_tx_unlock_bh(vi->dev);
> cancel_delayed_work_sync(&vi->refill);
>
> if (netif_running(vi->dev)) {
> @@ -2304,7 +2305,9 @@ static int virtnet_restore_up(struct virtio_device *vdev)
> }
> }
>
> + netif_tx_lock_bh(vi->dev);
> netif_device_attach(vi->dev);
> + netif_tx_unlock_bh(vi->dev);
> return err;
> }
>
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] VSOCK: The performance problem of vhost_vsock.
From: Jason Wang @ 2018-10-17 12:31 UTC (permalink / raw)
To: jiangyiwen, stefanha; +Cc: netdev, kvm, virtualization
In-Reply-To: <5BC72006.9010000@huawei.com>
On 2018/10/17 下午7:41, jiangyiwen wrote:
> On 2018/10/17 17:51, Jason Wang wrote:
>> On 2018/10/17 下午5:39, Jason Wang wrote:
>>>> Hi Jason and Stefan,
>>>>
>>>> Maybe I find the reason of bad performance.
>>>>
>>>> I found pkt_len is limited to VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE(4K),
>>>> it will cause the bandwidth is limited to 500~600MB/s. And once I
>>>> increase to 64k, it can improve about 3 times(~1500MB/s).
>>>
>>> Looks like the value was chosen for a balance between rx buffer size and performance. Allocating 64K always even for small packet is kind of waste and stress for guest memory. Virito-net try to avoid this by inventing the merge able rx buffer which allows big packet to be scattered in into different buffers. We can reuse this idea or revisit the idea of using virtio-net/vhost-net as a transport of vsock.
>>>
>>> What interesting is the performance is still behind vhost-net.
>>>
>>> Thanks
>>>
>>>> By the way, I send to 64K in application once, and I don't use
>>>> sg_init_one and rewrite function to packet sg list because pkt_len
>>>> include multiple pages.
>>>>
>>>> Thanks,
>>>> Yiwen.
>>
>> Btw, if you're using vsock for transferring large files, maybe it's more efficient to implement sendpage() for vsock to allow sendfile()/splice() work.
>>
>> Thanks
>>
> I can't agree more.
>
> why vhost_vsock is still behind vhost_net?
> Because I use sendfile() to test performance at first, and then
> I found vsock don't implement sendpage() and cause the bandwidth
> can't be increased. So I use read() and send() to replace sendfile(),
> it will increase some switch between kernel and user mode, and sendfile()
> can support zero copy. I think this is main reason.
>
> Thanks.
Want to post patches for this then :) ?
Thanks
>
>> .
>>
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ 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