From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id A3AEB7D30 for ; Fri, 8 Dec 2017 14:57:37 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Dec 2017 05:57:36 -0800 Message-Id: Date: 08 Dec 2017 05:57:35 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,377,1508828400"; d="scan'208";a="16567191" Received: from unknown (HELO localhost.localdomain) ([10.240.176.250]) by orsmga002.jf.intel.com with ESMTP; 08 Dec 2017 05:57:35 -0800 Content-Type: multipart/alternative; boundary="===============6148544909773306095==" MIME-Version: 1.0 From: sys_stv@intel.com To: test-report@dpdk.org CC: maxime.coquelin@redhat.com Subject: [dpdk-test-report] |FAILURE| pw32028 [PATCH] vhost_user: protect active rings from async ring changes Re: [PATCH] vhost_user: protect active rings from async ring>> changes>>>>>>>> On 12/08/2017 03:14 AM Tan Jianfeng wrote:>>>>>>>>>> -----Original Message----->>>> Re: [PATCH] vhost_user: protect active rings from async ring>>>> changes>>>>>>>>>>>>>>>> On 12/07/2017 10:33 AM Tan Jianfeng wrote:>>>>>>>>>>>>>>>> -----Original Message----->>>>>> [PATCH] vhost_user: protect active rings from async ring>> changes>>>>>>>>>>>> When performing live migration or memory hot-plugging >>>>>> the changes to the device and vrings made by message handler>>>>>> done independently from vring usage by PMD threads.>>>>>>>>>>>> This causes for example segfauls during live-migration>>>>>>>>>> segfauls ->segfaults?>>>>>>>>>>> with MQ enable but in general virtually any request>>>>>> sent by qemu changing the state of device can cause>>>>>> problems.>>>>>>>>>>>> These patches fixes all above issues by adding a spinlock>>>>>> to every vring and requiring message handler to start operation>>>>>> only after ensuring that all PMD threads related to the divece>>>>>>>>>> Another typo: divece.>>>>>>>>>>> are out of critical section accessing the vring data.>>>>>>>>>>>> Each vring has its own lock in order to not create contention>>>>>> between PMD threads of different vrings and to prevent>>>>>> performance degradation by scaling queue pair number.>>>>>>>>>> Also wonder how much overhead it brings.>>>>>>>>>> Instead of locking each vring can we just waiting a while (10us for>> example)>>>> after call destroy_device() callback so that every PMD thread has enough>>>> time to skip out the criterial area?>>>>>>>> No because we are not destroying the device when it is needed.>>>> Actually once destroy_device() is called it is likely that the>>>> application has taken care the ring aren't being processed anymore>>>> before returning from the callback (This is at least the case with Vhost>>>> PMD).>>>>>> OK I did not put it right way as there are multiple cases above: migration>> and memory hot plug. Let me try again:>>>>>> Whenever a vhost thread handles a message affecting PMD threads (like>> SET_MEM_TABLE GET_VRING_BASE etc) we can remove the dev flag ->> VIRTIO_DEV_RUNNING and wait for a while so that PMD threads skip out of>> those criterial area. After message handling reset the flag ->> VIRTIO_DEV_RUNNING.>>>> I think you mean clearing vq's enabled flag because PMD threads never>> check the VIRTIO_DEV_RUNNING flag.> > Ah yes.> >>>>> I suppose it can work basing on an assumption that PMD threads work in>> polling mode and can skip criterial area quickly and inevitably.>>>> That sounds very fragile because if the CPU aren't perfectly isolated >> your PMD thread can be preempted for interrupt handling for example.>>>> Or what if for some reason the PMD thread CPU stalls for a short while?>>>> The later is unlikely but if it happens it will be hard to debug.>>>> Let's see first the performance impact of using the spinlock. It might>> not be that important because 99.9999% of the times it will not even>> spin.> > Fair enough.I did some benchmarks on my Broadwell test bench (see patch below) andit seems that depending on the benchmark perfs are on par or betterwith the spinlock! I guess it explains because with the spinlock thereis a better batching and less concurrent accesses on the rings but I'mnot sure.Please find my results below (CPU E5-2667 v4 @ 3.20GHz): Bench v17.11 v17.11 + spinlock ---------------- ----------- ------------------- PVP Bidir run1 19.29Mpps 19.26Mpps PVP Bidir run2 19.26Mpps 19.28Mpps TxOnly 18.47Mpps 18.83Mpps RxOnly 13.83Mpps 13.83Mpps IO Loopback 7.94Mpps 7.97MppsPatch:--------------------------------------------------------- From 7e18cefce682235558fed66a1fb87ab937ec9297 Mon Sep 17 00:00:00 2001 X-BeenThere: test-report@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: automatic DPDK test reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Dec 2017 13:57:38 -0000 --===============6148544909773306095== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 VGVzdC1MYWJlbDogSW50ZWwtY29tcGlsYXRpb24KVGVzdC1TdGF0dXM6IEZBSUxVUkUKaHR0cDov L2RwZGsub3JnL3BhdGNoLzMyMDI4CgpfYXBwbHkgcGF0Y2ggZmlsZSBmYWlsdXJlXwoKU3VibWl0 dGVyOiBNYXhpbWUgQ29xdWVsaW4gPG1heGltZS5jb3F1ZWxpbkByZWRoYXQuY29tPgpEYXRlOiBG cmksIDggRGVjIDIwMTcgMTE6MTE6NDAgKzAxMDBGcmksIDggRGVjIDIwMTcgMDQ6MjE6MjUgLTA1 MDAKRFBESyBnaXQgYmFzZWxpbmU6IFJlcG86ZHBkay1uZXh0LWV2ZW50ZGV2LCBCcmFuY2g6bWFz dGVyLCBDb21taXRJRDpjMmQxMmNlNjNkNGQ5ZWEyNzQwOTNmZThjNDJmZTgwNjI4N2ZlZjhhCiAg ICAgICAgICAgICAgICAgICBSZXBvOmRwZGstbmV4dC1jcnlwdG8sIEJyYW5jaDptYXN0ZXIsIENv bW1pdElEOjIyNDM3NGNjMGUzY2E0NGFmNTE0MWZiNzAzNWE5N2YzMzhkMDBjMTgKICAgICAgICAg ICAgICAgICAgIFJlcG86ZHBkay1uZXh0LW5ldCwgQnJhbmNoOm1hc3RlciwgQ29tbWl0SUQ6Y2U3 ZWFiNDA4NDVlYzNiZDA5N2IxMDcxZjEyZTNkOWQ2ODA5ZmMwYQogICAgICAgICAgICAgICAgICAg UmVwbzpkcGRrLW5leHQtdmlydGlvLCBCcmFuY2g6bWFzdGVyLCBDb21taXRJRDo0Zjc2MWM5NGI1 MjBjZTcxYzU2YmUxYzkxM2U1NGE0MTc5YjgxYzQzCiAgICAgICAgICAgICAgICAgICBSZXBvOmRw ZGssIEJyYW5jaDptYXN0ZXIsIENvbW1pdElEOjIyNDM3NGNjMGUzY2E0NGFmNTE0MWZiNzAzNWE5 N2YzMzhkMDBjMTgKICAgICAgICAgICAgICAgICAgIApBcHBseSBwYXRjaCBmaWxlIGZhaWxlZDoK UmVwbzogZHBkawozMjAyODoKcGF0Y2hpbmcgZmlsZSBsaWIvbGlicnRlX3Zob3N0L3Zob3N0LmMK SHVuayAjMSBGQUlMRUQgYXQgMjE5LgoxIG91dCBvZiAxIGh1bmsgRkFJTEVEIC0tIHNhdmluZyBy ZWplY3RzIHRvIGZpbGUgbGliL2xpYnJ0ZV92aG9zdC92aG9zdC5jLnJlagpwYXRjaGluZyBmaWxl IGxpYi9saWJydGVfdmhvc3Qvdmhvc3QuaApIdW5rICMxIEZBSUxFRCBhdCAxMzEuCjEgb3V0IG9m IDEgaHVuayBGQUlMRUQgLS0gc2F2aW5nIHJlamVjdHMgdG8gZmlsZSBsaWIvbGlicnRlX3Zob3N0 L3Zob3N0LmgucmVqCnBhdGNoaW5nIGZpbGUgbGliL2xpYnJ0ZV92aG9zdC92aXJ0aW9fbmV0LmMK SHVuayAjMSBGQUlMRUQgYXQgMzI5LgpIdW5rICMyIEZBSUxFRCBhdCA0MTkuCkh1bmsgIzMgRkFJ TEVEIGF0IDY1NC4KSHVuayAjNCBGQUlMRUQgYXQgNzE1LgpIdW5rICM1IEZBSUxFRCBhdCAxMTgz LgpIdW5rICM2IEZBSUxFRCBhdCAxMzU2Lgo2IG91dCBvZiA2IGh1bmtzIEZBSUxFRCAtLSBzYXZp bmcgcmVqZWN0cyB0byBmaWxlIGxpYi9saWJydGVfdmhvc3QvdmlydGlvX25ldC5jLnJlagoKUmVw bzogZHBkay1uZXh0LWNyeXB0bwozMjAyODoKcGF0Y2hpbmcgZmlsZSBsaWIvbGlicnRlX3Zob3N0 L3Zob3N0LmMKSHVuayAjMSBGQUlMRUQgYXQgMjE5LgoxIG91dCBvZiAxIGh1bmsgRkFJTEVEIC0t IHNhdmluZyByZWplY3RzIHRvIGZpbGUgbGliL2xpYnJ0ZV92aG9zdC92aG9zdC5jLnJlagpwYXRj aGluZyBmaWxlIGxpYi9saWJydGVfdmhvc3Qvdmhvc3QuaApIdW5rICMxIEZBSUxFRCBhdCAxMzEu CjEgb3V0IG9mIDEgaHVuayBGQUlMRUQgLS0gc2F2aW5nIHJlamVjdHMgdG8gZmlsZSBsaWIvbGli cnRlX3Zob3N0L3Zob3N0LmgucmVqCnBhdGNoaW5nIGZpbGUgbGliL2xpYnJ0ZV92aG9zdC92aXJ0 aW9fbmV0LmMKSHVuayAjMSBGQUlMRUQgYXQgMzI5LgpIdW5rICMyIEZBSUxFRCBhdCA0MTkuCkh1 bmsgIzMgRkFJTEVEIGF0IDY1NC4KSHVuayAjNCBGQUlMRUQgYXQgNzE1LgpIdW5rICM1IEZBSUxF RCBhdCAxMTgzLgpIdW5rICM2IEZBSUxFRCBhdCAxMzU2Lgo2IG91dCBvZiA2IGh1bmtzIEZBSUxF RCAtLSBzYXZpbmcgcmVqZWN0cyB0byBmaWxlIGxpYi9saWJydGVfdmhvc3QvdmlydGlvX25ldC5j LnJlagoKUmVwbzogZHBkay1uZXh0LW5ldAozMjAyODoKcGF0Y2hpbmcgZmlsZSBsaWIvbGlicnRl X3Zob3N0L3Zob3N0LmMKSHVuayAjMSBGQUlMRUQgYXQgMjE5LgoxIG91dCBvZiAxIGh1bmsgRkFJ TEVEIC0tIHNhdmluZyByZWplY3RzIHRvIGZpbGUgbGliL2xpYnJ0ZV92aG9zdC92aG9zdC5jLnJl agpwYXRjaGluZyBmaWxlIGxpYi9saWJydGVfdmhvc3Qvdmhvc3QuaApIdW5rICMxIEZBSUxFRCBh dCAxMzEuCjEgb3V0IG9mIDEgaHVuayBGQUlMRUQgLS0gc2F2aW5nIHJlamVjdHMgdG8gZmlsZSBs aWIvbGlicnRlX3Zob3N0L3Zob3N0LmgucmVqCnBhdGNoaW5nIGZpbGUgbGliL2xpYnJ0ZV92aG9z dC92aXJ0aW9fbmV0LmMKSHVuayAjMSBGQUlMRUQgYXQgMzI5LgpIdW5rICMyIEZBSUxFRCBhdCA0 MTkuCkh1bmsgIzMgRkFJTEVEIGF0IDY1NC4KSHVuayAjNCBGQUlMRUQgYXQgNzE1LgpIdW5rICM1 IEZBSUxFRCBhdCAxMTgzLgpIdW5rICM2IEZBSUxFRCBhdCAxMzU2Lgo2IG91dCBvZiA2IGh1bmtz IEZBSUxFRCAtLSBzYXZpbmcgcmVqZWN0cyB0byBmaWxlIGxpYi9saWJydGVfdmhvc3QvdmlydGlv X25ldC5jLnJlagoKUmVwbzogZHBkay1uZXh0LXZpcnRpbwozMjAyODoKcGF0Y2hpbmcgZmlsZSBs aWIvbGlicnRlX3Zob3N0L3Zob3N0LmMKSHVuayAjMSBGQUlMRUQgYXQgMjE5LgoxIG91dCBvZiAx IGh1bmsgRkFJTEVEIC0tIHNhdmluZyByZWplY3RzIHRvIGZpbGUgbGliL2xpYnJ0ZV92aG9zdC92 aG9zdC5jLnJlagpwYXRjaGluZyBmaWxlIGxpYi9saWJydGVfdmhvc3Qvdmhvc3QuaApIdW5rICMx IEZBSUxFRCBhdCAxMzEuCjEgb3V0IG9mIDEgaHVuayBGQUlMRUQgLS0gc2F2aW5nIHJlamVjdHMg dG8gZmlsZSBsaWIvbGlicnRlX3Zob3N0L3Zob3N0LmgucmVqCnBhdGNoaW5nIGZpbGUgbGliL2xp YnJ0ZV92aG9zdC92aXJ0aW9fbmV0LmMKSHVuayAjMSBGQUlMRUQgYXQgMzI5LgpIdW5rICMyIEZB SUxFRCBhdCA0MTkuCkh1bmsgIzMgRkFJTEVEIGF0IDY1NC4KSHVuayAjNCBGQUlMRUQgYXQgNzE1 LgpIdW5rICM1IEZBSUxFRCBhdCAxMTgzLgpIdW5rICM2IEZBSUxFRCBhdCAxMzU2Lgo2IG91dCBv ZiA2IGh1bmtzIEZBSUxFRCAtLSBzYXZpbmcgcmVqZWN0cyB0byBmaWxlIGxpYi9saWJydGVfdmhv c3QvdmlydGlvX25ldC5jLnJlagoKUmVwbzogZHBkay1uZXh0LWV2ZW50ZGV2CjMyMDI4OgpwYXRj aGluZyBmaWxlIGxpYi9saWJydGVfdmhvc3Qvdmhvc3QuYwpIdW5rICMxIEZBSUxFRCBhdCAyMTku CjEgb3V0IG9mIDEgaHVuayBGQUlMRUQgLS0gc2F2aW5nIHJlamVjdHMgdG8gZmlsZSBsaWIvbGli cnRlX3Zob3N0L3Zob3N0LmMucmVqCnBhdGNoaW5nIGZpbGUgbGliL2xpYnJ0ZV92aG9zdC92aG9z dC5oCkh1bmsgIzEgRkFJTEVEIGF0IDEzMS4KMSBvdXQgb2YgMSBodW5rIEZBSUxFRCAtLSBzYXZp bmcgcmVqZWN0cyB0byBmaWxlIGxpYi9saWJydGVfdmhvc3Qvdmhvc3QuaC5yZWoKcGF0Y2hpbmcg ZmlsZSBsaWIvbGlicnRlX3Zob3N0L3ZpcnRpb19uZXQuYwpIdW5rICMxIEZBSUxFRCBhdCAzMjku Ckh1bmsgIzIgRkFJTEVEIGF0IDQxOS4KSHVuayAjMyBGQUlMRUQgYXQgNjU0LgpIdW5rICM0IEZB SUxFRCBhdCA3MTUuCkh1bmsgIzUgRkFJTEVEIGF0IDExODMuCkh1bmsgIzYgRkFJTEVEIGF0IDEz NTYuCjYgb3V0IG9mIDYgaHVua3MgRkFJTEVEIC0tIHNhdmluZyByZWplY3RzIHRvIGZpbGUgbGli L2xpYnJ0ZV92aG9zdC92aXJ0aW9fbmV0LmMucmVqCgoKRFBESyBTVFYgdGVhbQo= --===============6148544909773306095==--